Skip to content

Better handling of local class when creating local variable-like elements. #9041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lahodaj wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better handling of local class when creating local variable-like elements. #9041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion java/java.hints/src/org/netbeans/modules/java/hints/errors/ChangeTypeFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
11 changes: 7 additions & 4 deletions java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElementUtilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions java/java.hints/src/org/netbeans/modules/java/hints/errors/Utilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 1 addition & 6 deletions java/java.hints/src/org/netbeans/modules/java/hints/introduce/InstanceRefFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -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>();
}
Expand Down
2 changes: 1 addition & 1 deletion ...ints/src/org/netbeans/modules/java/hints/introduce/IntroduceExpressionBasedMethodFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceFieldFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down
13 changes: 10 additions & 3 deletions java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceHint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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>();
Expand All @@ -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);
Expand Down Expand Up @@ -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>();
Expand Down
2 changes: 1 addition & 1 deletion java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceVariableFix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...
}
Expand Down
96 changes: 96 additions & 0 deletions ...ints/test/unit/src/org/netbeans/modules/java/hints/errors/AddParameterOrLocalFixTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions java/java.hints/test/unit/src/org/netbeans/modules/java/hints/errors/ChangeTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading