[asukaminato0721]

Summary

Fixes #4713

the implicit-import warning now offers an "Insert import" quick fix.

Existing limitation: aliased references can retain the warning after the correct import is inserted.

Test Plan

add test

[meta-codesync]

This pull request has been imported. If you are a Meta employee, you can view this in D121025008. (Because this pull request was imported automatically, there will not be any future comments.)

[stroxler]

The diagnostic plumbing itself looks good, but there are some problematic behaviors:

Preserve the existing import’s scope

At [lsp.rs:3607–3616](https://www.internalfb.com/code/fbsource/[D121025008-V11]/fbcode/pyrefly/pyrefly/lib/state/lsp.rs?lines=3607-3616), this code:

def f():
    import foo
    return foo.bar.x

gets a module-level import foo.bar before def f. That turns a lazy dependency into an eager one, potentially breaking module initialization. The implicit-import warning also remains, because the local import does not inherit the outer scope’s submodule imports.

Insert beside the relevant local import, or withhold the action when its scope cannot be preserved.

Aliased receivers can introduce builtin shadowing

The same action always inserts a plain canonical import. With a package named str, and its submodule already loaded by another dependency:

import str as s
y = str(1)
x = s.bar.x

the fix adds import str.bar, binding the global name str. The previously valid builtin call then raises TypeError; rechecking also produces NotCallable.

Use a collision-free import plus a targeted reference rewrite, or suppress the action when introducing the root name is unsafe.

The alias fix creates an “unused import” that invites undoing it

For the PR’s own import foo as f; x = f.bar.x case, adding import foo.bar clears the warning but records one unused import: foo. The editor’s removal action then undoes the fix.

The [new test](https://www.internalfb.com/code/fbsource/[D121025008-V11]/fbcode/pyrefly/pyrefly/lib/test/lsp/code_actions.rs?lines=934-946) checks ordinary errors, so it misses this. Add unused-import coverage and account for imports needed by aliased accesses.

[stroxler]

Looks like it needs another rebase before we can merge. The update addresses the earlier scope, builtin-shadowing, and unused-import concerns; since we need another update anyway though, here are some additional concerns that came up:

Handle ambiguous reaching imports

At [implicit_import.rs:41–53](https://github.com/meta/pyrefly/blob/7f8abad4370fb8c4092ec8dea877ddad66ecb0c9/pyrefly/lib/state/lsp/quick_fixes/implicit_import.rs#L41-L53), a singleton find_definition result does not establish a unique reaching import: navigation follows only the first branch of a Binding::Phi.

For example:

def f(flag: bool):
    if flag:
        import foo
    else:
        import foo
    return foo.bar.x

The action changes only the first branch to import foo.bar. With an empty foo/__init__.py and foo/bar.py containing x = 1, Pyrefly clears the warning, but a fresh-process f(False) still raises AttributeError.

Please either update all reaching imports or withhold the action when binding provenance is ambiguous. Add regression coverage for branch merges and loop-carried reimports; checking the navigation result’s length is insufficient.

Exercise the alias guard directly

Add this case using the existing foo.bar.baz fixture, expecting no import action:

import foo.bar as foo
x = foo.baz.x

The existing alias tests return early because the receiver name differs from the canonical root. This case reaches alias.asname.is_some() and protects against accidentally rewriting the import to import foo.bar.baz as foo, which would change what foo refers to.

The imported diff remains untouched; no fix-forward diffs were submitted.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅