Bug 560018 - Multiple syntax errors lambda cleanup on Platform UI repo

- add additional check to LambdaExpressionsFixCore to look if the
  anonymous class is used as a field initializer and the anonymous
  class attempts to access a final field within
- add tests to AssistQuickFixTest18 and CleanUpTest18

Change-Id: I0ad55d0ec309b391e1e6c2bd94cd099b35f7b4a8

3 files changed, 265 insertions, 27 deletions

diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFixCore.java
index 98d3ab05ba..62cdb5d07a 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFixCore.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFixCore.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2019 IBM Corporation and others.
+ * Copyright (c) 2013, 2020 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -39,10 +39,13 @@ 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.IAnnotationBinding;
 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.LambdaExpression;
 import org.eclipse.jdt.core.dom.MarkerAnnotation;
 import org.eclipse.jdt.core.dom.MethodDeclaration;
@@ -50,6 +53,7 @@ import org.eclipse.jdt.core.dom.MethodInvocation;
 import org.eclipse.jdt.core.dom.Modifier;
 import org.eclipse.jdt.core.dom.Name;
 import org.eclipse.jdt.core.dom.NormalAnnotation;
+import org.eclipse.jdt.core.dom.QualifiedName;
 import org.eclipse.jdt.core.dom.ReturnStatement;
 import org.eclipse.jdt.core.dom.SimpleName;
 import org.eclipse.jdt.core.dom.SingleMemberAnnotation;
@@ -66,6 +70,7 @@ import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
 import org.eclipse.jdt.core.dom.rewrite.ImportRewrite.ImportRewriteContext;
 import org.eclipse.jdt.core.dom.rewrite.ImportRewrite.TypeLocation;
 import org.eclipse.jdt.core.manipulation.ICleanUpFixCore;
+import org.eclipse.jdt.core.util.IModifierConstants;
 
 import org.eclipse.jdt.internal.core.manipulation.dom.ASTResolving;
 import org.eclipse.jdt.internal.corext.codemanipulation.CodeGenerationSettings;
@@ -200,6 +205,109 @@ public class LambdaExpressionsFixCore extends CompilationUnitRewriteOperationsFi
 		}
 	}
 
+	public static final class FinalFieldAccessInFieldDeclarationFinder extends HierarchicalASTVisitor {
+
+		private MethodDeclaration fMethodDeclaration;
+		private ASTNode fFieldDeclaration;
+		static boolean hasReference(MethodDeclaration node) {
+			try {
+				FinalFieldAccessInFieldDeclarationFinder finder= new FinalFieldAccessInFieldDeclarationFinder();
+				ClassInstanceCreation cic= (ClassInstanceCreation) node.getParent().getParent();
+				cic.getType().resolveBinding();
+				finder.fMethodDeclaration= node;
+				finder.fFieldDeclaration= finder.findFieldDeclaration(node);
+				if (finder.fFieldDeclaration == null) {
+					return false;
+				}
+				node.accept(finder);
+			} catch (AbortSearchException e) {
+				return true;
+			}
+			return false;
+		}
+
+		private ASTNode findFieldDeclaration(ASTNode node) {
+			while (node != null) {
+				if (node instanceof FieldDeclaration) {
+					return node;
+				}
+				if (node instanceof AbstractTypeDeclaration) {
+					return null;
+				}
+				node= node.getParent();
+			}
+			return null;
+		}
+
+		@Override
+		public boolean visit(AnonymousClassDeclaration node) {
+			return false;
+		}
+
+		@Override
+		public boolean visit(BodyDeclaration node) {
+			return false;
+		}
+
+		@Override
+		public boolean visit(MethodDeclaration node) {
+			return node == fMethodDeclaration;
+		}
+
+		private void checkForUninitializedFinalReference(IBinding binding) {
+			if (binding instanceof IVariableBinding) {
+				int modifiers= ((IVariableBinding)binding).getModifiers();
+				if ((modifiers & IModifierConstants.ACC_FINAL) == IModifierConstants.ACC_FINAL) {
+					if (((IVariableBinding) binding).isField()) {
+						ASTNode decl= ((CompilationUnit)fMethodDeclaration.getRoot()).findDeclaringNode(binding);
+						if (decl instanceof VariableDeclaration && ((VariableDeclaration)decl).getInitializer() == null) {
+							throw new AbortSearchException();
+						}
+					}
+				}
+			}
+		}
+
+		@Override
+		public boolean visit(SuperFieldAccess node) {
+			IVariableBinding binding= node.resolveFieldBinding();
+			if (binding == null) {
+				return true;
+			}
+			IVariableBinding decl= binding.getVariableDeclaration();
+			checkForUninitializedFinalReference(decl);
+			return true;
+		}
+
+		@Override
+		public boolean visit(SimpleName node) {
+			node.getParent();
+			IBinding binding= node.resolveBinding();
+			checkForUninitializedFinalReference(binding);
+			return true;
+		}
+
+		@Override
+		public boolean visit(QualifiedName node) {
+			node.getParent();
+			IBinding binding= node.resolveBinding();
+			checkForUninitializedFinalReference(binding);
+			return true;
+		}
+
+		@Override
+		public boolean visit(FieldAccess node) {
+			IVariableBinding binding= node.resolveFieldBinding();
+			if (binding == null) {
+				return true;
+			}
+			IVariableBinding decl= binding.getVariableDeclaration();
+			checkForUninitializedFinalReference(decl);
+			return true;
+		}
+
+	}
+
 	public static final class SuperThisQualifier extends HierarchicalASTVisitor {
 
 		private ITypeBinding fQualifierTypeBinding;
@@ -697,6 +805,13 @@ public class LambdaExpressionsFixCore extends CompilationUnitRewriteOperationsFi
 			return false;
 		}
 
+		// lambda cannot access this and so we should avoid lambda conversion
+		// when anonymous class is used to initialize field and refers to
+		// final fields that may or may not be initialized
+		if (FinalFieldAccessInFieldDeclarationFinder.hasReference(methodDecl)) {
+			return false;
+		}
+
 		if (ASTNodes.getTargetType(node) == null) {
 			return false;
 		}
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest18.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest18.java
index 1725415477..692a2895b9 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest18.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest18.java
@@ -1533,6 +1533,74 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 	}
 
 	@Test
+	public void testConvertToLambda28() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		String buf=
+		"package test;\n" +
+		"public class C1 {\n" +
+		"    private final String s;\n" +
+		"    Runnable run = new Runnable() {\n" +
+		"        @Override\n" +
+		"        public void run() {\n" +
+		"           for (int i=0; i < s.length(); ++i) {\n" +
+		"               int j = i;\n" +
+		"           }\n" +
+		"        }\n" +
+		"    };\n" +
+		"    public C1() {\n" +
+		"        s = \"abc\";\n" +
+		"    }\n" +
+		"}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("C1.java", buf, false, null);
+
+		int offset= buf.toString().indexOf("new");
+		AssistContext context= getCorrectionContext(cu, offset, 0);
+		assertNoErrors(context);
+		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
+
+		assertNumberOfProposals(proposals, 0);
+	}
+
+	@Test
+	public void testConvertToLambda29() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		String buf= ""
+				+ "package test;\n"
+				+ "public class C1 {\n"
+				+ "    private final String s = \"ABC\";\n"
+				+ "    Runnable run = new Runnable() {\n"
+				+ "        @Override\n"
+				+ "        public void run() {\n"
+				+ "           for (int i=0; i < s.length(); ++i) {\n"
+				+ "               int j = i;\n"
+				+ "           }\n"
+				+ "        }\n"
+				+ "    };\n"
+				+ "}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("C1.java", buf, false, null);
+
+		int offset= buf.toString().indexOf("new");
+		AssistContext context= getCorrectionContext(cu, offset, 0);
+		assertNoErrors(context);
+		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
+
+		assertNumberOfProposals(proposals, 1);
+		assertCorrectLabels(proposals);
+
+		String expected1= ""
+				+ "package test;\n"
+				+ "public class C1 {\n"
+				+ "    private final String s = \"ABC\";\n"
+			    + "    Runnable run = () -> {\n"
+			    + "       for (int i=0; i < s.length(); ++i) {\n"
+			    + "           int j = i;\n"
+			    + "       }\n"
+			    + "    };\n"
+				+ "}\n";
+		assertExpectedExistInProposals(proposals, new String[] { expected1 });
+	}
+
+	@Test
 	public void testConvertToLambdaAmbiguousOverridden() throws Exception {
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 		StringBuffer buf= new StringBuffer();
@@ -1551,7 +1619,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    }\n");
 		buf.append("}\n");
 		ICompilationUnit cu= pack1.createCompilationUnit("Test.java", buf.toString(), false, null);
-		
+
 		int offset= buf.toString().indexOf("public boolean test(");
 		AssistContext context= getCorrectionContext(cu, offset, 0);
 		assertNoErrors(context);
@@ -1573,7 +1641,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 
 		assertExpectedExistInProposals(proposals, new String[] { expected1 });
 	}
-	
+
 	@Test
 	public void testConvertToAnonymousClassCreation1() throws Exception {
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
@@ -2081,15 +2149,15 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    IntFunction<String> toString= (int i) -> Integer.toString(i);\n");
 		buf.append("}\n");
 		ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
-	
+
 		int offset= buf.toString().indexOf("->");
 		AssistContext context= getCorrectionContext(cu, offset, 0);
 		assertNoErrors(context);
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
-	
+
 		assertNumberOfProposals(proposals, 4);
 		assertCorrectLabels(proposals);
-	
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("import java.util.function.IntFunction;\n");
@@ -2102,7 +2170,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    };\n");
 		buf.append("}\n");
 		String expected1= buf.toString();
-	
+
 		assertExpectedExistInProposals(proposals, new String[] { expected1 });
 	}
 
@@ -3430,14 +3498,14 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    }\n");
 		buf.append("}\n");
 		ICompilationUnit cu= pack1.createCompilationUnit("E10.java", buf.toString(), false, null);
-		
+
 		int offset= buf.toString().indexOf("::") + 1;
 		AssistContext context= getCorrectionContext(cu, offset, 0);
 		assertNoErrors(context);
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
 		assertNumberOfProposals(proposals, 4);
 		assertCorrectLabels(proposals);
-		
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("import java.lang.annotation.*;\n");
@@ -3452,7 +3520,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    }\n");
 		buf.append("}\n");
 		String expected1= buf.toString();
-		
+
 		assertExpectedExistInProposals(proposals, new String[] { expected1 });
 	}
 
@@ -4812,7 +4880,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("    };\n");
 		buf.append("}\n");
 		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);
-		
+
 	}
 
 	@Test
@@ -4821,7 +4889,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		options.put(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
 		JavaCore.setOptions(options);
 		JavaProjectHelper.addLibrary(fJProject1, new Path(Java18ProjectTestSetup.getJdtAnnotations20Path()));
-		
+
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 
 		StringBuffer buf;
@@ -4912,7 +4980,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("	}\n");
 		buf.append("}\n");
 		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);
-		
+
 	}
 
 	@Test
@@ -4921,7 +4989,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		options.put(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
 		JavaCore.setOptions(options);
 		JavaProjectHelper.addLibrary(fJProject1, new Path(Java18ProjectTestSetup.getJdtAnnotations20Path()));
-		
+
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 
 		StringBuffer buf;
@@ -5020,7 +5088,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("	}\n");
 		buf.append("}\n");
 		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);
-		
+
 	}
 
 	@Test
@@ -5029,7 +5097,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		options.put(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
 		JavaCore.setOptions(options);
 		JavaProjectHelper.addLibrary(fJProject1, new Path(Java18ProjectTestSetup.getJdtAnnotations20Path()));
-		
+
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 
 		StringBuffer buf;
@@ -5132,7 +5200,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("	}\n");
 		buf.append("}\n");
 		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);
-		
+
 	}
 
 	@Test
@@ -5141,7 +5209,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		options.put(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
 		JavaCore.setOptions(options);
 		JavaProjectHelper.addLibrary(fJProject1, new Path(Java18ProjectTestSetup.getJdtAnnotations20Path()));
-		
+
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 
 		StringBuffer buf;
@@ -5213,8 +5281,8 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("");
 		pack1.createCompilationUnit("Test.java", buf.toString(), false, null);
 
-		// --- Classes in which the quick assists are checked (without and with NonNullByDefault in effect at the target location) --- 
-		
+		// --- Classes in which the quick assists are checked (without and with NonNullByDefault in effect at the target location) ---
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("\n");
@@ -5252,7 +5320,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		List<IJavaCompletionProposal> proposals2= collectAssists(context2, false);
 
 		// --- Convert to method reference without and with NNBD ---
-		
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("\n");
@@ -5282,7 +5350,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		assertProposalPreviewEquals(buf.toString(), "Convert to method reference", proposals2);
 
 		// --- Add inferred lambda parameter types without and with NNBD ---
-		
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("\n");
@@ -5330,7 +5398,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("	}\n");
 		buf.append("}\n");
 		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals1);
-		
+
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
 		buf.append("\n");
@@ -5392,9 +5460,9 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 		buf.append("        }\n");
 		buf.append("    };\n");
 		buf.append("}\n");
-		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);		
+		assertProposalPreviewEquals(buf.toString(), "Convert to anonymous class creation", proposals);
 	}
-	
+
 	@Test
 	public void testNoRedundantNonNullInConvertArrayForLoop() throws Exception {
 		NullTestUtils.prepareNullTypeAnnotations(fSourceFolder);
@@ -5414,7 +5482,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 
 	        AssistContext context= getCorrectionContext(cu, buf.toString().indexOf("for"), 0);
 			ArrayList<IJavaCompletionProposal> proposals= collectAssists(context, false);
-			
+
 			assertCorrectLabels(proposals);
 			assertNumberOfProposals(proposals, 3);
 
@@ -5454,7 +5522,7 @@ public class AssistQuickFixTest18 extends QuickFixTest {
 
 	        AssistContext context= getCorrectionContext(cu, buf.toString().indexOf("for"), 0);
 			ArrayList<IJavaCompletionProposal> proposals= collectAssists(context, false);
-			
+
 			assertCorrectLabels(proposals);
 			assertNumberOfProposals(proposals, 3);
 
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java
index 769636bb3a..71be2f2e1f 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java
@@ -1238,4 +1238,59 @@ public class CleanUpTest18 extends CleanUpTestCase {
 
 		assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
 	}
+
+	// fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=560018
+	@Test
+	public void testConvertToLambdaInFieldInitializerWithFinalFieldReference() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		String buf= ""
+				+ "package test;\n"
+				+ "public class C1 {\n"
+				+ "    final String s;\n"
+				+ "    Runnable run1 = new Runnable() {\n"
+				+ "        @Override\n"
+				+ "        public void run() {\n"
+				+ "            System.out.println(s\n"
+				+ "        }\n"
+				+ "    };\n"
+				+ "    public C1() {\n"
+				+ "        s = \"abc\";\n"
+				+ "    };\n"
+				+ "}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("C1.java", buf, false, null);
+
+		enable(CleanUpConstants.CONVERT_FUNCTIONAL_INTERFACES);
+		enable(CleanUpConstants.USE_LAMBDA);
+
+		assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
+	}
+
+	// fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=560018
+	@Test
+	public void testConvertToLambdaInFieldInitializerWithFinalFieldReference2() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		String buf= ""
+				+ "package test;\n"
+				+ "public class C1 {\n"
+				+ "    final String s = \"abc\";\n"
+				+ "    Runnable run1 = new Runnable() {\n"
+				+ "        @Override\n"
+				+ "        public void run() {\n"
+				+ "            System.out.println(s);\n"
+				+ "        }\n"
+				+ "    };\n"
+				+ "}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("C1.java", buf, false, null);
+
+		enable(CleanUpConstants.CONVERT_FUNCTIONAL_INTERFACES);
+		enable(CleanUpConstants.USE_LAMBDA);
+
+		String expected1= ""
+				+ "package test;\n"
+				+ "public class C1 {\n"
+				+ "    final String s = \"abc\";\n"
+				+ "    Runnable run1 = () -> System.out.println(s);\n"
+				+ "}\n";
+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected1 });
+	}
 }