[asukaminato0721]

Summary

Fixes #1490

continue #1580

Removed unknown-name repair and its second check, retained import merging and LSP support and applied edits in one batch.

• I am a human, and I fully understand the code I am submitting.

Test Plan

add test

[meta-codesync]

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

[stroxler]

Looks like this needs a rebase (there's a lot of churn now that we're merging more of your PRs!)

Since it needs a rebase anyway, I asked for any new issues flagged in AI review and there are a few, although it's definitely improving :)

Issues flagged in second-round review:

• Import-resolution panic: valid per-file configurations make both infer and --dry-run crash with exit 101 when a type’s defining module isn’t directly importable.
• Alias rebinding: merging Foo into from file_two import marker as Foo, make overwrites the existing alias and changes runtime behavior.
• Conditional imports become unconditional: an existing TYPE_CHECKING import is overlooked. Inference adds a runtime import, making a previously working program fail with ImportError.
• Lost quick-fix choices: imports from different modules both produce ", Thing" merge edits. Text-only deduplication silently removes one alternative.

---

More detailed report, including some suggested fixes

---

1. P2 — Import merging overwrites existing aliases and changes runtime behavior

Location: ide.rs:423–435.

The planner checks whether the requested export is already imported, but not whether its local name is already bound by a different import alias. Appending that name to the end of the statement overwrites the alias.

# file_two.py
class Foo:
    pass
marker = 17

def make() -> Foo:
    return Foo()

# main.py
from file_two import marker as Foo, make

def foo():
    return make()

assert Foo == 17

Running infer rewrites the import to from file_two import marker as Foo, make, Foo and annotates foo with -> Foo. Inference exits 0, but executing the rewritten program now fails: Foo is the class instead of the original value 17. Before inference, execution succeeds. The base's separately prepended import leaves the later alias binding intact; the new merge changes that ordering.

Fix: detect local binding collisions. Qualify through a safe module binding, use a collision-free alias, or skip the unsafe annotation/import; do not append a name that overwrites an existing alias. Add an end-to-end inference regression.

2. P2 — Type-only and guarded imports become unconditional runtime imports

Locations: infer.rs:378–401, ide.rs:413–458.

The base skipped an exact import line already present anywhere in the file. The new already-imported check only searches the initial contiguous import block. Imports inside if TYPE_CHECKING, try blocks, or after non-import statements are overlooked.

# main.py
from __future__ import annotations
from typing import TYPE_CHECKING
from file_two import get_a

if TYPE_CHECKING:
    from optional_dep import ExampleA

def foo():
    return get_a()

For the compiled reproducer, file_two.get_a has a postponed ExampleA return annotation and imports that class only under TYPE_CHECKING. The optional dependency defines ExampleA, then raises ImportError("optional backend absent") when imported at runtime. The original program loads successfully.

infer adds an unconditional from optional_dep import ExampleA after the future import, despite the existing type-only import. Inference exits 0; the rewritten program now fails to load with ImportError. Postponed annotations did not require that runtime dependency. The same search restriction can defeat deliberately guarded optional imports or introduce import cycles.

Fix: recognize annotation bindings/imports outside the mergeable leading block without making intentionally conditional imports unconditional. Use bindings/import tracking rather than restoring substring matching. Keep merge placement restricted to the leading block.

3. P2 — Valid per-file configurations cause infer and --dry-run to panic

Location: infer.rs:385–388.

An inferred class can be visible through a facade while its defining module is not directly importable under the caller's configuration. The new import-handle lookup assumes otherwise and calls .expect("infer import source should always resolve").

Reproducer layout:

nested/
  pyrefly.toml
  main.py
  facade/
    pyrefly.toml
    __init__.py
hidden/
  secret.py

• nested/pyrefly.toml: search-path = ["."], disable-search-path-heuristics = true.
• nested/facade/pyrefly.toml: search-path = ["../../hidden"], disable-search-path-heuristics = true.
• hidden/secret.py defines Foo.
• The facade re-exports Foo using from secret import Foo as Foo and defines make() -> Foo.
• main.py imports only make from the facade and defines an unannotated function returning make().

Run in file mode, without supplying a global --config:

pyrefly check nested/main.py --skip-interpreter-query
pyrefly infer nested/main.py --parameter-types=false --skip-interpreter-query
pyrefly infer nested/main.py --parameter-types=false --skip-interpreter-query --dry-run

Compiled-head results: check exits 0 with zero errors; both inference commands exit 101, panicking at infer.rs:388 with the new expect message. This is a user-reachable failure, not an established invariant.

Fix: do not panic on failed caller-relative resolution. Prefer a reachable re-export or skip/report annotations whose imports cannot be supplied safely. Since the absolute-format path only needs the already-known ModuleName, also reconsider whether this extra handle lookup is necessary.

4. P2 — Distinct import quick-fix choices collapse after merging

Locations: the new suffix at ide.rs:432–435; affected existing deduplication at lsp.rs:3792–3796.

Both a and b export Thing. Request quick fixes on Thing here:

from a import marker
from b import marker
Thing()

Each import choice now inserts the same suffix, ", Thing", but into a different statement. The unchanged dedup_by(|a, b| a.insert_text == b.insert_text) treats them as the same import and removes one choice.

End-to-end Rust validation: a temporary test using the existing batched LSP report helper expected both from a import Thing and from b import Thing choices. It failed: the report contained only the a import choice. Failing test run. The temporary test was removed after validation.

Fix: deduplicate by semantic import identity or complete edit identity, including target range/module—not insertion text alone. Audit the analogous text-only sets in pytest fixture import batching. Add this LSP-level regression to the PR.

Smaller review concerns

• Inlay-hint ownership/performance: inlay_hints.rs:144–175 computes merge plans, then discards their ranges and insertion text and emits separate full lines using display_text. The imports field is already a vector. Either retain structured edits or use a spelling-only helper; avoid repeated import-block scans for plans that will not be used. This is source-level analysis, not a measured latency claim.
• Weakened test assertion: infer.rs:1203 accepts a pre-existing from short import get_a plus an annotation containing MyClass, even if MyClass was never imported. Assert the actual imported names or the exact merged statement.

[stroxler]

Whoops, meant to request changes

[github-actions]

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