Skip to content

Commit

Permalink
Fix removal of this cleanup to avoid forward field refs in lambdas (#…
Browse files Browse the repository at this point in the history
…1212)

- fixes #1211
- add check for a field reference in a lambda in a field initializer
  in which case, skip
- add new test to CleanUpTest1d8
  • Loading branch information
jjohnstn authored Mar 9, 2024
1 parent 678d43e commit bef8bb1
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2019, 2023 IBM Corporation and others.
* Copyright (c) 2019, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -31,16 +31,19 @@
import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
import org.eclipse.jdt.core.dom.Assignment;
import org.eclipse.jdt.core.dom.Block;
import org.eclipse.jdt.core.dom.BodyDeclaration;
import org.eclipse.jdt.core.dom.ClassInstanceCreation;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.Expression;
import org.eclipse.jdt.core.dom.ExpressionStatement;
import org.eclipse.jdt.core.dom.FieldAccess;
import org.eclipse.jdt.core.dom.FieldDeclaration;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.Initializer;
import org.eclipse.jdt.core.dom.LambdaExpression;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.Name;
Expand Down Expand Up @@ -266,6 +269,12 @@ public boolean visit(final FieldAccess node) {
if (hasConflict(expression.getStartPosition(), name, ScopeAnalyzer.VARIABLES | ScopeAnalyzer.CHECK_VISIBILITY))
return true;

ASTNode ancestor= ASTNodes.getFirstAncestorOrNull(node, LambdaExpression.class, BodyDeclaration.class);
if (ancestor instanceof LambdaExpression lambdaAncestor) {
if (ASTNodes.getFirstAncestorOrNull(lambdaAncestor, FieldDeclaration.class) != null) {
return false;
}
}
Name qualifier= ((ThisExpression) expression).getQualifier();
if (qualifier != null) {
ITypeBinding outerClass= (ITypeBinding) qualifier.resolveBinding();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6481,4 +6481,79 @@ public void testDoNotDoDeprecatedCleanup4() throws Exception { // https://github
assertRefactoringHasNoChange(new ICompilationUnit[] { cu1, cu2 });
}

@Test
public void testRemoveThisIssue1211() throws Exception { // https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1211
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);

String sample= "" //
+ "package test1;\n" //
+ "\n" //
+ "public class A {\n" //
+ " public interface PropertyChangeListener {\n" //
+ " void propertyChange(Object evt);\n" //
+ "\n" //
+ " }\n" //
+ "\n" //
+ " private final PropertyChangeListener listener = evt -> {\n" //
+ " this.clientCache.get();\n" //
+ " };\n" //
+ "\n" //
+ " public void x() {\n" //
+ " PropertyChangeListener listener = evt -> {\n" //
+ " this.clientCache.get();\n" //
+ " };\n" //
+ " listener.propertyChange(listener);\n" //
+ " }\n" //
+ " interface Cache<V> {\n" //
+ " V get();\n" //
+ " }\n" //
+ "\n" //
+ " final Cache<String> clientCache = new Cache<>() {\n" //
+ " @Override\n" //
+ " public String get() {\n" //
+ " listener.propertyChange(null);\n" //
+ " return \"\";\n" //
+ " }\n" //
+ " };\n" //
+ "}\n";
ICompilationUnit cu1= pack1.createCompilationUnit("A.java", sample, false, null);

enable(CleanUpConstants.MEMBER_ACCESSES_NON_STATIC_FIELD_USE_THIS);
enable(CleanUpConstants.MEMBER_ACCESSES_NON_STATIC_FIELD_USE_THIS_IF_NECESSARY);

sample= "" //
+ "package test1;\n" //
+ "\n" //
+ "public class A {\n" //
+ " public interface PropertyChangeListener {\n" //
+ " void propertyChange(Object evt);\n" //
+ "\n" //
+ " }\n" //
+ "\n" //
+ " private final PropertyChangeListener listener = evt -> {\n" //
+ " this.clientCache.get();\n" //
+ " };\n" //
+ "\n" //
+ " public void x() {\n" //
+ " PropertyChangeListener listener = evt -> {\n" //
+ " clientCache.get();\n" //
+ " };\n" //
+ " listener.propertyChange(listener);\n" //
+ " }\n" //
+ " interface Cache<V> {\n" //
+ " V get();\n" //
+ " }\n" //
+ "\n" //
+ " final Cache<String> clientCache = new Cache<>() {\n" //
+ " @Override\n" //
+ " public String get() {\n" //
+ " listener.propertyChange(null);\n" //
+ " return \"\";\n" //
+ " }\n" //
+ " };\n" //
+ "}\n";
String expected1= sample;
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 }, null);
}

}

0 comments on commit bef8bb1

Please sign in to comment.