Add change variable type quick-fix code action - #8645
Merged
tgodzik merged 10 commits intoAug 7, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughAdds a compiler-backed ChangesChangeVariableType code action
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant CodeActionProvider
participant JavaPresentationCompiler
participant JavaChangeVariableTypeProvider
participant Javac
Client->>CodeActionProvider: request ChangeVariableType actions
CodeActionProvider->>JavaPresentationCompiler: request diagnostic-scoped edits
JavaPresentationCompiler->>JavaChangeVariableTypeProvider: pass diagnostic range
JavaChangeVariableTypeProvider->>Javac: infer initializer type
Javac-->>JavaChangeVariableTypeProvider: return inferred type
JavaChangeVariableTypeProvider-->>JavaPresentationCompiler: return replacement and import edits
JavaPresentationCompiler-->>CodeActionProvider: return code action
CodeActionProvider-->>Client: return ChangeVariableType action
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zielinsky
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala (1)
243-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication:
node.getType()computed twice.
typ(lines 243-249) andtypeRange(line 250) each independently callOption(node.getType()). Hoisting it into a singlevalavoids the repeated call/wrap.♻️ Proposed refactor
+ val typeTree = Option(node.getType()) treeRange(node).map { range => JavaVariable( tree = node, name = variableName, range = range, nameRange = findNameRange( lineMap, text, range.startOffset, range.endOffset, variableName, ).getOrElse(range), - typ = Option(node.getType()) match { + typ = typeTree match { case Some(t) => t.toString() case None => // This can happen if a variable is declared with inferred type. // This is going to change in Java 27, see https://bugs.openjdk.org/browse/JDK-8268850 "var" }, - typeRange = Option(node.getType()).flatMap(treeRange), + typeRange = typeTree.flatMap(treeRange), initializerRange = Option(node.getInitializer()).flatMap(treeRange),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala` around lines 243 - 251, The Java tree parsing logic computes node.getType() twice when building the result in JavaTrees, once for typ and again for typeRange. Hoist the type lookup into a single local val inside the parsing block, then derive both typ and typeRange from that shared value to avoid the repeated Option wrapping and keep the logic in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala`:
- Around line 103-106: SourceVisibility.from is using raw-text regex scans via
packageDeclaration and importDeclaration, which can mis-detect commented-out
package/import lines as real declarations. Update the visibility check to avoid
scanning unprocessed source text: either strip comments and string literals
before applying these regexes, or use the parsed compilation unit data from
javaTrees to derive package/import visibility. Ensure visibleName and renderType
only treat genuinely declared imports/packages as visible.
In `@metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala`:
- Around line 250-251: The variable range extraction in JavaTrees currently
treats legacy array declarators like regular types, causing
ChangeVariableType.isSingleDeclaration to reject declarations such as int arr[]
= ... because node.getType() includes the post-name brackets. Update the
handling around node.getType() in JavaTrees so legacy array declarators are
detected explicitly and either normalized or excluded before producing
typeRange, and make sure the downstream single-declaration check in
ChangeVariableType can still identify valid cases.
In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavacDiagnostic.scala`:
- Around line 44-65: The IncompatibleTypes extractor in JavacDiagnostic
currently matches only the localized English diagnostic text, so the “Change
variable type” action can break when javac wording or locale changes. Update
IncompatibleTypes.unapply to guard on the javac code as well, using the
diagnostic key compiler.err.prob.found.req similarly to CannotFindSymbol, and
keep the existing regex parsing as the fallback for extracting found and
required types.
---
Nitpick comments:
In `@metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala`:
- Around line 243-251: The Java tree parsing logic computes node.getType() twice
when building the result in JavaTrees, once for typ and again for typeRange.
Hoist the type lookup into a single local val inside the parsing block, then
derive both typ and typeRange from that shared value to avoid the repeated
Option wrapping and keep the logic in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 884fb923-2f33-48cc-8baf-93644f80160c
📒 Files selected for processing (5)
metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scalametals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scalametals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavacDiagnostic.scalatests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
3 times, most recently
from
July 8, 2026 23:03
72c5cf3 to
112af68
Compare
zielinsky
marked this pull request as ready for review
July 8, 2026 23:23
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
July 8, 2026 23:23
112af68 to
b7c4729
Compare
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
2 times, most recently
from
July 8, 2026 23:33
ae8f3a0 to
e7f3d3e
Compare
tgodzik
requested changes
Jul 9, 2026
tgodzik
left a comment
tgodzik
left a comment
Contributor
There was a problem hiding this comment.
One thing I am wondering is whether we shouldn't do it via the presentation compiler. Will the printed types here good enough? Could you add some more complex types? Maybe something that needs an import (var being given a method return value that is not available in the current scope) or maybe generic types?
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
July 9, 2026 22:22
c88f84d to
1ffab8e
Compare
Member
Author
|
@tgodzik We take the replacement type directly from javac's diagnostic <- params.getContext().getDiagnostics().asScala.toSeq
foundType <- changedType(diagnostic, variable, initializerRange).toSeq
replacement = renderType(foundType, sourceVisibility, legacyDimensions)
private def changedType(
diagnostic: l.Diagnostic,
variable: JavaVariable,
initializerRange: JavaRange,
): Option[String] =
for {
mismatch <- JavacDiagnostic.IncompatibleTypes.unapply(diagnostic)
if isRenderableType(mismatch.found)
if isInitializerMismatch(diagnostic, initializerRange)
if sameType(mismatch.required, variable.typ)
} yield mismatch.found |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala (1)
30-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
CodeActioninstead of using the fully qualified name.Line 6 imports
org.eclipse.lsp4j.Diagnostic. Line 30 uses the fully qualifiedorg.eclipse.lsp4j.CodeAction. Use one style.♻️ Proposed change
+import org.eclipse.lsp4j.CodeAction import org.eclipse.lsp4j.Diagnostic- private val onlyChangeType: org.eclipse.lsp4j.CodeAction => Boolean = + private val onlyChangeType: CodeAction => Boolean = _.getTitle() == ChangeVariableType.title🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala` around lines 30 - 31, Update the onlyChangeType declaration to use an imported org.eclipse.lsp4j.CodeAction type, matching the existing Diagnostic import style, and remove the fully qualified type reference.mtags-java/src/main/scala/scala/meta/internal/jpc/JavaPresentationCompiler.scala (1)
334-334: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm that the metals action declares
maybeCodeActionId.
CodeActionProvider.actionsForParamsfilters actions with_.maybeCodeActionId.forall(supportedCodeActions.contains). The newChangeVariableTypeaction inmetals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scaladoes not overridemaybeCodeActionId, so advertising the id here has no gating effect. OverridemaybeCodeActionIdin the action if you want the compiler capability to control availability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaPresentationCompiler.scala` at line 334, Update ChangeVariableType to override maybeCodeActionId with CodeActionId.ChangeVariableType, so CodeActionProvider.actionsForParams can gate it using supportedCodeActions. Keep the ChangeVariableType advertisement in JavaPresentationCompiler aligned with this declaration.mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala (2)
369-401: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm nested-class handling in
visibleName.
visibleNamesplits on the last dot. For a nested class such asa.Outer.Inner,packageNamebecomesa.Outer, so the import and package checks fail and the fully qualified name remains. That output still compiles, so this is not a defect. Add a test for a nested class initializer type if you want to lock the behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala` around lines 369 - 401, Add a regression test covering a nested-class initializer type such as a.Outer.Inner and assert that visibleName preserves the fully qualified name under the current SourceVisibility logic. Do not alter visibleName or its visibility checks, since the existing output remains compilable.
70-110: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a cancellation check before the second compilation.
inferInitializerTypetriggers a full second compile of the patched source.textEditschecks cancellation only before the firstnodeAtPositioncall.ChangeVariableType.contributesends one request per matching diagnostic, so a file with many incompatible-type diagnostics causes two compilations per diagnostic. A cancellation check before the second compile reduces wasted work.♻️ Proposed change
val adjustedParams = CompilerOffsetParams( params.uri(), adjustedText, adjustedOffset, params.token(), params.outlineFiles() ) + params.checkCanceled() compiler.nodeAtPosition(adjustedParams).flatMap {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala` around lines 70 - 110, In inferInitializerType, check the request cancellation state immediately before the second compiler.nodeAtPosition call on the patched source. Reuse the existing cancellation mechanism and abort without compiling when cancellation has been requested; leave the initializer type inference flow unchanged otherwise.metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala (1)
30-57: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFilter diagnostics by the cursor position before sending compiler requests.
Every matching diagnostic in the file produces one
compilers.codeActionrequest, and each request runs two compilations insideJavaChangeVariableTypeProvider. All requests use the same cursor position, sodiagnosticMatchesInitializerdiscards results for diagnostics that do not enclose that position. RestrictmatchingDiagnosticsto diagnostics whose range containspositionto avoid the extra work.♻️ Proposed change
val matchingDiagnostics = - params.getContext().getDiagnostics().asScala.toSeq.collect { - case diagnostic - if JavacDiagnostic.IncompatibleTypes - .unapply(diagnostic) - .isDefined => - diagnostic - } + params + .getContext() + .getDiagnostics() + .asScala + .toSeq + .filter(diagnostic => + JavacDiagnostic.IncompatibleTypes.unapply(diagnostic).isDefined + )Add a range containment check on
positionin the samefilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala` around lines 30 - 57, Update matchingDiagnostics in the ChangeVariableType action to retain only incompatible-type diagnostics whose range contains position, combining this check with the existing JavacDiagnostic.IncompatibleTypes filter before invoking compilers.codeAction. Keep the subsequent request and edit-building flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala`:
- Around line 211-227: The isSingleDeclaration method incorrectly uses raw text
scanning to detect comma-separated declarators, so comments can hide sibling
variables. In
mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala
lines 211-227, inspect sibling VariableTree nodes from the enclosing tree and
use that structure to determine whether the declaration is single, removing the
comma scan. In
tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
lines 221-234, add a checkActionsOnly case covering a comment before the comma,
such as int a = 1 /* c */, b = "test";.
---
Nitpick comments:
In
`@metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala`:
- Around line 30-57: Update matchingDiagnostics in the ChangeVariableType action
to retain only incompatible-type diagnostics whose range contains position,
combining this check with the existing JavacDiagnostic.IncompatibleTypes filter
before invoking compilers.codeAction. Keep the subsequent request and
edit-building flow unchanged.
In
`@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala`:
- Around line 369-401: Add a regression test covering a nested-class initializer
type such as a.Outer.Inner and assert that visibleName preserves the fully
qualified name under the current SourceVisibility logic. Do not alter
visibleName or its visibility checks, since the existing output remains
compilable.
- Around line 70-110: In inferInitializerType, check the request cancellation
state immediately before the second compiler.nodeAtPosition call on the patched
source. Reuse the existing cancellation mechanism and abort without compiling
when cancellation has been requested; leave the initializer type inference flow
unchanged otherwise.
In
`@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaPresentationCompiler.scala`:
- Line 334: Update ChangeVariableType to override maybeCodeActionId with
CodeActionId.ChangeVariableType, so CodeActionProvider.actionsForParams can gate
it using supportedCodeActions. Keep the ChangeVariableType advertisement in
JavaPresentationCompiler aligned with this declaration.
In
`@tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala`:
- Around line 30-31: Update the onlyChangeType declaration to use an imported
org.eclipse.lsp4j.CodeAction type, matching the existing Diagnostic import
style, and remove the fully qualified type reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a799aacc-f716-4481-8d9a-553faadb0717
📒 Files selected for processing (8)
metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scalametals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scalametals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scalamtags-interfaces/src/main/java/scala/meta/pc/CodeActionId.javamtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaPresentationCompiler.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavacDiagnostic.scalatests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
🚧 Files skipped from review as they are similar to previous changes (3)
- metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala
- mtags-java/src/main/scala/scala/meta/internal/jpc/JavacDiagnostic.scala
- metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala (1)
273-315: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLegacy-array-suffix trimming is duplicated across two files. Both files implement the identical algorithm (detect a
[]suffix after the variable name with the sameLegacyArrayDimensionsregex, then trim the type range back to the last non-whitespace character before the name) using near-identical code on different local range types. This duplication risks the two call sites (hover/navigation ranges versus the quick-fix edit range) silently diverging if a fix is applied in only one place.
metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala#L273-L315: extracttypeRange/onlyLegacyArrayDimensions/lastNonWhitespaceBeforeand theLegacyArrayDimensionsregex intoPositions(mirroring thefindNameOffsetextraction already done in this PR), operating on plain offsets/LineMap/textinstead of the file-localJavaRange.mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala#L134-L186: replacetrimLegacyArraySuffixandContext.lastNonWhitespaceBeforewith calls to the same sharedPositionsutility, removing the localLegacyArrayDimensionsregex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala` around lines 273 - 315, Extract the shared legacy-array-suffix detection and trimming logic into Positions, using plain offsets, LineMap, and text and following the existing findNameOffset extraction pattern. In metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala lines 273-315, replace typeRange, onlyLegacyArrayDimensions, lastNonWhitespaceBefore, and LegacyArrayDimensions with the shared utility while preserving JavaRange conversion. In mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala lines 134-186, replace trimLegacyArraySuffix and Context.lastNonWhitespaceBefore with Positions calls and remove the local regex.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala`:
- Around line 199-215: The isSingleDeclaration check incorrectly treats mixed
declarators as separate because it compares type-tree identity. Compare sibling
and current VariableTree type source ranges using the existing Context.startOf
helper, preserving the sibling exclusion, and add an LSP test covering int a,
b[]; to verify the quick-fix is blocked. Use Metals MCP tools to compile and run
the relevant tests.
---
Nitpick comments:
In `@metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala`:
- Around line 273-315: Extract the shared legacy-array-suffix detection and
trimming logic into Positions, using plain offsets, LineMap, and text and
following the existing findNameOffset extraction pattern. In
metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala lines 273-315,
replace typeRange, onlyLegacyArrayDimensions, lastNonWhitespaceBefore, and
LegacyArrayDimensions with the shared utility while preserving JavaRange
conversion. In
mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala
lines 134-186, replace trimLegacyArraySuffix and Context.lastNonWhitespaceBefore
with Positions calls and remove the local regex.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e84d799f-c85c-4734-8cdf-98ec632f72fc
📒 Files selected for processing (7)
metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scalametals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaImplementAbstractMembersProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaTypeShortener.scalamtags-java/src/main/scala/scala/meta/internal/jpc/Positions.scalatests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
🚧 Files skipped from review as they are similar to previous changes (2)
- metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala
- tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 3, 2026 12:08
228f0cf to
7d9a747
Compare
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 3, 2026 12:10
7d9a747 to
f6855fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaTypeShortener.scala`:
- Around line 132-160: Update JavaTypeShortener.forPath to collect packages from
non-static wildcard imports and pass them into JavaTypeShortener, then update
shortenTopLevel to treat types from those packages as already accessible before
creating explicit imports. Preserve existing handling for java.lang, the current
package, claimed names, and explicit imports, and validate the change with the
requested wildcard-import scenarios.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bbe719c1-3ca5-47fc-9939-47a20e09463f
📒 Files selected for processing (7)
metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scalametals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaImplementAbstractMembersProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaTypeShortener.scalamtags-java/src/main/scala/scala/meta/internal/jpc/Positions.scalatests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
🚧 Files skipped from review as they are similar to previous changes (4)
- mtags-java/src/main/scala/scala/meta/internal/jpc/Positions.scala
- metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala
- metals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scala
- tests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 4, 2026 08:01
4a6d7ac to
aa3973b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scala`:
- Around line 195-201: Update renderInferredType to return None immediately when
initializerType.getKind is TypeKind.INTERSECTION, before calling
shortener.shorten or creating the rendered type. Preserve the existing
annotation removal and isRenderableType validation for all other initializer
types.
In `@mtags-java/src/main/scala/scala/meta/internal/jpc/JavaTypeShortener.scala`:
- Around line 92-96: Update shortenTopLevel’s wildcard-import branch so it does
not shorten a type when multiple wildcard imports can resolve the same
simpleName, including conflicts with enclosing-type members; retain the
qualified name or use compiler scope/import resolution to create an explicit
import, and only claim simpleName when resolution is unambiguous.
- Around line 162-186: The current implementation only collects member type
names from the immediate enclosing class via the single call to
enclosingClass(path).map(collectMemberTypeNames), which misses type declarations
from outer enclosing scopes. Modify the collection logic to recursively traverse
all enclosing classes up the hierarchy rather than stopping at the first one,
gathering member types from every ClassTree in the parent chain until reaching
the root. Keep the overall flow of combining topLevelTypeNames with the
collected names but ensure memberTypeNames now includes contributions from all
nesting levels.
In `@mtags-java/src/main/scala/scala/meta/internal/jpc/Positions.scala`:
- Around line 21-28: Update findNameOffset to resolve declarator names using
Java token boundaries or the compiler-provided name position instead of scanning
raw text for the first identifier match. Ensure matches inside comments and
literals are skipped, preserving the correct offset for JavaTrees and
trimLegacyArraySuffix.
- Around line 40-44: Update the hasLegacyArraySuffix detection in
Positions.scala so it identifies legacy array dimensions using compiler or token
positions rather than matching the raw substring between nameEnd and typeEnd.
Allow comments and annotations between the variable name and dimensions, and
preserve the full javac type range so the resulting edit never includes or
deletes the variable name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b2381b2-7a2e-40b1-9e81-f4887a102869
📒 Files selected for processing (7)
metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scalametals/src/main/scala/scala/meta/internal/parsing/JavaTrees.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaChangeVariableTypeProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaImplementAbstractMembersProvider.scalamtags-java/src/main/scala/scala/meta/internal/jpc/JavaTypeShortener.scalamtags-java/src/main/scala/scala/meta/internal/jpc/Positions.scalatests/unit/src/test/scala/tests/codeactions/ChangeVariableTypeLspSuite.scala
🚧 Files skipped from review as they are similar to previous changes (2)
- mtags-java/src/main/scala/scala/meta/internal/jpc/JavaImplementAbstractMembersProvider.scala
- metals/src/main/scala/scala/meta/internal/metals/codeactions/ChangeVariableType.scala
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 4, 2026 09:57
aa3973b to
565de14
Compare
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 4, 2026 11:40
f2ad7a4 to
f366630
Compare
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
2 times, most recently
from
August 4, 2026 13:42
f6b555a to
54c17bd
Compare
zielinsky
force-pushed
the
metalsv2/change-variable-type-quick-fix
branch
from
August 5, 2026 08:27
54c17bd to
d9cc97f
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 similar comment
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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
2 participants
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.
Part of #8502
Summary by CodeRabbit
New Features
Bug Fixes
Tests