Bug 434507: [1.8][clean up][quick assist] "Convert anonymous to lambda" results in ambiguous method error

Fix for comment 2: bad handling of void-compatibility
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 8bff0c1..62e5faa 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
@@ -185,7 +185,8 @@
 		

 		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { original });

 	}

-

+
+	// fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=434507#c5

 	public void testConvertToLambdaAmbiguous01() throws Exception {

 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

 		StringBuffer buf= new StringBuffer();

@@ -334,6 +335,7 @@
 		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { original });

 	}

 

+	// fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=434507#c5

 	public void testConvertToLambdaAmbiguous02() throws Exception {

 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

 		StringBuffer buf= new StringBuffer();

@@ -480,6 +482,75 @@
 		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { original });

 	}

 

+	// fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=434507#c2

+	public void testConvertToLambdaAmbiguous03() throws Exception {

+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

+		StringBuffer buf= new StringBuffer();

+		buf.append("package test;\n");

+		buf.append("public interface E {\n");

+		buf.append("    default void m() {\n");

+		buf.append("        bar(0, new FI() {\n");

+		buf.append("            @Override\n");

+		buf.append("            public int foo(int x) {\n");

+		buf.append("                return x++;\n");

+		buf.append("            }\n");

+		buf.append("        });\n");

+		buf.append("        baz(0, new ZI() {\n");

+		buf.append("            @Override\n");

+		buf.append("            public int zoo() {\n");

+		buf.append("                return 1;\n");

+		buf.append("            }\n");

+		buf.append("        });\n");

+		buf.append("    }\n");

+		buf.append("\n");

+		buf.append("    void bar(int i, FI fi);\n");

+		buf.append("    void bar(int i, FV fv);\n");

+		buf.append("\n");

+		buf.append("    void baz(int i, ZI zi);\n");

+		buf.append("    void baz(int i, ZV zv);\n");

+		buf.append("}\n");

+		buf.append("\n");

+		buf.append("@FunctionalInterface interface FI { int  foo(int a); }\n");

+		buf.append("@FunctionalInterface interface FV { void foo(int a); }\n");

+		buf.append("\n");

+		buf.append("@FunctionalInterface interface ZI { int  zoo(); }\n");

+		buf.append("@FunctionalInterface interface ZV { void zoo(); }\n");

+		String original= buf.toString();

+		ICompilationUnit cu1= pack1.createCompilationUnit("E.java", original, false, null);

+		

+		enable(CleanUpConstants.CONVERT_FUNCTIONAL_INTERFACES);

+		enable(CleanUpConstants.USE_LAMBDA);

+		

+		buf= new StringBuffer();

+		buf.append("package test;\n");

+		buf.append("public interface E {\n");

+		buf.append("    default void m() {\n");

+		buf.append("        bar(0, (FI) x -> x++);\n");

+		buf.append("        baz(0, () -> 1);\n");

+		buf.append("    }\n");

+		buf.append("\n");

+		buf.append("    void bar(int i, FI fi);\n");

+		buf.append("    void bar(int i, FV fv);\n");

+		buf.append("\n");

+		buf.append("    void baz(int i, ZI zi);\n");

+		buf.append("    void baz(int i, ZV zv);\n");

+		buf.append("}\n");

+		buf.append("\n");

+		buf.append("@FunctionalInterface interface FI { int  foo(int a); }\n");

+		buf.append("@FunctionalInterface interface FV { void foo(int a); }\n");

+		buf.append("\n");

+		buf.append("@FunctionalInterface interface ZI { int  zoo(); }\n");

+		buf.append("@FunctionalInterface interface ZV { void zoo(); }\n");

+		String expected1= buf.toString();

+		

+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 });

+		

+		disable(CleanUpConstants.USE_LAMBDA);

+		enable(CleanUpConstants.USE_ANONYMOUS_CLASS_CREATION);

+		

+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { original });

+	}

+

 	public void testConvertToAnonymousWithWildcards() throws Exception {

 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

 		StringBuffer buf= new StringBuffer();

diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
index f44cabc..a1e7b78 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
@@ -603,7 +603,7 @@
 			return referenceType; // don't lose the unchecked conversion
 
 		} else if (initializer instanceof LambdaExpression || initializer instanceof MethodReference) {
-			if (isTargetAmbiguous(reference)) {
+			if (isTargetAmbiguous(reference, isExplicitlyTypedLambda(initializer))) {
 				return referenceType;
 			} else {
 				ITypeBinding targetType= getTargetType(reference);
@@ -626,12 +626,14 @@
 	 * interface instance.
 	 * 
 	 * @param expression the method argument, which is a functional interface instance
+	 * @param expressionIsExplicitlyTyped <code>true</code> iff the intended replacement for <code>expression</code>
+	 *         is an explicitly typed lambda expression (JLS8 15.27.1)
 	 * @return <code>true</code> if overloaded methods can result in an ambiguous method call or a semantic change,
 	 *         <code>false</code> otherwise
 	 * 
 	 * @since 3.10
 	 */
-	public static boolean isTargetAmbiguous(Expression expression) {
+	public static boolean isTargetAmbiguous(Expression expression, boolean expressionIsExplicitlyTyped) {
 		StructuralPropertyDescriptor locationInParent= expression.getLocationInParent();
 
 		while (locationInParent == ParenthesizedExpression.EXPRESSION_PROPERTY
@@ -711,7 +713,7 @@
 				invocationTargetType= methodBinding.getDeclaringClass();
 			}
 			if (invocationTargetType != null) {
-				TypeBindingVisitor visitor= new AmbiguousTargetMethodAnalyzer(invocationTargetType, methodBinding, argumentIndex, argumentCount);
+				TypeBindingVisitor visitor= new AmbiguousTargetMethodAnalyzer(invocationTargetType, methodBinding, argumentIndex, argumentCount, expressionIsExplicitlyTyped);
 				return !(visitor.visit(invocationTargetType) && Bindings.visitHierarchy(invocationTargetType, visitor));
 			}
 		}
@@ -724,6 +726,7 @@
 		private IMethodBinding fOriginalMethod;
 		private int fArgIndex;
 		private int fArgumentCount;
+		private boolean fExpressionIsExplicitlyTyped;
 
 		/**
 		 * @param declaringType the type binding declaring the <code>originalMethod</code>
@@ -731,12 +734,15 @@
 		 * @param argumentIndex the index of the functional interface instance argument in the
 		 *            method call
 		 * @param argumentCount the number of arguments in the method call
+		 * @param expressionIsExplicitlyTyped <code>true</code> iff the intended replacement for <code>expression</code>
+		 *         is an explicitly typed lambda expression (JLS8 15.27.1)
 		 */
-		public AmbiguousTargetMethodAnalyzer(ITypeBinding declaringType, IMethodBinding originalMethod, int argumentIndex, int argumentCount) {
+		public AmbiguousTargetMethodAnalyzer(ITypeBinding declaringType, IMethodBinding originalMethod, int argumentIndex, int argumentCount, boolean expressionIsExplicitlyTyped) {
 			fDeclaringType= declaringType;
 			fOriginalMethod= originalMethod;
 			fArgIndex= argumentIndex;
 			fArgumentCount= argumentCount;
+			fExpressionIsExplicitlyTyped= expressionIsExplicitlyTyped;
 		}
 
 		public boolean visit(ITypeBinding type) {
@@ -774,6 +780,20 @@
 					if (couldBeAmbiguous) {
 						ITypeBinding parameterType= ASTResolving.getParameterTypeBinding(candidate, fArgIndex);
 						if (parameterType != null && parameterType.getFunctionalInterfaceMethod() != null) {
+							if (!fExpressionIsExplicitlyTyped) {
+								/* According to JLS8 15.12.2.2, implicitly typed lambda expressions are not "pertinent to applicability"
+								 * and hence potentially applicable methods are always "applicable by strict invocation",
+								 * regardless of whether argument expressions are compatible with the method's parameter types or not.
+								 * If there are multiple such methods, 15.12.2.5 results in an ambiguous method invocation.
+								 */
+								return false;
+							}
+							/* Explicitly typed lambda expressions are pertinent to applicability, and hence
+							 * compatibility with the corresponding method parameter type is checked. And since this check
+							 * separates functional interface methods by their void-compatibility state, functional interfaces
+							 * with a different void compatibility are not applicable any more and hence can't cause
+							 * an ambiguous method invocation.
+							 */
 							ITypeBinding origParamType= ASTResolving.getParameterTypeBinding(fOriginalMethod, fArgIndex);
 							boolean originalIsVoidCompatible=  Bindings.isVoidType(origParamType.getFunctionalInterfaceMethod().getReturnType());
 							boolean candidateIsVoidCompatible= Bindings.isVoidType(parameterType.getFunctionalInterfaceMethod().getReturnType());
@@ -931,6 +951,16 @@
 		return false;
 	}
 
+	private static boolean isExplicitlyTypedLambda(Expression expression) {
+		if (!(expression instanceof LambdaExpression))
+			return false;
+		LambdaExpression lambda= (LambdaExpression) expression;
+		List<VariableDeclaration> parameters= lambda.parameters();
+		if (parameters.isEmpty())
+			return true;
+		return parameters.get(0) instanceof SingleVariableDeclaration;
+	}
+
 	/**
 	 * Returns the closest ancestor of <code>node</code> that is an instance of <code>parentClass</code>, or <code>null</code> if none.
 	 * <p>
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java
index a6ccf08..39ee07a 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java
@@ -243,7 +243,7 @@
 				
 				lambdaExpression.setBody(rewrite.createCopyTarget(lambdaBody));
 				Expression replacement= lambdaExpression;
-				if (ASTNodes.isTargetAmbiguous(classInstanceCreation)) {
+				if (ASTNodes.isTargetAmbiguous(classInstanceCreation, lambdaParameters.isEmpty())) {
 					CastExpression cast= ast.newCastExpression();
 					cast.setExpression(lambdaExpression);
 					ImportRewrite importRewrite= cuRewrite.getImportRewrite();