Repository navigation
Changed Rename on a Kotlin package folder to rename the directory together with its packages - #151
Conversation
…ether with its packages Refactor → Rename on a package folder now renames the physical directory, derives every descendant's package from its resulting path (including files whose package did not match the directory), and updates supported Kotlin imports and references. Rename Kotlin Package Directory... stays directory-only, Change Kotlin Package... stays package-only, and Rename is no longer offered for the src/main/kotlin and src/test/kotlin source roots. On a .kt file, Rename also renames a matching top-level Kotlin class and its supported Kotlin references. KaChangePackageComputer keeps the copied IDEA processor's usage discovery, conflict checking, and package rewriting, but retargets references from the captured source text: in standalone NetBeans, rewriting a package directive invalidates every PSI smart pointer held by a K2 UsageInfo, which made retargetUsagesAfterMove crash with PsiInvalidElementAccessException. Imports and fully qualified references are now rewritten by FQN, and callers that lose an implicit same-package binding receive an explicit import. Also added: - KotlinPackageRenameMapping for path-derived target package calculation. - RegistryValue.addListener(RegistryValueListener, Disposable) so the bundled Kotlin code links against the local registry stub. Co-Authored-By: Claude Code <noreply@anthropic.com>
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Text-based retargeting now adds imports for declarations that stay in a moved
file's former package, rewrites fully qualified references inside ${...}
string templates, leaves triple-quoted raw strings untouched, and never
rewrites declarations moved out of the root package. A package directive
inserted into a file that had none is separated from the following declaration.
Co-Authored-By: Claude Code <noreply@anthropic.com>
…orts, and unbound chains Callers that imported the whole old package with a wildcard now receive an explicit import for the moved declaration, while the wildcard itself is kept for declarations that stay behind. Explicit imports of default-package declarations are rewritten to the new package even though bare-name code references remain untouched to protect declaration names. Replaced the lexical fully qualified name scanner with replacement ranges captured from PSI before the package directive changes: direct import directives plus qualified chains that K2 resolved to a moved declaration. Comments, string literals, default-package declaration names, and receiver chains bound to unrelated members are therefore never rewritten, while template expressions still follow moved declarations. Added integration tests for wildcard importers, default-package imports, and receiver chains that only spell a moved FQN. Co-Authored-By: Claude Code <noreply@anthropic.com>
A caller that stays in the default package now receives the explicit import it needs even though it has no package directive and no imports: the import is inserted before its first declaration, after file annotations and header comments, using an insertion point captured from PSI before mutation. Import replacement ranges now come from the imported reference PSI instead of searching the rendered import path in the directive text. A backticked segment made that search fail with a negative offset and corrupted text before the directive. Reference retargeting is now captured from the declaration-level usage scan as well, so qualified references to moved type aliases follow the declaration and same-package callers of an alias gain an explicit import. Added integration tests for root-package callers, backticked import paths, and type alias references. Co-Authored-By: Claude Code <noreply@anthropic.com>
…matching Import insertion points and existing-import detection now come from PSI captured before package mutation, so import-looking text inside raw strings or block comments neither counts as an existing import nor places new imports inside a literal. Qualified reference chains are matched by identifier segments instead of exact source text, so backticked and multi-line spellings are retargeted while the captured PSI range still delimits the replacement. Co-Authored-By: Claude Code <noreply@anthropic.com>
…erated names Replacement ranges are shifted by the package directive's length delta only when they start after the directive, so references in file annotations that precede it keep their original offsets. Generated qualified references and imports now render FQN segments with Kotlin source escaping, so declarations whose names need backticks stay valid after the move. Co-Authored-By: Claude Code <noreply@anthropic.com>
Generic Rename of a Kotlin file now rejects a hard-keyword basename that would produce an invalid class declaration and reports a blocking problem when the requested class name is already declared by another class, object, or type alias in the same package. Outgoing dependencies of a moved file now also cover Java classes and constructor calls, which have no Kotlin named declaration; the import is derived from the symbol's class id. Generated declaration names are rendered through KotlinSourceNames, and a package directive inserted into a root-package file is separated from file annotations and adjacent declarations. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2bd45683c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A caller that only uses nested declarations of a moved class is now discovered through its class-scoped wildcard import, and the import prefix follows the class to its new package. Package wildcards are left untouched so they keep binding declarations that stay behind. An aliased import no longer counts as an unaliased binding: a moved file that calls the original name and the alias receives the missing unaliased import next to the alias directive. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93794165df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Renaming a class together with its file now also retargets explicit constructor calls: such a call resolves to a constructor symbol whose PSI is the primary constructor, not the class declaration, so the rename matches the constructor's class id as well. A caller that imported a moved declaration only through an alias is no longer treated as already importing the original name; it receives the unaliased import it needs while the alias directive is rewritten to the new package. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
…e changes A constructor call resolves to a constructor symbol whose PSI is the KtPrimaryConstructor inside the class declaration, not a top-level declaration. The outgoing-dependency lookup tested the PSI first and returned early, so a moved file calling `Helper(1)` lost `import a.Helper`. Resolve class-like and constructor symbols through their class id before falling back to the PSI branch. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e98ecc29e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…otlin refactorings Renaming a class no longer rewrites references bound through an import alias: `import a.Foo as Alias` updates the imported name while `Alias(...)` keeps its name, since only the alias binds the symbol. Change Package no longer adds an outgoing import when the moved file already spells out the former package (`old.source.Helper(1)`), which previously produced a conflicting import next to an existing same-named class import. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df3c5b2f0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Change Package no longer adds an unaliased import when the moved file already reaches the declaration through an import alias, which previously collided with a same-named class imported from another package. Rename now suppresses only references actually bound through an alias: a redundant `import p.Foo as Foo` no longer prevents qualified `p.Foo(1)` and `p.Foo` type references from following the rename. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e383873e1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A type reference spelled with its package (`a.Helper`) is wrapped in a KtUserType rather than a dot expression, so outgoing dependency capture added an unneeded `import a.Helper`. Package-qualified user types are now recognized alongside dot-qualified accesses. Incoming retargeting no longer requests an import for a usage it rewrites in place (`a.moved()` becomes `b.moved()`), which previously conflicted with an unrelated same-named import in a wildcard importer. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aad58a3f9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A caller that reaches a moved declaration only through `import a.moved as alias` needs no unaliased import: the rewritten alias directive keeps the binding. Requesting `import b.moved` made the caller's unrelated same-named import ambiguous, so the incoming pass now skips alias-bound usages the same way the outgoing pass already does. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3333f9ad5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…bound order - Preserved the nullability of a nullable type-parameter occurrence when checking operator arguments, so `<T : Host> getValue(thisRef: T?, ...)` still accepts a null thisRef. - Substituted the receiver's type arguments into member operator parameter types before the applicability check, so a `Grid<T>` member setter is no longer treated as fitting a `Grid<String>` write it cannot serve. - Compared constructor type-parameter bounds as an unordered set, so the same intersection written in a different order is reported as a conflicting overload. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba316cc48f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rewritten-import captures Reported by Codex review 5871698109: - selectExtension compared extension arguments with the unsubstituted type parameters of the declared receiver, so `operator fun <T> Grid<T>.set(index: T, value: Int)` looked applicable for a `Grid<String>` receiver and shadowed the moving `Base.set`. - returnTypeInferredFrom only unified the declared receiver, so a provider whose type parameter appears only in `thisRef` (`operator fun <T> Delegate.provideDelegate(thisRef: T, ...)`) kept returning `Provided<T>` and the moving accessor was never imported. - isClassifierBoundInScope consulted default imports and the caller's package for callers whose explicit import the rename rewrites, rejecting a safe rename to a name such as `String`. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c004d82af5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… rewritten value imports - KaOperatorApplicability: substituted the receiver-derived instantiations into the provider's parameter types, so an argument only has to be assignable to the instantiated parameter instead of contradicting the receiver (`Host : Base` with `Delegate<Base>` still infers `Provided<Base>`). - KaOperatorApplicability: a nullable type-parameter occurrence (`T?`) now checks the argument's non-nullable part against the bounds, so `var value: Host?` fits `operator fun <T : Host> Box.setValue(..., value: T?)`. - KotlinImportBindingCheck/KaRenameComputer: value positions (constructor calls) honour the rewritten explicit import priority, which outranks the file's declarations, wildcard imports and Kotlin's default imports. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abd8f99011
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…receiver ranking and reordered type parameters - Rejected non-operator provideDelegate methods so only operators decide the delegated accessor. - Bound the assigned value of an indexed write to the setter's final parameter and allowed omitted defaulted index parameters in setter applicability. - Compared extension receivers as instantiated for the actual receiver and ranked concrete receivers ahead of generic ones with fewer type parameters. - Derived the constructor/function type-parameter correspondence from the compared signature instead of the declaration positions. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdb0b7c58d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Inherited an operator modifier through overrides: a delegate provider override without its own modifier still decides the accessor, while an override of an ordinary method stays ineligible. Non-operator members no longer hide operator extensions of indexed writes, and overload ranking prefers the setter that omits no default parameters. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f655e97e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…overrides - Overload selection now compares parameter specificity before the type-parameter count and the omitted-default count, and compares it symmetrically: <T : Host> Box.getValue(T, ...) beats Box.getValue(Any?, ...), and set(i: Int, j: Int = 0, value) beats set(i: Number, j: Number) for `box[0]++`. - A generic parameter is compared by its upper bound, which is exactly what it accepts, and only the parameters the call binds take part. - Operator eligibility inherited through an override now requires the overridden declaration to have the same (instantiated) signature, so overriding an ordinary same-arity overload next to an operator one is no longer treated as an operator. - Added regression tests pinning the alias case: K2 reports a type alias's own classifier, so a wildcard-imported alias stays imported when its target package gains a same-named declaration. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9805f6b954
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…vider ranking - A `private` member `set` is no longer counted as hiding an extension at a caller outside its declaring class, so a compound indexed write that binds an extension keeps its import when that extension moves. - Member overload ranking now instantiates the parameter types with the declaring class's type arguments, so `Delegate<Local>` selects `provideDelegate(T, ...)` over `provideDelegate(Host, ...)` and the accessor of the selected provider is the one recorded. - Added regression tests for both shapes. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Separates the three rename refactorings and makes the generic one semantic:
.ktfileGeneric Rename is no longer offered for the
src/main/kotlinandsrc/test/kotlinsource roots; the explicit physical command still works there.The semantic pass runs on the original paths, stages minimal document hunks through
KotlinRefactoringTransaction, and lets a single commit write those documents before renaming the folder, so conflicts are reported before any physical change and Undo Last Refactoring restores paths, texts, and caret positions together.Why reference retargeting works on source text
The copied IDEA
K2ChangePackageRefactoringProcessoris still used for usage discovery, conflict checking, and package rewriting. Its upstreamretargetUsagesAfterMove()cannot run in standalone NetBeans: rewriting a package directive invalidates every PSI smart pointer, including those stored in K2UsageInfo, and the retargeter then fails withPsiInvalidElementAccessException(observed for bothKtImportListandKtProperty). IDEA survives this only because its documents are synchronised between phases.KaChangePackageComputertherefore captures source texts and replacement ranges before mutation: direct import directives plus qualified reference chains that K2 resolved to a moved declaration. Only those ranges change, so comments, string literals, declaration names, and receiver chains bound to unrelated members are never rewritten, while executable references inside string templates still follow the declaration. Callers that lose an implicit same-package binding — including callers relying on a wildcard import — receive an explicit import.Known limitation
Files that start in one package but end in distinct unrelated target packages within the same operation are not covered: K2 usage search does not report the cross-reference between them.
Testing
mvn clean test— 1238 tests, 0 failuresmvn clean package -DskipTests—.nbmbuiltAlso changed
KotlinPackageRenameMapping— path-derived target package calculation with identifier validation, plus unit testsRegistryValue.addListener(RegistryValueListener, Disposable)— binary-compatible no-op overload required by the bundled Kotlin code🤖 Generated with Claude Code