Skip to content

Commit

Permalink
GROOVY-10055, GROOVY-10166
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Oct 18, 2021
1 parent c0a8850 commit 5077522
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 37 deletions.
48 changes: 40 additions & 8 deletions ...ovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,13 @@ public void testTypeChecked12() {
};
//@formatter:on

runNegativeTest(sources, "");
runNegativeTest(sources,
"----------\n" +
"1. ERROR in Main.groovy (at line 4)\n" +
"\tSet<String> keys = args.keySet()\n" +
"\t ^^^^^^^^^^^^^\n" +
"Groovy:[Static type checking] - Incompatible generic argument types. Cannot assign java.util.Set<java.lang.Object> to: java.util.Set<java.lang.String>\n" +
"----------\n");
}

@Test
Expand Down Expand Up @@ -3280,6 +3286,38 @@ public void testTypeChecked10053() {
runConformTest(sources, "[42]");
}

@Test
public void testTypeChecked10055() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" new C<>().foo('x').bar('y').baz('z')\n" +
"}\n",

"Types.groovy",
"abstract class A<Self extends A<Self>> {\n" +
" Self foo(inputs) {\n" +
" this\n" +
" }\n" +
"}\n" +
"abstract class B<Self extends B<Self>> extends A<Self> {\n" +
" Self bar(inputs) {\n" +
" this\n" +
" }\n" +
"}\n" +
"class C<Self extends C<Self>> extends B<Self> {\n" +
" Self baz(inputs) {\n" +
" this\n" +
" }\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources, "");
}

@Test
public void testTypeChecked10056() {
//@formatter:off
Expand Down Expand Up @@ -3681,13 +3719,7 @@ public void testTypeChecked10166() {
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in Main.groovy (at line 17)\n" +
"\ta.c.get(1)\n" +
"\t^^^^^^^^^^\n" +
"Groovy:[Static type checking] - Cannot find matching method T#get(int). Please check if the declared type is correct and if the method exists.\n" +
"----------\n");
runConformTest(sources);
}

@Test
Expand Down
16 changes: 14 additions & 2 deletions ...rg.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6715,7 +6715,6 @@ private static boolean isGenericsPlaceHolderOrArrayOf(ClassNode cn) {
return cn.isGenericsPlaceHolder();
}


private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(MethodNode method, ClassNode receiver, ClassNode declaringClass) {
if (declaringClass.equals(OBJECT_TYPE)) {
Map<GenericsTypeName, GenericsType> resolvedPlaceholders = new HashMap<GenericsTypeName, GenericsType>();
Expand All @@ -6737,8 +6736,18 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(MethodNod
ClassNode current = item;
while (current != null) {
boolean continueLoop = true;
//extract the place holders
// extract the place holders
Map<GenericsTypeName, GenericsType> currentPlaceHolders = new HashMap<GenericsTypeName, GenericsType>();
// GRECLIPSE add -- GROOVY-10055, GROOVY-10166
if (current.getGenericsTypes() != null
? current.getGenericsTypes().length == 0
: current.redirect().getGenericsTypes() != null) {
for (GenericsType gt : current.redirect().getGenericsTypes()) {
ClassNode cn = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
currentPlaceHolders.put(new GenericsTypeName(gt.getName()), cn.getPlainNodeReference().asGenericsType());
}
}
// GRECLIPSE end
if (isGenericsPlaceHolderOrArrayOf(declaringClass) || declaringClass.equals(current)) {
extractGenericsConnections(currentPlaceHolders, current, declaringClass);
if (method != null) addMethodLevelDeclaredGenerics(method, currentPlaceHolders);
Expand Down Expand Up @@ -6769,6 +6778,9 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(MethodNod
// the actual receiver is Foo and declaringClass is Class
current = declaringClass;
}
// GRECLIPSE add -- GROOVY-10055, GROOVY-10166
else current = applyGenericsContext(currentPlaceHolders, current);
// GRECLIPSE end
}
}
if (resolvedPlaceholders == null) {
Expand Down
2 changes: 1 addition & 1 deletion base/org.codehaus.groovy30/src/groovy/grape/GrapeIvy.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ class GrapeIvy implements GrapeEngine {

URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
// check for mutually exclusive arguments
Set<String> keys = args.keySet()
Set<String> keys = (Set<String>) args.keySet()
keys.each { key ->
Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
if (badArgs && !badArgs.disjoint(keys)) {
Expand Down
13 changes: 13 additions & 0 deletions ...rg.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6380,6 +6380,16 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(final Met
boolean continueLoop = true;
// extract the place holders
Map<GenericsTypeName, GenericsType> currentPlaceHolders = new HashMap<>();
// GRECLIPSE add -- GROOVY-10055, GROOVY-10166
if (current.getGenericsTypes() != null
? current.getGenericsTypes().length == 0
: current.redirect().getGenericsTypes() != null) {
for (GenericsType gt : current.redirect().getGenericsTypes()) {
ClassNode cn = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
currentPlaceHolders.put(new GenericsTypeName(gt.getName()), cn.getPlainNodeReference().asGenericsType());
}
}
// GRECLIPSE end
if (isGenericsPlaceHolderOrArrayOf(declaringClass) || declaringClass.equals(current)) {
extractGenericsConnections(currentPlaceHolders, current, declaringClass);
if (method != null) addMethodLevelDeclaredGenerics(method, currentPlaceHolders);
Expand Down Expand Up @@ -6410,6 +6420,9 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(final Met
// the actual receiver is Foo and declaringClass is Class
current = declaringClass;
}
// GRECLIPSE add -- GROOVY-10055, GROOVY-10166
else current = applyGenericsContext(currentPlaceHolders, current);
// GRECLIPSE end
}
}
if (resolvedPlaceholders == null) {
Expand Down
2 changes: 1 addition & 1 deletion base/org.codehaus.groovy40/src/groovy/grape/GrapeIvy.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ class GrapeIvy implements GrapeEngine {

URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
// check for mutually exclusive arguments
Set<String> keys = args.keySet()
Set<String> keys = (Set<String>) args.keySet()
keys.each { key ->
Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
if (badArgs && !badArgs.disjoint(keys)) {
Expand Down
49 changes: 25 additions & 24 deletions ...rg.codehaus.groovy40/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5795,7 +5795,7 @@ && isClassType(receiver)
}

private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(final MethodNode method, ClassNode receiver, final ClassNode declaringClass) {
Map<GenericsTypeName, GenericsType> resolvedPlaceholders = null;
Map<GenericsTypeName, GenericsType> resolvedPlaceHolders = null;
if (isPrimitiveType(receiver) && !isPrimitiveType(declaringClass)) {
receiver = getWrapper(receiver);
}
Expand All @@ -5808,54 +5808,55 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(final Met
for (ClassNode type : todo) {
ClassNode current = type;
while (current != null) {
boolean continueLoop = true;
// extract the place holders
Map<GenericsTypeName, GenericsType> currentPlaceHolders = new HashMap<>();
if (isGenericsPlaceHolderOrArrayOf(declaringClass) || declaringClass.equals(current)) {
// GROOVY-10055: handle diamond or raw
if (current.getGenericsTypes() != null
? current.getGenericsTypes().length == 0
: current.redirect().getGenericsTypes() != null) {
for (GenericsType gt : current.redirect().getGenericsTypes()) {
ClassNode cn = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
currentPlaceHolders.put(new GenericsTypeName(gt.getName()), cn.getPlainNodeReference().asGenericsType());
}
}

boolean currentIsDeclaring = current.equals(declaringClass) || isGenericsPlaceHolderOrArrayOf(declaringClass);
if (currentIsDeclaring) {
extractGenericsConnections(currentPlaceHolders, current, declaringClass);
if (method != null) addMethodLevelDeclaredGenerics(method, currentPlaceHolders);
continueLoop = false;
if (method != null)
addMethodLevelDeclaredGenerics(method, currentPlaceHolders);
} else {
GenericsUtils.extractPlaceholders(current, currentPlaceHolders);
}

if (resolvedPlaceholders != null) {
// merge maps
if (resolvedPlaceHolders != null) { // merge maps
for (Map.Entry<GenericsTypeName, GenericsType> entry : currentPlaceHolders.entrySet()) {
GenericsType gt = entry.getValue();
if (!gt.isPlaceholder()) continue;
GenericsType referenced = resolvedPlaceholders.get(new GenericsTypeName(gt.getName()));
GenericsType referenced = resolvedPlaceHolders.get(new GenericsTypeName(gt.getName()));
if (referenced == null) continue;
entry.setValue(referenced);
}
}
resolvedPlaceholders = currentPlaceHolders;
resolvedPlaceHolders = currentPlaceHolders;

// we are done if we are now in the declaring class
if (!continueLoop) break;
if (currentIsDeclaring) break;

boolean isRawType = (current.getGenericsTypes() == null
&& current.redirect().getGenericsTypes() != null);
current = getNextSuperClass(current, declaringClass);
if (current == null && isClassType(declaringClass)) {
// this can happen if the receiver is Class<Foo>, then
// the actual receiver is Foo and declaringClass is Class
current = declaringClass;
} else if (isRawType) {
current = current.getPlainNodeReference();
} else {
current = applyGenericsContext(currentPlaceHolders, current);
}
}
}
if (resolvedPlaceholders == null) {
String descriptor = "<>";
if (method != null) descriptor = method.getTypeDescriptor();
throw new GroovyBugError(
"Declaring class for method call to '" +
descriptor + "' declared in " + declaringClass.getName() +
" was not matched with found receiver " + receiver.getName() + "." +
" This should not have happened!");
if (resolvedPlaceHolders == null) {
throw new GroovyBugError("Declaring class for method call to '" + (method != null ? method.getTypeDescriptor() : "<>") +
"' declared in " + declaringClass.getName() + " was not matched with found receiver " + receiver.getName() + ". This should not have happened!");
}
return resolvedPlaceholders;
return resolvedPlaceHolders;
}

protected boolean typeCheckMethodsWithGenericsOrFail(final ClassNode receiver, final ClassNode[] arguments, final MethodNode candidateMethod, final Expression location) {
Expand Down
3 changes: 2 additions & 1 deletion 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 @@ -94,7 +94,8 @@ public static GenericsMapper gatherGenerics(final ClassNode resolvedType, final
if (n > 0 && rgts.length == 0) {
rgts = new GenericsType[n]; // assume rCandidate is a raw type
for (int i = 0; i < n; i += 1) {
rgts[i] = new GenericsType(Optional.ofNullable(ugts[i].getUpperBounds()).map(bounds -> bounds[0]).orElse(VariableScope.OBJECT_CLASS_NODE));
rgts[i] = new GenericsType(Optional.ofNullable(ugts[i].getUpperBounds()).map(bounds -> bounds[0])
.orElse(ugts[i].getType().redirect()).getPlainNodeReference()); // GROOVY-10055, GROOVY-10166
}
}
assert rgts.length == ugts.length;
Expand Down

0 comments on commit 5077522

Please sign in to comment.