Skip to content

Commit

Permalink
Bug 434507: [1.8][clean up][quick assist] "Convert anonymous to lambd…
Browse files Browse the repository at this point in the history 
…a" results in ambiguous method error

Fix for comment 2: bad handling of void-compatibility
  • Loading branch information
@mkeller
mkeller committed May 12, 2014
1 parent b90b3db commit 7453368
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 6 deletions.
73 changes: 72 additions & 1 deletion org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ public void testConvertToLambdaNestedWithImports() throws Exception {

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();
Expand Down Expand Up @@ -334,6 +335,7 @@ public void testConvertToLambdaAmbiguous01() throws Exception {
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();
Expand Down Expand Up @@ -480,6 +482,75 @@ public void testConvertToLambdaAmbiguous02() throws Exception {
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();
Expand Down
38 changes: 34 additions & 4 deletions org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ else if (needsExplicitBoxing(reference))
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);
Expand All @@ -626,12 +626,14 @@ else if (needsExplicitBoxing(reference))
* 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
Expand Down Expand Up @@ -711,7 +713,7 @@ public static boolean isTargetAmbiguous(Expression expression) {
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));
}
}
Expand All @@ -724,19 +726,23 @@ private static class AmbiguousTargetMethodAnalyzer implements TypeBindingVisitor
private IMethodBinding fOriginalMethod;
private int fArgIndex;
private int fArgumentCount;
private boolean fExpressionIsExplicitlyTyped;

/**
* @param declaringType the type binding declaring the <code>originalMethod</code>
* @param originalMethod the method declaration binding corresponding to the method call
* @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) {
Expand Down Expand Up @@ -774,6 +780,20 @@ public boolean visit(ITypeBinding type) {
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());
Expand Down Expand Up @@ -931,6 +951,16 @@ private static boolean needsExplicitBoxing(Expression expression) {
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>
Expand Down
2 changes: 1 addition & 1 deletion ...lipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModel mod

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();
Expand Down

0 comments on commit 7453368

Please sign in to comment.