Keep an aliased import's alias when ChangeType retargets it - #8409
Merged
Conversation
knutwannheden
force-pushed
the
changetype-alias-detection-via-importservice
branch
2 times, most recently
from
August 6, 2026 12:17
aebda7d to
895a051
Compare
ChangeType via ImportServiceChangeType retargets it
ChangeType reuses an aliased import's alias for the target import so that references to the alias keep resolving. It considered every J.Import it visited, including those nested in a block — a function-local import or one under `if TYPE_CHECKING:` in Python. Those bind a narrower scope than the file-level import added in their place, which the nested binding then goes on shadowing, so the file gained a line that changed no behaviour. Consider only imports with no enclosing block. Java, Kotlin, and Groovy have no nested imports and are unaffected.
Groovy parses import aliases and its printer emits them, but it has no ImportService override and so adds imports through Java's AddImport, which dropped the alias: `import java.util.ArrayList as MyList` became `import java.util.LinkedList`, leaving the bound name unresolved. The alias has reached the visitor since #3560, which threaded it through maybeAddImport and addImportVisitor so Kotlin's own AddImport could receive it, but it was never applied to the J.Import that Java's implementation constructs. Set it there. JavaPrinter emits no alias, so a Java file prints identically whatever the import holds, and the non-aliased add ChangeType also queues is skipped because the type is by then explicitly imported.
Identifiers inside import statements are bindings rather than uses, but the check counted them, so a module named only by an existing import satisfied only_if_referenced and a second, redundant import was added. RemoveImport's collector and Java's AddImport already draw this distinction.
knutwannheden
force-pushed
the
changetype-alias-detection-via-importservice
branch
from
August 6, 2026 12:54
895a051 to
1495d9e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
ChangeTypereuses an aliased import's alias when it retargets a type, so thatimport a.b.Original as MyAliasbecomesimport x.y.Target as MyAliasand references toMyAliaskeep resolving (#3558, #3560). Verifying that behaviour across languages turned up three ways it goes wrong outside Kotlin.In Python, imports can appear inside a function body or under
if TYPE_CHECKING:. The alias capture considered everyJ.Importit visited, so such an import produced a file-level aliased import while the nested one stayed put and went on shadowing it wherever it applied. The file gained a line that changed no behaviour.In Groovy, the alias was found but silently discarded on the way out:
import java.util.ArrayList as MyListbecameimport java.util.LinkedList, leavingMyListunresolved and the file broken.Python's
AddImportalso treated a name bound by an existing import as a reference to that name, soonly_if_referencedwas satisfied by the import itself and a redundant second import was added.Examples
Python keeps the alias, and usages need no rewriting:
A function-local import is left alone. Previously the file gained a
from builtins import list as Lline abovedef foo, while the local import stayed and kept shadowing it inside the function:The same holds for
if TYPE_CHECKING:, where the added line would sit outside the guard that exists to keep the import off the runtime path:Groovy keeps the alias instead of dropping it:
Summary
ChangeType's alias capture to imports with no enclosingJ.Block. Java, Kotlin, and Groovy have no nested imports and are unaffected; Python's function-local andif TYPE_CHECKING:imports are now skipped, since the file-level import that would replace them binds a wider scope than the original.J.Importthat Java'sAddImportconstructs. It has reached the visitor since Update AddImport with alias to support Kotlin alias import #3560 — threaded throughmaybeAddImport/addImportVisitorso Kotlin's ownAddImportcould receive it — but was never applied to the import Java's implementation builds, so Groovy lost it.JavaPrinteremits no alias, so a Java file prints identically whatever the import holds. The non-aliased add thatChangeTypealso queues is skipped, because by then the type is explicitly imported.AddImportreference check, matchingRemoveImport's collector and Java'sAddImport.Test plan
tests/rpc/test_java_recipes.pycovering the aliased from-import, a re-exported import named by its canonical type (collections.abc.Iterableis attributedtyping.Iterable), the standaloneimport <module> as <alias>statement, and the function-local andif TYPE_CHECKING:imports left unchanged. Both nested cases were confirmed to fail without the guard.changeImportKeepsAliasinChangeTypeAdaptabilityTest, which fails onmainand passes here.test_add_import.pycase for a name occurring only inside an import statement.rewrite-java-test(2167 tests),rewrite-kotlin(1309, includingchangeImportAlias,changeTypeWithGenericArgumentAlias,addImportAlias,updateImportAlias),rewrite-groovy(678), and Python (1803 acrosstests/python,tests/recipes,tests/rpc).RequirementsTxtParserTest.markerContainsDependenciesFromFreezefails on this branch, but fails identically on a clean checkout ofmain— it resolves live packages from PyPI and is unrelated to these changes.