Skip to content

Commit

Permalink
Fix for #1199: use LHS type to further resolve method pointer/reference
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Nov 18, 2020
1 parent 47f4441 commit 9068081
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 39 deletions.
58 changes: 33 additions & 25 deletions ...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/GenericInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,24 +855,36 @@ public void testClosure6() {

@Test
public void testClosure6a() {
assumeTrue(isAtLeastGroovy(30));
assumeTrue(isParrotParser());
String contents =
"void test(List<String> list) {\n" +
" def array = list.stream().toArray(String[].&new)\n" +
" def array = list.stream().toArray(String[]::new)\n" +
"}\n";
assertType(contents, "toArray", "java.lang.String[]");
assertType(contents, "array", "java.lang.String[]");
}

@Test
public void testClosure6b() {
assumeTrue(isParrotParser());
assumeTrue(isAtLeastGroovy(30));
String contents =
"void test(List<String> list) {\n" +
" def array = list.stream().toArray(String[]::new)\n" +
" def array = list.stream().toArray(String[].&new)\n" +
"}\n";
assertType(contents, "toArray", "java.lang.String[]");
assertType(contents, "array", "java.lang.String[]");
}

@Test // incorrect LHS type should not alter RHS type
public void testClosure6c() {
String contents =
"void test(List<String> list) {\n" +
" Number[] array = list.stream().toArray(String[].&new)\n" +
"}\n";
assertType(contents, "toArray", "java.lang.String[]");
assertType(contents, "array", "java.lang.Number[]");
}

@Test // GROOVY-9803
public void testClosure7() {
String contents =
Expand Down Expand Up @@ -919,6 +931,12 @@ public void testClosure10() {
assertDeclaringType(contents, "get", "java.util.Optional<java.lang.Integer>");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1199
public void testClosure11() {
String contents = "java.util.function.Function<Integer, List<Integer>> f = Arrays.&asList\n";
assertType(contents, "asList", "java.util.List<java.lang.Integer>");
}

@Test
public void testArrayDGM() {
String contents =
Expand Down Expand Up @@ -1212,7 +1230,15 @@ public void testStaticMethod6() {
}

@Test
public void testStaticMethod7() {
public void testStaticMethod9() {
// Collections: public static final <T> List<T> emptyList()
String contents = "def list = Collections.<String>emptyList()";
assertType(contents, "list", "java.util.List<java.lang.String>");
assertType(contents, "emptyList", "java.util.List<java.lang.String>");
}

@Test
public void testStaticMethod10() {
// Collections: public static final <T> List<T> singletonList(T)
String contents = "List<String> list = Collections.singletonList('')";
String toFind = "singletonList";
Expand All @@ -1222,26 +1248,8 @@ public void testStaticMethod7() {
assertEquals("Parameter type should be resolved", "java.lang.String", printTypeName(m.getParameters()[0].getType()));
}

@Test @Ignore
public void testStaticMethod8() { // no help from parameters
// Collections: public static final <T> List<T> emptyList()
String contents = "List<String> list = Collections.emptyList()";
String toFind = "emptyList";
int start = contents.indexOf(toFind), end = start + toFind.length();
assertType(contents, start, end, "java.util.List<java.lang.String>");
}

@Test
public void testStaticMethod9() {
// Collections: public static final <T> List<T> emptyList()
String contents = "def list = Collections.<String>emptyList()";
String toFind = "list";
int start = contents.indexOf(toFind), end = start + toFind.length();
assertType(contents, start, end, "java.util.List<java.lang.String>");
}

@Test
public void testStaticMethod10() {
public void testStaticMethod11() {
// Collection: public boolean removeAll(Collection<?>)
String contents = "List<String> list = ['1','2']; list.removeAll(['1'])";
String toFind = "removeAll";
Expand All @@ -1253,7 +1261,7 @@ public void testStaticMethod10() {
}

@Test
public void testStaticMethod11() {
public void testStaticMethod12() {
// Collection: public boolean addAll(Collection<? extends E>)
String contents = "List<String> list = ['1','2']; list.addAll(['3'])";
String toFind = "addAll";
Expand Down
5 changes: 3 additions & 2 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/CategoryTypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodPointerExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.tools.GenericsUtils;
import org.codehaus.groovy.reflection.ParameterTypes;
import org.codehaus.groovy.runtime.MetaClassHelper;
import org.eclipse.jdt.groovy.core.util.GroovyUtils;
Expand Down Expand Up @@ -107,8 +108,8 @@ public TypeLookupResult lookupType(final Expression node, final VariableScope sc
}

// must resolve generics here because TypeLookupResult uses declaring class (instead of self type)
if (org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.missesGenericsTypes(resolvedType)) {
GenericsMapper mapper = GenericsMapper.gatherGenerics(selfType, selfType.redirect());
if (GenericsUtils.hasUnresolvedGenerics(resolvedType)) {
GenericsMapper mapper = GenericsMapper.gatherGenerics(selfType);
resolvedType = VariableScope.resolveTypeParameterization(mapper, VariableScope.clone(resolvedType));
}

Expand Down
23 changes: 18 additions & 5 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/GenericsMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,24 @@
public class GenericsMapper {

/**
* Creates a mapper for a particular resolved type tracing up the type hierarchy until the declaring type is reached.
* Creates a mapper for a resolved type.
* <p>
* This is the public entry point for this class.
*
* @param resolvedType unredirected type that has generic types already parameterized
* @param declaringType a type that is somewhere in resolvedType's hierarchy used to find the target of the mapping
* @param resolvedType type that has type parameters already resolved
*/
public static GenericsMapper gatherGenerics(final ClassNode resolvedType) {
return gatherGenerics(resolvedType, resolvedType);
}

/**
* Creates a mapper for a resolved type by tracing up the type hierarchy
* until the declaring type is reached.
* <p>
* This is the public entry point for this class.
*
* @param resolvedType type that has type parameters already resolved
* @param declaringType a type that is somewhere in {@code resolvedType}'s hierarchy used to find the target of the mapping
*/
public static GenericsMapper gatherGenerics(final ClassNode resolvedType, final ClassNode declaringType) {
GenericsMapper mapper = new GenericsMapper();
Expand Down Expand Up @@ -141,7 +154,7 @@ public static GenericsMapper gatherGenerics(final List<ClassNode> argumentTypes,
MethodNode sam = ClassHelper.findSAM(ubt);
if (sam != null) {
// read "T: A[]"
GenericsMapper um = gatherGenerics(ubt, ubt.redirect());
GenericsMapper um = gatherGenerics(ubt);
// read "T: String[]"
Map<String, ClassNode> rm = new HashMap<>();
readGenericsLinks(rm, rbt.getGenericsTypes()[0].getType(), sam.getReturnType());
Expand All @@ -164,7 +177,7 @@ public static GenericsMapper gatherGenerics(final List<ClassNode> argumentTypes,
} else {
// to resolve "T" follow "List<T> -> List<E>" then walk resolved type hierarchy to find "List<E>"
String key = GroovyUtils.getGenericsTypes(ubt.redirect())[j].getName();
GenericsMapper map = gatherGenerics(rbt, ubt.redirect());
GenericsMapper map = gatherGenerics(rbt, ubt);
ClassNode rt = map.findParameter(key, null);

if (rt != null && GroovyUtils.isAssignable(rt, ubt_gt_t)) {
Expand Down
6 changes: 4 additions & 2 deletions ...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ public void visitClosureExpression(final ClosureExpression node) {
try {
// if enclosing closure, owner type is 'Closure', otherwise it's 'typeof(this)'
if (parent.getEnclosingClosure() != null) {
ClassNode closureType = GenericsUtils.nonGeneric(VariableScope.CLOSURE_CLASS_NODE);
ClassNode closureType = VariableScope.CLOSURE_CLASS_NODE.getPlainNodeReference();
closureType.putNodeMetaData("outer.scope", parent.getEnclosingClosureScope());

scope.addVariable("owner", closureType, VariableScope.CLOSURE_CLASS_NODE);
Expand Down Expand Up @@ -2333,6 +2333,8 @@ private List<ClassNode> getMethodCallArgumentTypes(ASTNode node) {
types.add(VariableScope.MATCHER_CLASS_NODE);
} else if (expression instanceof BinaryExpression && ((BinaryExpression) expression).getOperation().isOneOf(new int[] {Types.COMPARISON_OPERATOR, Types.LOGICAL_OPERATOR, Types.MATCH_REGEX, Types.KEYWORD_IN, Types.KEYWORD_INSTANCEOF, 129, 130})) {
types.add(VariableScope.BOOLEAN_CLASS_NODE);
} else if (expression instanceof MethodPointerExpression && ((MethodPointerExpression) expression).getExpression() instanceof ClassExpression && "new".equals(((MethodPointerExpression) expression).getMethodName().getText())/* && isGroovy3+()*/) {
types.add(createParameterizedClosure(((MethodPointerExpression) expression).getExpression().getType()));
} else {
VariableScope scope = scopes.getLast();
scope.setMethodCallArgumentTypes(null);
Expand Down Expand Up @@ -2471,7 +2473,7 @@ private ClassNode[] inferClosureParamTypes(final ClosureExpression node, final V
MethodNode sam;
// check for SAM-type coercion of closure/lambda expression
if (primaryType != null && (sam = ClassHelper.findSAM(primaryType)) != null) {
GenericsMapper m = GenericsMapper.gatherGenerics(primaryType, primaryType.redirect());
GenericsMapper m = GenericsMapper.gatherGenerics(primaryType);
for (ClassNode t : GroovyUtils.getParameterTypes(sam.getParameters())) {
if (i == inferredTypes.length) break;
inferredTypes[i++] = VariableScope.resolveTypeParameterization(m, t);
Expand Down
46 changes: 42 additions & 4 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeLookupResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
*/
package org.eclipse.jdt.groovy.search;

import static java.util.Collections.singletonList;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;

import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotationNode;
Expand All @@ -27,7 +32,9 @@
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.DeclarationExpression;
import org.codehaus.groovy.ast.expr.MethodPointerExpression;
import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.tools.GenericsUtils;
Expand Down Expand Up @@ -144,12 +151,11 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
}

if (!(declaration instanceof MethodNode)) {
GenericsMapper mapper = GenericsMapper.gatherGenerics(objectType, declaringType.redirect());
GenericsMapper mapper = GenericsMapper.gatherGenerics(objectType, declaringType);
ClassNode maybe = VariableScope.resolveTypeParameterization(mapper, VariableScope.clone(type));
if (!maybe.toString(false).equals(type.toString(false))) {
TypeLookupResult result = new TypeLookupResult(maybe, declaringType, declaration, confidence, scope, extraDoc);
result.enclosingAnnotation = enclosingAnnotation;
result.enclosingAssignment = enclosingAssignment;
result.isGroovy = isGroovy;
return result;
}
Expand Down Expand Up @@ -190,10 +196,27 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
mapper = GenericsMapper.gatherGenerics(GroovyUtils.getParameterTypes(sam.getParameters()), declaringType, method);
method = VariableScope.resolveTypeParameterization(mapper, method);

mapper = GenericsMapper.gatherGenerics(targetType, targetType.redirect());
mapper = GenericsMapper.gatherGenerics(targetType);
method = VariableScope.resolveTypeParameterization(mapper, method);
}
}
} else if (testEnclosingAssignment(scope, rhs -> rhs == scope.getEnclosingNode())) {
// maybe the assign target type can help resolve type parameters of method pointer
ClassNode targetType = scope.getEnclosingAssignment().getLeftExpression().getType();

ClassNode returnType; // aligned with referenced method
if (targetType.equals(VariableScope.CLOSURE_CLASS_NODE)) {
returnType = Optional.of(targetType).map(ClassNode::getGenericsTypes)
.filter(Objects::nonNull).map(gts -> gts[0].getType()).orElse(VariableScope.OBJECT_CLASS_NODE);
} else {
returnType = Optional.ofNullable(ClassHelper.findSAM(targetType)).map(sam ->
VariableScope.resolveTypeParameterization(GenericsMapper.gatherGenerics(targetType), VariableScope.clone(sam.getReturnType()))
).orElse(null);
}
if (returnType != null) {
mapper = GenericsMapper.gatherGenerics(singletonList(returnType), declaringType, returnTypeStub(method));
method = VariableScope.resolveTypeParameterization(mapper, method);
}
}
} else {
if (scope.getMethodCallArgumentTypes() != null) argumentTypes.addAll(scope.getMethodCallArgumentTypes());
Expand All @@ -203,12 +226,27 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
if (method != declaration) {
TypeLookupResult result = new TypeLookupResult(method.getReturnType(), method.getDeclaringClass(), method, confidence, scope, extraDoc);
result.enclosingAnnotation = enclosingAnnotation;
result.enclosingAssignment = enclosingAssignment;
result.isGroovy = isGroovy;
return result;
}
}
}
return this;
}

//--------------------------------------------------------------------------

private static MethodNode returnTypeStub(final MethodNode node) {
MethodNode stub = new MethodNode("", 0, VariableScope.VOID_CLASS_NODE, new Parameter[] {new Parameter(node.getReturnType(), "")}, null, null);
stub.setDeclaringClass(node.getDeclaringClass());
stub.setGenericsTypes(node.getGenericsTypes());
return stub;
}

private static boolean testEnclosingAssignment(final VariableScope scope, final Predicate<Expression> rhsTest) {
return Optional.ofNullable(scope.getEnclosingAssignment())
.filter(bexp -> bexp instanceof DeclarationExpression)
.map(BinaryExpression::getRightExpression).filter(rhsTest)
.isPresent();
}
}
2 changes: 1 addition & 1 deletion ...y.eclipse.dsl/src/org/codehaus/groovy/eclipse/dsl/contributions/DSLContributionGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private void internalDelegatesTo(final AnnotatedNode expr, final boolean useName
}
if (!VariableScope.OBJECT_CLASS_NODE.equals(type)) {
// use this to resolve parameterized types
GenericsMapper mapper = GenericsMapper.gatherGenerics(type, type.redirect());
GenericsMapper mapper = GenericsMapper.gatherGenerics(type);

// naked variants of getter and setter methods must be added at the end
// FIXADE why???
Expand Down

0 comments on commit 9068081

Please sign in to comment.