Skip to content

Commit

Permalink
Fix for #1349: fix NPE for raw type inference
Browse files Browse the repository at this point in the history 
GROOVY-9968, GROOVY-10327, GROOVY-10528
  • Loading branch information
@eric-milles
eric-milles committed Mar 29, 2022
1 parent 50f6eb9 commit c6bbb0e
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 44 deletions.
124 changes: 124 additions & 0 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 @@ -522,6 +522,130 @@ public void testTypeChecked21() {
runConformTest(sources);
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1349
public void testTypeChecked22() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"void test(... args) {\n" +
" args?.each { item ->\n" +
" item.properties.each { key, value ->\n" +
" if (value instanceof Iterable) value.each { test(it) }\n" + // NPE
" }\n" +
" }\n" +
"}\n" +
"test([new Object()])\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked23() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" Set<Integer> ints = [1,2,3]\n" +
" assert ints == [1,2,3] as Set\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked24() {
//@formatter:off
String[] sources = {
"Main.groovy",
"import groovy.transform.*\n" +
"import static org.codehaus.groovy.ast.ClassHelper.*\n" +
"import static org.codehaus.groovy.transform.stc.StaticTypesMarker.*\n" +
"@TypeChecked void test() {\n" +
" @ASTTest(phase=INSTRUCTION_SELECTION, value={\n" +
" def type = node.getNodeMetaData(INFERRED_TYPE)\n" +
" assert type == LIST_TYPE\n" +
" assert type.genericsTypes != null\n" +
" assert type.genericsTypes.length == 1\n" +
" assert type.genericsTypes[0].type == STRING_TYPE\n" +
" })\n" +
" Iterable<String> list = (List) null\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTypeChecked25() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" final type = new Type<String,Integer>('works')\n" +
" print type.map { length() }\n" +
"}\n" +
"test()\n",

"Type.groovy",
"@groovy.transform.TupleConstructor(defaults=false)\n" +
"class Type<T,U> {\n" +
" final T value\n" +
" U map(@DelegatesTo(type='T') Closure<U> producer) {\n" +
" producer.delegate = value\n" +
" producer()\n" +
" }\n" +
"}\n",
};
//@formatter:on

runConformTest(sources, "5");
}

@Test
public void testTypeChecked26() {
//@formatter:off
String[] sources = {
"Main.groovy",
"void proc(Pogo p, Closure<Boolean> predicate) {\n" +
" if (predicate.call(p)) {\n" +
//...
" }\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" proc(new Pogo(name:'Abe', age:55)) {\n" +
" it.age >= 18\n" +
" }\n" +
"}\n" +
"test()\n",

"Pogo.groovy",
"class Pogo {\n" +
" String name\n" +
" int age\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in Main.groovy (at line 8)\n" +
"\tit.age >= 18\n" +
"\t^^^^^^" + (isParrotParser() ? "" : "^") + "\n" +
"Groovy:[Static type checking] - No such property: age for class: java.lang.Object\n" +
"----------\n");
}

@Test
public void testTypeChecked5450() {
//@formatter:off
Expand Down
10 changes: 7 additions & 3 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 @@ -3796,7 +3796,7 @@ private ClassNode[] resolveGenericsFromTypeHint(final ClassNode receiver, final
dummyMN.setGenericsTypes(orig.getGenericsTypes());
}
ClassNode classNode = inferReturnTypeGenerics(receiver, dummyMN, arguments);
/* GRECLIPSE edit -- GROOVY-10327
/* GRECLIPSE edit -- GROOVY-9968, GROOVY-10327, GROOVY-10528
ClassNode[] inferred = new ClassNode[classNode.getGenericsTypes().length];
for (int i = 0; i < classNode.getGenericsTypes().length; i++) {
GenericsType genericsType = classNode.getGenericsTypes()[i];
Expand All @@ -3807,8 +3807,12 @@ private ClassNode[] resolveGenericsFromTypeHint(final ClassNode receiver, final
GenericsType[] returnTypeGenerics = classNode.getGenericsTypes();
ClassNode[] inferred = new ClassNode[returnTypeGenerics.length];
for (int i = 0, n = returnTypeGenerics.length; i < n; i += 1) {
GenericsType genericsType = returnTypeGenerics[i];
inferred[i] = getCombinedBoundType(genericsType);
GenericsType gt = returnTypeGenerics[i];
if (!gt.isPlaceholder()) {
inferred[i] = getCombinedBoundType(gt);
} else {
inferred[i] = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
}
}
// GRECLIPSE end
return inferred;
Expand Down
10 changes: 7 additions & 3 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 @@ -3636,7 +3636,7 @@ private ClassNode[] resolveGenericsFromTypeHint(final ClassNode receiver, final
dummyMN.setGenericsTypes(orig.getGenericsTypes());
}
ClassNode classNode = inferReturnTypeGenerics(receiver, dummyMN, arguments);
/* GRECLIPSE edit -- GROOVY-10327
/* GRECLIPSE edit -- GROOVY-9968, GROOVY-10327, GROOVY-10528
ClassNode[] inferred = new ClassNode[classNode.getGenericsTypes().length];
for (int i = 0; i < classNode.getGenericsTypes().length; i++) {
GenericsType genericsType = classNode.getGenericsTypes()[i];
Expand All @@ -3647,8 +3647,12 @@ private ClassNode[] resolveGenericsFromTypeHint(final ClassNode receiver, final
GenericsType[] returnTypeGenerics = classNode.getGenericsTypes();
ClassNode[] inferred = new ClassNode[returnTypeGenerics.length];
for (int i = 0, n = returnTypeGenerics.length; i < n; i += 1) {
GenericsType genericsType = returnTypeGenerics[i];
inferred[i] = getCombinedBoundType(genericsType);
GenericsType gt = returnTypeGenerics[i];
if (!gt.isPlaceholder()) {
inferred[i] = getCombinedBoundType(gt);
} else {
inferred[i] = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
}
}
// GRECLIPSE end
return inferred;
Expand Down
74 changes: 36 additions & 38 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 @@ -3079,17 +3079,11 @@ private void doInferClosureParameterTypes(final ClassNode receiver, final Expres
List<ClassNode[]> closureSignatures = getSignaturesFromHint(selectedMethod, hintClass, options, expression);
List<ClassNode[]> candidates = new LinkedList<>();
for (ClassNode[] signature : closureSignatures) {
// in order to compute the inferred types of the closure parameters, we're using the following trick:
// 1. create a dummy MethodNode for which the return type is a class node for which the generic types are the types returned by the hint
// 2. call inferReturnTypeGenerics
// 3. fetch inferred types from the result of inferReturnTypeGenerics
// In practice, it could be done differently but it has the main advantage of reusing
// existing code, hence reducing the amount of code to debug in case of failure.
ClassNode[] inferred = resolveGenericsFromTypeHint(receiver, arguments, selectedMethod, signature);
resolveGenericsFromTypeHint(receiver, arguments, selectedMethod, signature);
if (signature.length == closureParams.length // same number of arguments
|| (signature.length == 1 && closureParams.length == 0) // implicit it
|| (closureParams.length > signature.length && inferred[inferred.length - 1].isArray())) { // vargs
candidates.add(inferred);
|| (closureParams.length > signature.length && last(signature).isArray())) { // vargs
candidates.add(signature);
}
}
if (candidates.size() > 1) {
Expand Down Expand Up @@ -3147,47 +3141,51 @@ private void doInferClosureParameterTypes(final ClassNode receiver, final Expres
}
}

private ClassNode[] resolveGenericsFromTypeHint(final ClassNode receiver, final Expression arguments, final MethodNode selectedMethod, final ClassNode[] signature) {
ClassNode dummyResultNode = new ClassNode("ClForInference$" + UNIQUE_LONG.incrementAndGet(), 0, OBJECT_TYPE).getPlainNodeReference();
GenericsType[] genericTypes = new GenericsType[signature.length];
for (int i = 0, n = signature.length; i < n; i += 1) {
genericTypes[i] = new GenericsType(signature[i]);
}
dummyResultNode.setGenericsTypes(genericTypes);
/**
* Computes the inferred types of the closure parameters using the following trick:
* <ol>
* <li> creates a dummy MethodNode for which the return type is a class node
* for which the generic types are the types returned by the hint
* <li> calls {@link #inferReturnTypeGenerics}
* <li> returns inferred types from the result
* </ol>
* In practice it could be done differently but it has the main advantage of
* reusing existing code, hence reducing the amount of code to debug in case
* of failure.
*/
private void resolveGenericsFromTypeHint(final ClassNode receiver, final Expression arguments, final MethodNode selectedMethod, final ClassNode[] signature) {
ClassNode returnType = new ClassNode("ClForInference$" + UNIQUE_LONG.incrementAndGet(), 0, OBJECT_TYPE).getPlainNodeReference();
returnType.setGenericsTypes(Arrays.stream(signature).map(ClassNode::asGenericsType).toArray(GenericsType[]::new));

MethodNode dummyMN = selectedMethod instanceof ExtensionMethodNode ? ((ExtensionMethodNode) selectedMethod).getExtensionMethodNode() : selectedMethod;
dummyMN = new MethodNode(
dummyMN.getName(),
dummyMN.getModifiers(),
dummyResultNode,
dummyMN.getParameters(),
dummyMN.getExceptions(),
EmptyStatement.INSTANCE
);
dummyMN = new MethodNode(dummyMN.getName(), dummyMN.getModifiers(), returnType, dummyMN.getParameters(), dummyMN.getExceptions(), null);
dummyMN.setDeclaringClass(selectedMethod.getDeclaringClass());
dummyMN.setGenericsTypes(selectedMethod.getGenericsTypes());
if (selectedMethod instanceof ExtensionMethodNode) {
ExtensionMethodNode orig = (ExtensionMethodNode) selectedMethod;
dummyMN = new ExtensionMethodNode(
dummyMN,
dummyMN.getName(),
dummyMN.getModifiers(),
dummyResultNode,
orig.getParameters(),
orig.getExceptions(),
EmptyStatement.INSTANCE,
orig.isStaticExtension()
returnType,
selectedMethod.getParameters(),
selectedMethod.getExceptions(),
null,
((ExtensionMethodNode) selectedMethod).isStaticExtension()
);
dummyMN.setDeclaringClass(orig.getDeclaringClass());
dummyMN.setGenericsTypes(orig.getGenericsTypes());
dummyMN.setDeclaringClass(selectedMethod.getDeclaringClass());
dummyMN.setGenericsTypes(selectedMethod.getGenericsTypes());
}
ClassNode returnType = inferReturnTypeGenerics(receiver, dummyMN, arguments);

returnType = inferReturnTypeGenerics(receiver, dummyMN, arguments);
GenericsType[] returnTypeGenerics = returnType.getGenericsTypes();
ClassNode[] inferred = new ClassNode[returnTypeGenerics.length];
for (int i = 0, n = returnTypeGenerics.length; i < n; i += 1) {
GenericsType genericsType = returnTypeGenerics[i];
inferred[i] = getCombinedBoundType(genericsType);
GenericsType gt = returnTypeGenerics[i];
if (gt.isPlaceholder()) { // GROOVY-9968, GROOVY-10528, et al.
signature[i] = gt.getUpperBounds() != null ? gt.getUpperBounds()[0] : gt.getType().redirect();
} else {
signature[i] = getCombinedBoundType(gt);
}
}
return inferred;
}

private static String[] convertToStringArray(final Expression options) {
Expand Down Expand Up @@ -3234,7 +3232,7 @@ private void checkClosureWithDelegatesTo(final ClassNode receiver, final MethodN
);
if (resolved != null) {
if (resolved.length == 1) {
resolved = resolveGenericsFromTypeHint(receiver, arguments, mn, resolved);
resolveGenericsFromTypeHint(receiver, arguments, mn, resolved);
expression.putNodeMetaData(DELEGATION_METADATA, newDelegationMetadata(resolved[0], stInt));
} else {
addStaticTypeError("Incorrect type hint found in method " + (mn), type);
Expand Down

0 comments on commit c6bbb0e

Please sign in to comment.