From d90427e5e7bb13adbadf8550646991ee1372865b Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Wed, 26 Nov 2025 17:49:39 +0100 Subject: [PATCH] Better handling of local class when creating local variable-like elements. --- .../modules/java/hints/errors/ChangeType.java | 2 +- .../java/hints/errors/ChangeTypeFix.java | 2 +- .../java/hints/errors/CreateElement.java | 11 ++- .../hints/errors/CreateElementUtilities.java | 3 - .../modules/java/hints/errors/Utilities.java | 6 +- .../hints/introduce/InstanceRefFinder.java | 7 +- .../IntroduceExpressionBasedMethodFix.java | 2 +- .../hints/introduce/IntroduceFieldFix.java | 2 +- .../java/hints/introduce/IntroduceHint.java | 13 ++- .../hints/introduce/IntroduceVariableFix.java | 2 +- .../errors/AddParameterOrLocalFixTest.java | 96 +++++++++++++++++++ .../java/hints/errors/ChangeTypeTest.java | 25 +++++ .../hints/introduce/IntroduceHintTest.java | 81 ++++++++++++++++ 13 files changed, 228 insertions(+), 24 deletions(-) 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 index a3914ea80639..d8220827e98a 100644 --- 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 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 index 7dd4d1859f7d..80b71403a254 100644 --- 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 index e34e0dde2036..e65ea33e9d8d 100644 --- 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 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 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 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 index f5b1e264a509..552333795666 100644 --- 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 computeAssignment(Set 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 index 36740a5ee2c0..57d1d9ab8c52 100644 --- 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 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 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 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 index 56973bbb47c9..c7e5fb056c2e 100644 --- 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 superReferences = Collections.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(); } 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 index 8fabf3426802..8cd17dce6812 100644 --- 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 index 16abc0748441..0580bda5523e 100644 --- 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 index cba45fe10aef..c5c2d032cd80 100644 --- 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 duplicatesForVariable = isVariable ? SourceUtils.computeDuplicates(info, resolved, method, cancel) : null; Set 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 exceptions = new HashSet(info.getTreeUtilities().getUncaughtExceptions(resolved)); Set exceptionHandles = new HashSet(); @@ -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 computeError(CompilationInfo info, int start, int end, Map fixesMap, Map errorMessage, AtomicBoolean cancel) { List hints = new LinkedList(); List fixes = new LinkedList(); 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 index 01245cef5475..22b56347216b 100644 --- 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 index 44ea3ea45d15..09725332988a 100644 --- 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> 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 computeFixes(CompilationInfo info, String diagnosticCode, int pos, TreePath path) throws Exception { List 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 index 0cba909aa3f3..8f326eeb2a5a 100644 --- 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 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 index 62daba64f035..9c5515711a53 100644 --- 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 @@ -2618,6 +2618,87 @@ public void testIntroduceMethodReturn233433() throws Exception { 1, 0); } + public void testLocalType1() throws Exception { + sourceLevel = "17"; + performCheckFixesTest( + """ + package test; + public class Test { + public static void test() { + record R(int i) {} + |new R(0)|; + } + } + """, + "[IntroduceFix:r:1:CREATE_VARIABLE]", + "[IntroduceField:r:1:true:false:[3, 3]]"); + } + + public void testLocalType2() throws Exception { + sourceLevel = "17"; + performCheckFixesTest( + """ + package test; + public class Test { + public static void test() { + record R(int i) {} + R r = |new R(0)|; + } + } + """, + "[IntroduceFix:r1:1:CREATE_VARIABLE]", + "[IntroduceField:r:1:true:false:[3, 3]]"); + } + + public void testIntroduceVariableLocalType() throws Exception { + sourceLevel = "17"; + performFixTest(""" + package test; + public class Test { + public static void test() { + record R(int i) {} + |new R(0)|; + } + } + """, + """ + package test; + public class Test { + public static void test() { + record R(int i) {} + R name = new R(0); + } + } + """.replaceAll("[ \t\n]+", " "), + new DialogDisplayerImpl("name", false, false, true), + 2, 0); + } + + public void testIntroduceFieldLocalType() throws Exception { + sourceLevel = "17"; + performFixTest(""" + package test; + public class Test { + public static void test() { + record R(int i) {} + |new R(0)|; + } + } + """, + """ + package test; + public class Test { + private static Record name; + public static void test() { + record R(int i) {} + name = new R(0); + } + } + """.replaceAll("[ \t\n]+", " "), + new DialogDisplayerImpl("name", false, false, true), + 2, 1); + } + protected void prepareTest(String code) throws Exception { clearWorkDir();