Skip to content

Python: an import in a module-scope if binds at module scope - #8767

Merged
knutwannheden merged 1 commit into
mainfrom
attribution-excludes-imports-in-a-module-scope-if
Sep 3, 2026
Merged

Python: an import in a module-scope if binds at module scope#8767
knutwannheden merged 1 commit into
mainfrom
attribution-excludes-imports-in-a-module-scope-if

Conversation

@knutwannheden

@knutwannheden knutwannheden commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

A call to a removed API gets no declaring type when the import binding it sits inside a module-scope if — the shape a codebase mid-migration actually writes:

if sys.version_info < (3, 11):
    from gettext import lgettext

Measured on gettext.lgettext, which 3.14's typeshed does not declare. Position is not the problem: ty resolves a live symbol from every one of these.

live symbol removed symbol
module scope owner ✓ owner ✓
star import owner ✓ owner ✓
aliased owner ✓ owner ✓
if-guarded owner ✓ owner ✗
function-local owner ✓ owner ✗
try/except owner ✓ owner ✗

The mechanism

A symbol ty cannot type is attributed from the file's own import bindings instead. _import_bindings indexed tree.body only, so a nested alias never entered the index; _rebound_names then added every alias outside that index to the shadowed set, so the name was actively excluded rather than merely absent. That exclusion is deliberate — a name bound twice cannot be read off an import — and right for a function-local or else-guarded import.

The fix

_module_scope_statements walks a module body plus the bodies of ifs with no else or elif: those bind into the enclosing scope, and nothing else binds them. Only that shape widens.

The two other failing rows keep failing, and need no new predicate. A try's handler can bind the name to a shim (gone = None), a genuine second binding _rebound_names already excludes on its own merits. A function-local import is scoped, and a module-level reference is not that binding.

Tests

  • The new rows go in TestSymbolTheStubsDoNotDeclare, which calls a symbol its fixture module does not define. A fixture using a symbol the typeshed still declares passes whether or not the binding path was reached — the confound behind the wrong conclusions on Python: let a project's declared version pick the typeshed #8759.

test_an_import_under_an_unconditional_if_binds_at_module_scope covers one guard and nested guards; ..._with_an_else_branch_... and ..._in_a_try_... pin the declines. Each mutation-checked against the line it claims: dropping the orelse guard fails the else test, widening the helper to ast.Try fails the try test, and dropping its recursion fails the nested row.

Downstream

In moderneinc/rewrite-migrate-python this is necessary but not sufficient. Run against the same guarded fixture per recipe, on this branch:

  • FindSslMatchHostname and FindLocaleGetdefaultlocale act on it. The first gates on _is_ssl_method(...) before member_called(...); the second is a bare MethodMatcher.create("locale getdefaultlocale(..)"). Both read the declaring type directly.
  • gettext lgettext, locale resetlocale and platform popen still decline. They reach the receiver-less form only through FromImports.member_called, whose index is built from cu.statements — top-level only, exactly as this one was.

That helper's docstring gives the limit try/except's reasoning, applied to the unconditional if too: the same conflation, one layer down. It widens separately, after this merges.

Full downstream suite on this branch: 668 passed, no test changed outcome in either direction.

A call to a symbol ty cannot type is attributed from the file's own import
bindings. That index read `tree.body` only, and `_rebound_names` then excluded
every alias outside it, so a version-guarded `from gettext import lgettext`
lost its declaring type. Walk `if` bodies with no `else` too, which is the rule
`import_utils.unconditional_body` already states for `RemoveImport`.
@knutwannheden
knutwannheden force-pushed the attribution-excludes-imports-in-a-module-scope-if branch from fafddbd to 3879ed2 Compare September 3, 2026 16:36
@knutwannheden
knutwannheden merged commit ff3fcc5 into main Sep 3, 2026
1 check passed
@knutwannheden
knutwannheden deleted the attribution-excludes-imports-in-a-module-scope-if branch September 3, 2026 16:39
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant