Skip to content

Python: fix AutoFormat crash on import ... as ...#7412

Merged
knutwannheden merged 1 commit intomainfrom
fix-spacesvisitor.visit_import-clobbering-alias-wrapper
Apr 17, 2026
Merged

Python: fix AutoFormat crash on import ... as ...#7412
knutwannheden merged 1 commit intomainfrom
fix-spacesvisitor.visit_import-clobbering-alias-wrapper

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

AutoFormat crashed on any Python file containing import X as Y or from X import Y as Z, surfacing as AttributeError: 'Identifier' object has no attribute 'before' deep in SpacesVisitor. Discovered via flagship recipe alerts while running UnwrapElseAfterReturn on real sources.

Root cause is a subtle mismatch between public and private fields on padded LST nodes. Import._alias is typed Optional[JLeftPadded[Identifier]], while the public alias property unwraps to the bare Identifier. The previous code did:

imp = imp.replace(alias=space_before(imp.alias, True))
imp = imp.padding.replace(alias=space_before_left_padded(imp.padding.alias, True))

replace_if_changed maps the public kwarg alias to the private field _alias without type-checking the value, so the first line clobbered the JLeftPadded wrapper with a raw Identifier. The second line then read imp.padding.alias back as a bare Identifier and crashed on .before.

Summary

  • SpacesVisitor.visit_import now applies both spacing adjustments to the JLeftPadded wrapper (using space_before_left_padded for the space before as, and space_before_left_padded_element for the space before the alias identifier) before a single padding.replace.
  • Added a regression test covering both import X as Y and from X import Y as Z with non-normalized spacing.

Test plan

  • New test_import_with_alias_normalizes_spacing fails on trunk with the reported AttributeError and passes after the fix
  • Full format + import parse suite: pytest tests/python/all/format/ tests/python/all/tree/import_test.py tests/python/format/ — 181 passed
  • Broader smoke: pytest tests/ --ignore=tests/rpc — 1157 passed, 6 skipped

`SpacesVisitor.visit_import` wrote a raw `Identifier` into `Import._alias`
(typed `JLeftPadded[Identifier]`) because `replace_if_changed` maps public
kwarg names to private fields without type-checking the value. The second
spacing adjustment then read `imp.padding.alias` back as a bare `Identifier`
and crashed with `AttributeError: 'Identifier' object has no attribute
'before'`.

Apply both the before-`as` and before-identifier spacing to the
`JLeftPadded` wrapper before writing it back.
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Apr 17, 2026
@knutwannheden knutwannheden merged commit d0c843e into main Apr 17, 2026
1 check passed
@knutwannheden knutwannheden deleted the fix-spacesvisitor.visit_import-clobbering-alias-wrapper branch April 17, 2026 10:17
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Apr 17, 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