Merge pull request #9041 from lahodaj/java-hints-local-class

lahodaj · web-flow · commit 2602169290b9 · 2026-02-19T20:20:23.000+01:00
Better handling of local class when creating local variable-like elements.
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeType.java b/java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeType.java
@@ -168,7 +168,7 @@ public List<Fix> run(CompilationInfo info,
                     return null;
                 }
                 //anonymous class?
-                expressionType[0] = org.netbeans.modules.java.hints.errors.Utilities.convertIfAnonymous(expressionType[0]);
+                expressionType[0] = org.netbeans.modules.java.hints.errors.Utilities.convertIfAnonymous(expressionType[0], true);
 
                 result.add(new ChangeTypeFix(info, treePath,
                         ((VariableTree) leaf[0]).getName().toString(), 
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeTypeFix.java b/java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeTypeFix.java
@@ -61,7 +61,7 @@ protected void performRewrite(TransformationContext ctx) throws Exception {
         ChangeType.computeType(working, position, tm, expressionType, leaf);
 
         //anonymous class?
-        expressionType[0] = Utilities.convertIfAnonymous(expressionType[0]);
+        expressionType[0] = Utilities.convertIfAnonymous(expressionType[0], true);
 
         if (leaf[0] instanceof VariableTree) {
             VariableTree oldVariableTree = ((VariableTree)leaf[0]);
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElement.java b/java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElement.java
@@ -358,6 +358,7 @@ private static List<Fix> analyzeImpl(CompilationInfo info, String diagnosticKey,
         if (!ErrorFixesFakeHint.enabled(info.getFileObject(), FixKind.CREATE_LOCAL_VARIABLE)) {
             fixTypes.remove(ElementKind.LOCAL_VARIABLE);
         }
+        TypeMirror localType;
         final TypeMirror type;
         
         if (types != null && !types.isEmpty()) {
@@ -376,13 +377,15 @@ private static List<Fix> analyzeImpl(CompilationInfo info, String diagnosticKey,
                 i++;
             }
             //XXX: should reasonably consider all the found type candidates, not only the one:
-            type = types.get(0);
+            localType = Utilities.convertIfAnonymous(types.get(0), true);
+            type = Utilities.convertIfAnonymous(types.get(0), false);
             
             if (superType[0] == null) {
                 // the type must be already un-captured.
                 superType[0] = type;
             }
         } else {
+            localType = null;
             type = null;
         }
 
@@ -500,11 +503,11 @@ private static List<Fix> analyzeImpl(CompilationInfo info, String diagnosticKey,
             if (ee != null && fixTypes.contains(ElementKind.PARAMETER) && !Utilities.isMethodHeaderInsideGuardedBlock(info, (MethodTree) firstMethod.getLeaf()))
                 result.add(new AddParameterOrLocalFix(info, type, simpleName, ElementKind.PARAMETER, identifierPos).toEditorFix());
             if ((firstMethod != null || firstInitializer != null || firstLambda != null) && fixTypes.contains(ElementKind.LOCAL_VARIABLE) && ErrorFixesFakeHint.enabled(ErrorFixesFakeHint.FixKind.CREATE_LOCAL_VARIABLE))
-                result.add(new AddParameterOrLocalFix(info, type, simpleName, ElementKind.LOCAL_VARIABLE, identifierPos).toEditorFix());
+                result.add(new AddParameterOrLocalFix(info, localType, simpleName, ElementKind.LOCAL_VARIABLE, identifierPos).toEditorFix());
             if (fixTypes.contains(ElementKind.RESOURCE_VARIABLE))
-                result.add(new AddParameterOrLocalFix(info, type, simpleName, ElementKind.RESOURCE_VARIABLE, identifierPos).toEditorFix());
+                result.add(new AddParameterOrLocalFix(info, localType, simpleName, ElementKind.RESOURCE_VARIABLE, identifierPos).toEditorFix());
             if (fixTypes.contains(ElementKind.OTHER))
-                result.add(new AddParameterOrLocalFix(info, type, simpleName, ElementKind.OTHER, identifierPos).toEditorFix());
+                result.add(new AddParameterOrLocalFix(info, localType, simpleName, ElementKind.OTHER, identifierPos).toEditorFix());
         }
 
         return result;
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElementUtilities.java b/java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElementUtilities.java
@@ -390,9 +390,6 @@ private static List<? extends TypeMirror> computeAssignment(Set<ElementKind> typ
             type = info.getTrees().getTypeMirror(new TreePath(parent, at.getExpression()));
 
             if (type != null) {
-                //anonymous class?
-                type = org.netbeans.modules.java.hints.errors.Utilities.convertIfAnonymous(type);
-
                 if (type.getKind() == TypeKind.EXECUTABLE) {
                     //TODO: does not actualy work, attempt to solve situations like:
                     //t = Collections.emptyList()
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/errors/Utilities.java b/java/java.hints/src/org/netbeans/modules/java/hints/errors/Utilities.java
@@ -656,13 +656,13 @@ public static <T extends Tree> T copyComments(WorkingCopy wc, Tree from, T to, b
      * 
      * @return typemirror of supertype/iface, initial tm if not anonymous
      */
-    public static TypeMirror convertIfAnonymous(TypeMirror tm) {
+    public static TypeMirror convertIfAnonymous(TypeMirror tm, boolean keepLocal) {
         //anonymous class?
         Set<ElementKind> fm = EnumSet.of(ElementKind.METHOD, ElementKind.FIELD);
         if (tm instanceof DeclaredType) {
             Element el = ((DeclaredType) tm).asElement();
             //XXX: the null check is needed for lambda type, not covered by test:
-            if (el != null && (el.getSimpleName().length() == 0 || fm.contains(el.getEnclosingElement().getKind()))) {
+            if (el != null && (el.getSimpleName().length() == 0 || (!keepLocal && fm.contains(el.getEnclosingElement().getKind())))) {
                 List<? extends TypeMirror> interfaces = ((TypeElement) el).getInterfaces();
                 if (interfaces.isEmpty()) {
                     tm = ((TypeElement) el).getSuperclass();
@@ -1156,7 +1156,7 @@ public static MethodArguments resolveArguments(CompilationInfo info, TreePath in
             TypeMirror tm = info.getTrees().getTypeMirror(argPath);
 
             //anonymous class?
-            tm = Utilities.convertIfAnonymous(tm);
+            tm = Utilities.convertIfAnonymous(tm, false);
 
             if (tm == null || tm.getKind() == TypeKind.NONE || containsErrorsRecursively(tm)) {
                 return null;
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/InstanceRefFinder.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/InstanceRefFinder.java
@@ -67,7 +67,6 @@ public class InstanceRefFinder extends ErrorAwareTreePathScanner {
      * Immediate enclosing element
      */
     private Element                 enclosingElement;
-    private TreePath                enclosingElementPath;
     private TypeElement             enclosingType;
     
     private Set<TypeElement>        superReferences = Collections.<TypeElement>emptySet();
@@ -135,8 +134,7 @@ private void findEnclosingElement() {
                 case INTERFACE:
                 case RECORD:
                 case NEW_CLASS:
-                    enclosingElementPath = path;
-                    enclosingElement = ci.getTrees().getElement(enclosingElementPath);
+                    enclosingElement = ci.getTrees().getElement(path);
                     if (enclosingElement == null) {
                         return;
                     }
@@ -152,9 +150,6 @@ private void findEnclosingElement() {
     }
     
     private void addLocalReference(TypeElement el) {
-        if (this.enclosingElement != el.getEnclosingElement()) {
-            return;
-        }
         if (localReferences.isEmpty()) {
             localReferences = new HashSet<TypeElement>();
         }
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceExpressionBasedMethodFix.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceExpressionBasedMethodFix.java
@@ -233,7 +233,7 @@ public void run(ResultIterator resultIterator) throws Exception {
                 if (expression == null || returnType == null) {
                     return; //TODO...
                 }
-                returnType = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(copy, returnType));
+                returnType = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(copy, returnType), false);
                 final TreeMaker make = copy.getTreeMaker();
                 Tree returnTypeTree = make.Type(returnType);
                 copy.tag(returnTypeTree, TYPE_TAG);
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceFieldFix.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceFieldFix.java
@@ -284,7 +284,7 @@ public void run(ResultIterator resultIterator) throws Exception {
             if (tm == null) {
                 return; //TODO...
             }
-            tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm));
+            tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm), false);
             TreePath pathToClass = findTargetClass(parameter, resolved);
             if (pathToClass == null) {
                 return; //TODO...
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceHint.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceHint.java
@@ -236,6 +236,7 @@ private static void addExpressionFixes(CompilationInfo info, int start, int end,
         TreePath method   = TreeUtils.findMethod(resolved);
         boolean variableRewrite = resolved.getLeaf().getKind() == Kind.VARIABLE;
         TreePath value = !variableRewrite ? resolved : new TreePath(resolved, ((VariableTree) resolved.getLeaf()).getInitializer());
+        boolean hasNonLocalRefs = hasNonLocalRefs(info, value);
         boolean isVariable = TreeUtils.findStatement(resolved) != null && method != null && !variableRewrite;
         Set<TreePath> duplicatesForVariable = isVariable ? SourceUtils.computeDuplicates(info, resolved, method, cancel) : null;
         Set<TreePath> duplicatesForConstant = /*isConstant ? */SourceUtils.computeDuplicates(info, resolved, new TreePath(info.getCompilationUnit()), cancel);// : null;
@@ -252,7 +253,7 @@ private static void addExpressionFixes(CompilationInfo info, int start, int end,
         Fix constant = IntroduceConstantFix.createConstant(resolved, info, value, guessedName, duplicatesForConstant.size() + 1, end, variableRewrite, cancel);
 
 
-        Fix parameter = isVariable ? new IntroduceParameterFix(h) : null;
+        Fix parameter = isVariable && !hasNonLocalRefs ? new IntroduceParameterFix(h) : null;
         IntroduceFieldFix field = null;
         IntroduceFixBase methodFix = null;
 
@@ -320,7 +321,7 @@ private static void addExpressionFixes(CompilationInfo info, int start, int end,
                     }
                 }
 
-                if (!error186980) {
+                if (!error186980 && !hasNonLocalRefs) {
                     Set<TypeMirror> exceptions = new HashSet<TypeMirror>(info.getTreeUtilities().getUncaughtExceptions(resolved));
 
                     Set<TypeMirrorHandle> exceptionHandles = new HashSet<TypeMirrorHandle>();
@@ -341,7 +342,7 @@ private static void addExpressionFixes(CompilationInfo info, int start, int end,
                     if (viableTargets != null && !viableTargets.isEmpty()) {
                         TypeMirror returnType = 
                                 Utilities.convertIfAnonymous(Utilities.resolveCapturedType(info, 
-                                        resolveType(info, resolved)));
+                                        resolveType(info, resolved)), false);
                         if (Utilities.isValidType(returnType)) {
                             methodFix = new IntroduceExpressionBasedMethodFix(info.getSnapshot().getSource(), h, params, TypeMirrorHandle.create(returnType),
                                     exceptionHandles, duplicatesCount, typeVars, end, viableTargets);
@@ -380,6 +381,12 @@ private static void addExpressionFixes(CompilationInfo info, int start, int end,
         } 
     }
 
+    private static boolean hasNonLocalRefs(final CompilationInfo info, TreePath path) {
+        InstanceRefFinder finder = new InstanceRefFinder(info, path);
+        finder.process();
+        return finder.containsLocalReferences();
+    }
+
     public static List<ErrorDescription> computeError(CompilationInfo info, int start, int end, Map<IntroduceKind, Fix> fixesMap, Map<IntroduceKind, String> errorMessage, AtomicBoolean cancel) {
         List<ErrorDescription> hints = new LinkedList<ErrorDescription>();
         List<Fix> fixes = new LinkedList<Fix>();
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceVariableFix.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceVariableFix.java
@@ -169,7 +169,7 @@ public void run(ResultIterator resultIterator) throws Exception {
                 if (tm == null) {
                     return; //TODO...
                 }
-                tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm));
+                tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm), true);
                 if (!Utilities.isValidType(tm)) {
                     return; // TODO...
                 }
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/errors/AddParameterOrLocalFixTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/errors/AddParameterOrLocalFixTest.java
@@ -666,6 +666,102 @@ private void tt(Function<String,  List<Number>> p) {}
                 """.replaceAll("[ \t\n]+", " "));
     }
 
+    public void testLocalClass1() throws Exception {
+        sourceLevel = "17";
+        performAnalysisTest("test/Test.java",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        record R(int i) {}
+                        r|r = new R(0);
+                    }
+                }
+                """,
+                "AddParameterOrLocalFix:rr:java.lang.Record:PARAMETER",
+                "AddParameterOrLocalFix:rr:R:LOCAL_VARIABLE");
+    }
+
+    public void testLocalClass2() throws Exception {
+        sourceLevel = "17";
+        performFixTest("test/Test.java",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        record R(int i) {}
+                        r|r = new R(0);
+                    }
+                }
+                """,
+                "AddParameterOrLocalFix:rr:R:LOCAL_VARIABLE",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        record R(int i) {}
+                        R rr = new R(0);
+                    }
+                }
+                """.replaceAll("[ \t\n]+", " "));
+                
+    }
+
+    public void testTWRLocalClass() throws Exception {
+        sourceLevel = "17";
+        performFixTest("test/Test.java",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        class Impl implements AutoCloseable {
+                            public void close() {}
+                        }
+                        try (|impl = new Impl()) {
+                        }
+                    }
+                }
+                """,
+                "AddParameterOrLocalFix:impl:Impl:RESOURCE_VARIABLE",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        class Impl implements AutoCloseable {
+                            public void close() {}
+                        }
+                        try (Impl impl = new Impl()) {
+                        }
+                    }
+                }
+                """.replaceAll("[ \t\n]+", " "));
+    }
+
+    public void testLocalClassForInit() throws Exception {
+        sourceLevel = "17";
+        performFixTest("test/Test.java",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        record R(int i) {}
+                        for (r|r = new R(0); ;) {}
+                    }
+                }
+                """,
+                "AddParameterOrLocalFix:rr:R:OTHER",
+                """
+                package test;
+                public class Test {
+                    private void t() {
+                        record R(int i) {}
+                        for (R rr = new R(0); ;) {}
+                    }
+                }
+                """.replaceAll("[ \t\n]+", " "));
+                
+    }
+
     @Override
     protected List<Fix> computeFixes(CompilationInfo info, String diagnosticCode, int pos, TreePath path) throws Exception {
         List<Fix> fixes = super.computeFixes(info, diagnosticCode, pos, path);
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/errors/ChangeTypeTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/errors/ChangeTypeTest.java
@@ -199,6 +199,31 @@ public void test235716FixType() throws Exception {
              "}").replaceAll("\\s+", " "));
     }
 
+    public void testLocalClass() throws Exception {
+        sourceLevel = "17";
+        performFixTest("test/Test.java",
+            """
+            package test;
+            public class Test {
+                private void test() {
+                    record R(int i) {}
+                    String str = new R(0);
+                }
+            }
+            """,
+            -1,
+            "Change type of str to R",
+            """
+            package test;
+            public class Test {
+                private void test() {
+                    record R(int i) {}
+                    R str = new R(0);
+                }
+            }
+            """.replaceAll("\\s+", " "));
+    }
+
 
     protected List<Fix> computeFixes(CompilationInfo info, int pos, TreePath path) {
         return new ChangeType().run(info, null, pos, path, null);
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/IntroduceHintTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/IntroduceHintTest.java

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ public List<Fix> run(CompilationInfo info,`
`168`	`168`	`return null;`
`169`	`169`	`}`
`170`	`170`	`//anonymous class?`
`171`		`- expressionType[0] = org.netbeans.modules.java.hints.errors.Utilities.convertIfAnonymous(expressionType[0]);`
	`171`	`+ expressionType[0] = org.netbeans.modules.java.hints.errors.Utilities.convertIfAnonymous(expressionType[0], true);`
`172`	`172`
`173`	`173`	`result.add(new ChangeTypeFix(info, treePath,`
`174`	`174`	`((VariableTree) leaf[0]).getName().toString(),`
Original file line number	Diff line number	Diff line change
`@@ -233,7 +233,7 @@ public void run(ResultIterator resultIterator) throws Exception {`
`233`	`233`	`if (expression == null \|\| returnType == null) {`
`234`	`234`	`return; //TODO...`
`235`	`235`	`}`
`236`		`- returnType = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(copy, returnType));`
	`236`	`+ returnType = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(copy, returnType), false);`
`237`	`237`	`final TreeMaker make = copy.getTreeMaker();`
`238`	`238`	`Tree returnTypeTree = make.Type(returnType);`
`239`	`239`	`copy.tag(returnTypeTree, TYPE_TAG);`
Original file line number	Diff line number	Diff line change
`@@ -284,7 +284,7 @@ public void run(ResultIterator resultIterator) throws Exception {`
`284`	`284`	`if (tm == null) {`
`285`	`285`	`return; //TODO...`
`286`	`286`	`}`
`287`		`- tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm));`
	`287`	`+ tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm), false);`
`288`	`288`	`TreePath pathToClass = findTargetClass(parameter, resolved);`
`289`	`289`	`if (pathToClass == null) {`
`290`	`290`	`return; //TODO...`
Original file line number	Diff line number	Diff line change
`@@ -169,7 +169,7 @@ public void run(ResultIterator resultIterator) throws Exception {`
`169`	`169`	`if (tm == null) {`
`170`	`170`	`return; //TODO...`
`171`	`171`	`}`
`172`		`- tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm));`
	`172`	`+ tm = Utilities.convertIfAnonymous(Utilities.resolveTypeForDeclaration(parameter, tm), true);`
`173`	`173`	`if (!Utilities.isValidType(tm)) {`
`174`	`174`	`return; // TODO...`
`175`	`175`	`}`