Skip to content

Commit

Permalink
Fix for #1564: find and resolve declaration from type variable bound(s)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Mar 30, 2024
1 parent 29e6a48 commit 0e249b3
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 44 deletions.
4 changes: 2 additions & 2 deletions ...sts.builder/src/org/eclipse/jdt/core/groovy/tests/search/DeclarationInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1613,11 +1613,11 @@ public void testJavaInterfaceWithDefaultMethod3() {
int offset = contents.indexOf("toArray");
try { // Java 11 adds default method toArray(IntFunction) to the Collection interface
java.util.Collection.class.getDeclaredMethod("toArray", java.util.function.IntFunction.class);
MethodNode method = assertDeclaration(contents, offset, offset + 7, "java.util.Collection<java.lang.String>", "toArray", DeclarationKind.METHOD);
MethodNode method = assertDeclaration(contents, offset, offset + 7, "java.util.Collection", "toArray", DeclarationKind.METHOD);
assertEquals("java.util.function.IntFunction<T[]>", printTypeName(method.getParameters()[0].getType()));
assertType(contents, "n", "java.lang.Integer");
} catch (Exception e) {
MethodNode method = assertDeclaration(contents, offset, offset + 7, "java.util.List<java.lang.String>", "toArray", DeclarationKind.METHOD);
MethodNode method = assertDeclaration(contents, offset, offset + 7, "java.util.List", "toArray", DeclarationKind.METHOD);
assertEquals("T[]", printTypeName(method.getParameters()[0].getType()));
}
}
Expand Down
51 changes: 45 additions & 6 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 @@ -584,7 +584,7 @@ public void testMap22() {
assertType(contents, offset, offset + 3, "java.util.LinkedHashMap<java.lang.String,java.lang.String>");

offset = contents.indexOf(".&put") + 2;
assertDeclaration(contents, offset, offset + 3, "java.util.HashMap<java.lang.String,java.lang.String>", "put", DeclarationKind.METHOD);
assertDeclaration(contents, offset, offset + 3, "java.util.HashMap", "put", DeclarationKind.METHOD);

offset = contents.indexOf("put(");
assertType(contents, offset, offset + 3, "groovy.lang.Closure<java.lang.String>");
Expand All @@ -601,7 +601,7 @@ public void testMap22() {
//

offset = contents.lastIndexOf(".&put") + 2;
assertDeclaration(contents, offset, offset + 3, "java.util.HashMap<java.lang.String,java.lang.String>", "put", DeclarationKind.METHOD);
assertDeclaration(contents, offset, offset + 3, "java.util.HashMap", "put", DeclarationKind.METHOD);

offset = contents.lastIndexOf("put(");
assertType(contents, offset, offset + 3, "groovy.lang.Closure<java.lang.String>");
Expand Down Expand Up @@ -1011,7 +1011,7 @@ public void testClosure16() {
assumeTrue(isParrotParser());
String contents = "Optional.of(21).map(num -> num * 2).get()\n";
assertType(contents, "get", "java.lang.Integer");
assertDeclaringType(contents, "get", "java.util.Optional<java.lang.Integer>");
assertDeclaringType(contents, "get", "java.util.Optional");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1199
Expand Down Expand Up @@ -1182,6 +1182,45 @@ public void testTypeVariable3() {
assertDeclaringType(contents, "getName", "java.lang.Thread");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1564
public void testTypeVariable4() {
String contents =
"class C<T extends Object & Runnable> {\n" +
" void test(T thingy) {\n" +
" thingy.run()\n" +
" }\n" +
"}\n";

assertType(contents, "run", "java.lang.Void");
assertDeclaringType(contents, "run", "java.lang.Runnable");
}

@Test
public void testTypeVariable5() {
String contents =
"class C<T extends List<String>> {\n" +
" void test(T thingy) {\n" +
" thingy.get(0)\n" +
" }\n" +
"}\n";

assertType(contents, "get", "java.lang.String");
assertDeclaringType(contents, "get", "java.util.List");
}

@Test
public void testTypeVariable6() {
String contents =
"class C<T extends Thread & List<String>> {\n" +
" void test(T thingy) {\n" +
" thingy.get(0)\n" +
" }\n" +
"}\n";

assertType(contents, "get", "java.lang.String");
assertDeclaringType(contents, "get", "java.util.List");
}

@Test // GRECLIPSE-997
public void testNestedGenerics1() {
String contents =
Expand Down Expand Up @@ -1288,7 +1327,7 @@ public void testNestedGenerics9() {
"use(Cat){new DirectChannelSpec().m()}\n";

assertType(contents, "prop", "X");
assertDeclaringType(contents, "prop", "ChannelSpec<X,Y>");
assertDeclaringType(contents, "prop", "ChannelSpec");

assertType(contents, "m", "DirectChannelSpec"); // GROOVY-10887
}
Expand Down Expand Up @@ -1602,7 +1641,7 @@ public void testStaticMethod11() {
String toFind = "removeAll";
int start = contents.indexOf(toFind), end = start + toFind.length();
assertType(contents, start, end, "java.lang.Boolean");
MethodNode m = assertDeclaration(contents, start, end, "java.util.List<java.lang.String>", toFind, DeclarationKind.METHOD);
MethodNode m = assertDeclaration(contents, start, end, "java.util.List", toFind, DeclarationKind.METHOD);
assertEquals("java.util.Collection<?>", printTypeName(m.getParameters()[0].getType()));
}

Expand All @@ -1613,7 +1652,7 @@ public void testStaticMethod12() {
String toFind = "addAll";
int start = contents.indexOf(toFind), end = start + toFind.length();
assertType(contents, start, end, "java.lang.Boolean");
MethodNode m = assertDeclaration(contents, start, end, "java.util.List<java.lang.String>", toFind, DeclarationKind.METHOD);
MethodNode m = assertDeclaration(contents, start, end, "java.util.List", toFind, DeclarationKind.METHOD);
assertEquals("Parameter type should be resolved", "java.util.Collection<java.lang.String>", printTypeName(m.getParameters()[0].getType()));
}

Expand Down
4 changes: 2 additions & 2 deletions ...core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTestSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ private SearchRequestor assertDeclaringType(GroovyCompilationUnit unit, int expr
return requestor;
}

protected <N extends ASTNode> N assertDeclaration(String contents, int exprStart, int exprUntil, String expectedType, String name, DeclarationKind kind) {
SearchRequestor requestor = assertDeclaringType(createUnit(DEFAULT_UNIT_NAME, contents), exprStart, exprUntil, expectedType, false);
protected <N extends ASTNode> N assertDeclaration(String contents, int exprStart, int exprUntil, String expectedDeclType, String name, DeclarationKind kind) {
SearchRequestor requestor = assertDeclaringType(createUnit(DEFAULT_UNIT_NAME, contents), exprStart, exprUntil, expectedDeclType, false);

switch (kind) {
case CLASS:
Expand Down
10 changes: 5 additions & 5 deletions ...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ public void testSuperInterfaceMethod2() {
assertUnknown(contents, "reversed");
} else {
assertType(contents, "reversed", "java.util.Comparator<java.lang.String>");
assertDeclaringType(contents, "reversed", "java.util.Comparator<java.lang.String>");
assertDeclaringType(contents, "reversed", "java.util.Comparator");
}
}

Expand Down Expand Up @@ -2314,15 +2314,15 @@ public void testAnonInner5() {
public void testAnonInner6() {
String contents = "def aic = new Comparable<String>() {\n int compareTo(String that) {}\n}\n" +
"aic.compareTo('x')";
assertDeclaringType(contents, "compareTo", "java.lang.Comparable<java.lang.String>");
assertDeclaringType(contents, "compareTo", "java.lang.Comparable");
}

@Test
public void testAnonInner7() {
String contents = "def aic = new Comparable<Integer>() {\n int compareTo(Integer that) {}\n}\n" +
"aic = new Comparable<String>() {\n int compareTo(String that) {}\n}\n" +
"aic.compareTo('x')";
assertDeclaringType(contents, "compareTo", "java.lang.Comparable<java.lang.String>");
assertDeclaringType(contents, "compareTo", "java.lang.Comparable");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/378
Expand Down Expand Up @@ -2543,7 +2543,7 @@ public void testListSort3() {
"} as Comparator)\n";
int offset = contents.lastIndexOf("sort");
assertType(contents, offset, offset + 4, jdkListSort ? "java.lang.Void" : "java.util.List<java.lang.Object>");
assertDeclaringType(contents, offset, offset + 4, jdkListSort ? "java.util.List<java.lang.Object>" : "org.codehaus.groovy.runtime.DefaultGroovyMethods");
assertDeclaringType(contents, offset, offset + 4, jdkListSort ? "java.util.List" : "org.codehaus.groovy.runtime.DefaultGroovyMethods");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/368
Expand All @@ -2561,7 +2561,7 @@ public void testListRemove() {
" }\n" +
"}";
int offset = contents.indexOf("a.remove") + 2;
MethodNode m = assertDeclaration(contents, offset, offset + "remove".length(), "java.util.List<java.lang.Object>", "remove", DeclarationKind.METHOD);
MethodNode m = assertDeclaration(contents, offset, offset + "remove".length(), "java.util.List", "remove", DeclarationKind.METHOD);
assertEquals("Should resolve to remove(int) due to return type of inner call", "int", printTypeName(m.getParameters()[0].getType()));
}

Expand Down
12 changes: 6 additions & 6 deletions ...re.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/StaticInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,42 +173,42 @@ public void testClassReference20() {
public void testClassReference22() {
String contents = "String.getPackage()";
assertType(contents, "getPackage", "java.lang.Package");
assertDeclaringType(contents, "getPackage", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "getPackage", "java.lang.Class");
}

@Test
public void testClassReference23() {
String contents = "String.package";
assertType(contents, "package", "java.lang.Package");
assertDeclaringType(contents, "package", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "package", "java.lang.Class");
}

@Test
public void testClassReference24() {
String contents = "String.class.getPackage()";
assertType(contents, "getPackage", "java.lang.Package");
assertDeclaringType(contents, "getPackage", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "getPackage", "java.lang.Class");
}

@Test
public void testClassReference25() {
String contents = "String.class.package";
assertType(contents, "package", "java.lang.Package");
assertDeclaringType(contents, "package", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "package", "java.lang.Class");
}

@Test
public void testClassReference26() {
String contents = "def clazz = String; clazz.getPackage()";
assertType(contents, "getPackage", "java.lang.Package");
assertDeclaringType(contents, "getPackage", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "getPackage", "java.lang.Class");
}

@Test
public void testClassReference27() {
String contents = "def clazz = String; clazz.package";
assertType(contents, "package", "java.lang.Package");
assertDeclaringType(contents, "package", "java.lang.Class<java.lang.String>");
assertDeclaringType(contents, "package", "java.lang.Class");
}

@Test
Expand Down
3 changes: 2 additions & 1 deletion ...ore.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/TraitInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,8 @@ public void testPublicMethod7() {
"}\n";
//@formatter:on

assertDeclType(source, "m", "A<java.lang.Number>");
assertDeclType(source, "m", "A");
assertExprType(source, "m", "java.lang.Number");
assertExprType(source, "x", "java.lang.Number");
}

Expand Down
31 changes: 18 additions & 13 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ public TypeLookupResult lookupType(final Expression node, final VariableScope sc

@Override
public TypeLookupResult lookupType(final FieldNode node, final VariableScope scope) {
return new TypeLookupResult(node.getType(), node.getDeclaringClass(), node, TypeConfidence.EXACT, scope);
return new TypeLookupResult(node.getType(), getDeclaringTypeFromDeclaration(node, null), node, TypeConfidence.EXACT, scope);
}

@Override
public TypeLookupResult lookupType(final MethodNode node, final VariableScope scope) {
return new TypeLookupResult(node.getReturnType(), node.getDeclaringClass(), node, TypeConfidence.EXACT, scope);
return new TypeLookupResult(node.getReturnType(), getDeclaringTypeFromDeclaration(node, null), node, TypeConfidence.EXACT, scope);
}

@Override
Expand All @@ -118,7 +118,7 @@ public TypeLookupResult lookupType(final ClassNode node, final VariableScope sco
type = node.getOuterClass();
}
}
return new TypeLookupResult(type, type, node, TypeConfidence.EXACT, scope);
return new TypeLookupResult(type, type.getPlainNodeReference(), node, TypeConfidence.EXACT, scope);
}

@Override
Expand Down Expand Up @@ -173,7 +173,7 @@ protected TypeLookupResult findType(final Expression node, final ClassNode decla

MethodNode target; // use value from node metadata if it's available
if (scope.isMethodCall() && (target = getMethodTarget(node)) != null) {
return new TypeLookupResult(target.getReturnType(), target.getDeclaringClass(), target, confidence, scope);
return new TypeLookupResult(target.getReturnType(), getDeclaringTypeFromDeclaration(target, declaringType), target, confidence, scope);
}

if (node instanceof VariableExpression) {
Expand Down Expand Up @@ -434,7 +434,7 @@ protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String
confidence = TypeConfidence.LOOSELY_INFERRED;
}
if (method != method.getOriginal() && (isTraitBridge(method) || isTraitHelper(method.getDeclaringClass()))) {
resolvedDeclaringType = method.getOriginal().getDeclaringClass();
resolvedDeclaringType = getDeclaringTypeFromDeclaration(method.getOriginal(), resolvedDeclaringType);
declaration = method.getOriginal(); // the trait method
}
}
Expand Down Expand Up @@ -505,7 +505,7 @@ protected TypeLookupResult findTypeForVariable(final VariableExpression var, fin
}

type = getTypeFromDeclaration(decl);
resolvedDeclaringType = ((AnnotatedNode) decl).getDeclaringClass();
resolvedDeclaringType = getDeclaringTypeFromDeclaration(decl, declaringType);
if (decl instanceof MethodNode || !((Variable) decl).isDynamicTyped()) variableInfo = null;
}
} else if (accessedVar instanceof DynamicVariable) {
Expand Down Expand Up @@ -643,6 +643,17 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
return findDeclaration(name, VariableScope.OBJECT_CLASS_NODE, isLhsExpression, isStaticExpression, 0, methodCallArgumentTypes);
}

if (declaringType.isGenericsPlaceHolder()) {
ClassNode[] bounds = GroovyUtils.getTypeParameterBounds(declaringType);
for (ClassNode tvb : bounds) {
ASTNode decl = findDeclaration(name, tvb, isLhsExpression, isStaticExpression, directFieldAccess, methodCallArgumentTypes);
if (decl != null) {
return decl;
}
}
if (bounds.length > 0) return null;
}

boolean isCallExpression = (!isLhsExpression && methodCallArgumentTypes != null);

if (isCallExpression) {
Expand Down Expand Up @@ -1092,13 +1103,7 @@ protected static ClassNode getDeclaringTypeFromDeclaration(final ASTNode declara
} else {
declaringType = VariableScope.OBJECT_CLASS_NODE;
}
// retain inferredDeclaringType's generics if possible
if (inferredDeclaringType.equals(declaringType) &&
!inferredDeclaringType.isGenericsPlaceHolder()) {
return inferredDeclaringType;
} else {
return declaringType.getPlainNodeReference();
}
return declaringType.getPlainNodeReference(); // Java includes type args
}

protected static Optional<FieldNode> findTraitField(final String name, final ClassNode type) {
Expand Down
10 changes: 7 additions & 3 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 @@ -170,6 +170,10 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
}
if (ClassHelper.isPrimitiveType(objectType)) {
objectType = ClassHelper.getWrapper(objectType);
} else if (objectType.isGenericsPlaceHolder()) { // T extends Type & Face; select Type or Face
objectType = Arrays.stream(GroovyUtils.getTypeParameterBounds(objectType)).filter(bound ->
bound.isDerivedFrom(declaringType) || bound.implementsInterface(declaringType)
).findFirst().orElse(objectType);
} else if (objectType.equals(VariableScope.CLASS_CLASS_NODE) &&
!declaringType.equals(VariableScope.CLASS_CLASS_NODE) && !isGroovy) {
objectType = VariableScope.getFirstGenerics(objectType); // peel Class<>
Expand All @@ -186,7 +190,7 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
if (!isStatic && method.getName().equals("getClass") && method.getParameters().length == 0) {
ClassNode classType = VariableScope.clone(method.getReturnType());
classType.getGenericsTypes()[0].setUpperBounds(new ClassNode[] {objectType});
return new TypeLookupResult(classType, method.getDeclaringClass(), method, confidence, scope, extraDoc);
return new TypeLookupResult(classType, declaringType, method, confidence, scope, extraDoc);
}
if (!GroovyUtils.isUsingGenerics(method) || !(GenericsUtils.hasUnresolvedGenerics(method.getReturnType()) ||
GroovyUtils.getParameterTypes(method.getParameters()).stream().anyMatch(GenericsUtils::hasUnresolvedGenerics))) {
Expand Down Expand Up @@ -220,7 +224,7 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
MethodNode source = method; // effectively final version
Parameter target = params[Math.min(index, params.length - 1)];
method = findClosureSignature(target, source, scope.getEnclosingModuleNode().getContext(), cat.call).map(types -> {
GenericsMapper gm = GenericsMapper.gatherGenerics(java.util.Arrays.asList(types), cat.declaringType, source);
GenericsMapper gm = GenericsMapper.gatherGenerics(Arrays.asList(types), cat.declaringType, source);
MethodNode mn = VariableScope.resolveTypeParameterization(gm, source);
return mn;
}).orElse(method);
Expand Down Expand Up @@ -280,7 +284,7 @@ public TypeLookupResult resolveTypeParameterization(final ClassNode objExprType,
}

if (method != declaration || returnType != method.getReturnType()) {
return new TypeLookupResult(returnType, method.getDeclaringClass(), method, this);
return new TypeLookupResult(returnType, declaringType, method, this);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...sing.test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectFieldsTests.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2023 the original author or authors.
* Copyright 2009-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -61,7 +61,7 @@ final class CodeSelectFieldsTests extends BrowsingTestSuite {
| }
|}'''.stripMargin()
], 'items')
assert elem.key == 'LFoo<TItem;>;.items)Ljava/util/List<LFoo;:TItem;>;'
assert elem.key == 'LFoo;.items)Ljava/util/List<LFoo;:TItem;>;'
}

@Test
Expand Down
Loading

0 comments on commit 0e249b3

Please sign in to comment.