Skip to content

Commit e7ff87c

Browse files
eric-milleseric-milles
committed
GROOVY-11290
see #1534
1 parent 29e7795 commit e7ff87c

File tree

8 files changed

+271
-34
lines changed

8 files changed

+271
-34
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3954,17 +3954,18 @@ public void testInstanceOf25() {
39543954
{"Object[]" , "Object[][]" , "java.lang.Object[][]" , },
39553955
{"CharSequence", "String" , "java.lang.String" , },
39563956
{"CharSequence", "Cloneable" , "java.lang.CharSequence & java.lang.Cloneable" , },
3957-
{"CharSequence", "Cloneable,Closeable" , "java.lang.CharSequence & java.lang.Cloneable & java.io.Closeable", "java.lang.CharSequence & java.io.Closeable"}, // GROOVY-11290
3958-
{"Object" , "CharSequence,Cloneable" , "java.lang.CharSequence & java.lang.Cloneable" , "java.lang.Cloneable" },
3959-
{"Object" , "CharSequence,Cloneable,Closeable", "java.lang.CharSequence & java.lang.Cloneable & java.io.Closeable", "java.io.Closeable" },
3960-
{"Number" , "BigInteger,Cloneable" , "java.math.BigInteger & java.lang.Cloneable" , "java.lang.Number & java.lang.Cloneable" },
3957+
{"CharSequence", "Cloneable,Closeable" , "java.lang.CharSequence & java.lang.Cloneable & java.io.Closeable", },
3958+
{"Object" , "CharSequence,Cloneable" , "java.lang.CharSequence & java.lang.Cloneable" , },
3959+
{"Object" , "CharSequence,Cloneable,Closeable", "java.lang.CharSequence & java.lang.Cloneable & java.io.Closeable", },
3960+
{"Number" , "BigInteger,Cloneable" , "java.math.BigInteger & java.lang.Cloneable" , },
39613961
{"Cloneable" , "Number" , "java.lang.Number & java.lang.Cloneable" , },
39623962
{"Cloneable" , "Number,Short" , "java.lang.Short & java.lang.Cloneable" , },
39633963
{"Comparable" , "Number,Short" , "java.lang.Short" , },
39643964
{"Object" , "Comparable,Short" , "java.lang.Short" , },
39653965
{"Object" , "Comparable,Number,Short" , "java.lang.Short" , },
3966-
{"Object" , "Float,Short" , "java.lang.Float" , "java.lang.Short" },
3967-
{"Cloneable" , "Float,Short" , "java.lang.Float & java.lang.Cloneable" , "java.lang.Short & java.lang.Cloneable" },
3966+
{"Object" , "Float,Short" , "java.lang.Float" , "java.lang.Float & java.lang.Short" },
3967+
{"Cloneable" , "Float,Short" , "java.lang.Float & java.lang.Cloneable" , "java.lang.Float & java.lang.Short & java.lang.Cloneable"},
3968+
{"List<java.lang.String>", "Iterable,Collection,List", "java.util.List<java.lang.String>" , },
39683969
};
39693970
//@formatter:on
39703971

@@ -3993,16 +3994,7 @@ public void testInstanceOf25() {
39933994
contents.append("}\n");
39943995

39953996
assertType(contents.toString(), "object", test[2]);
3996-
var expect = test[2];
3997-
if (types[types.length - 1].equals("Short")) { // TODO: STC won't reduce
3998-
if (!types[0].equals("Comparable")) {
3999-
expect = "java.lang." + types[0] + " & " + test[test.length - 1];
4000-
} else {
4001-
expect = java.util.Arrays.stream(types).skip(1).map(type -> "java.lang." + type)
4002-
.collect(java.util.stream.Collectors.joining(" & ")) + " & java.lang.Comparable";
4003-
}
4004-
}
4005-
assertType("@groovy.transform.TypeChecked " + contents, "object", expect);
3997+
assertType("@groovy.transform.TypeChecked " + contents, "object", test[test.length - 1]);
40063998
}
40073999

40084000
//
@@ -4016,11 +4008,12 @@ public void testInstanceOf25() {
40164008
contents.append("}\n");
40174009

40184010
int offset = contents.indexOf("object;");
4019-
assertType(contents.toString(), offset, offset + 6, "java.lang." + test[0]);
4020-
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, "java.lang." + test[0]);
4011+
var expect = test[0].startsWith("List") ? "java.util." + test[0] : "java.lang." + test[0];
4012+
assertType(contents.toString(), offset, offset + 6, expect);
4013+
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, expect);
40214014
offset = contents.lastIndexOf("object");
40224015
assertType(contents.toString(), offset, offset + 6, test[2]);
4023-
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, test[2]);
4016+
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, isAtLeastGroovy(50) ? test[test.length - 1] : test[2]);
40244017

40254018
//
40264019

@@ -4034,11 +4027,12 @@ public void testInstanceOf25() {
40344027
contents.append("}\n");
40354028

40364029
offset = contents.indexOf("object;");
4037-
assertType(contents.toString(), offset, offset + 6, "java.lang." + test[0]);
4038-
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, "java.lang." + test[0]);
4030+
expect = test[0].startsWith("List") ? "java.util." + test[0] : "java.lang." + test[0];
4031+
assertType(contents.toString(), offset, offset + 6, expect);
4032+
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, expect);
40394033
offset = contents.lastIndexOf("object");
40404034
assertType(contents.toString(), offset, offset + 6, test[2]);
4041-
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, test[2]);
4035+
assertType("@groovy.transform.TypeChecked " + contents, offset + 30, offset + 36, isAtLeastGroovy(50) ? test[test.length - 1] : test[2]);
40424036
}
40434037
}
40444038
}

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/ast/ClassNode.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,13 @@ public boolean isDerivedFrom(ClassNode type) {
957957
if (type.equals(ClassHelper.OBJECT_TYPE)) {
958958
return true;
959959
}
960+
// GRECLIPSE add -- GROOVY-11290
961+
if (this.isArray() && type.isArray()
962+
&& type.getComponentType().equals(ClassHelper.OBJECT_TYPE)
963+
&& !ClassHelper.isPrimitiveType(this.getComponentType())){
964+
return true;
965+
}
966+
// GRECLIPSE end
960967
ClassNode node = this;
961968
while (node != null) {
962969
if (type.equals(node)) {

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2319,7 +2319,18 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
23192319
public void visitExpressionStatement(final ExpressionStatement statement) {
23202320
typeCheckingContext.pushTemporaryTypeInfo();
23212321
super.visitExpressionStatement(statement);
2322+
/* GRECLIPSE edit -- GROOVY-11290
23222323
typeCheckingContext.popTemporaryTypeInfo();
2324+
*/
2325+
Map<?,List<ClassNode>> tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop();
2326+
if (!tti.isEmpty() && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
2327+
tti.forEach((k, tempTypes) -> {
2328+
if (tempTypes.contains(VOID_TYPE))
2329+
typeCheckingContext.temporaryIfBranchTypeInformation.peek()
2330+
.computeIfAbsent(k, x -> new LinkedList<>()).add(VOID_TYPE);
2331+
});
2332+
}
2333+
// GRECLIPSE end
23232334
}
23242335

23252336
@Override
@@ -2495,8 +2506,9 @@ protected ClassNode[] getArgumentTypes(final ArgumentListExpression args) {
24952506
}
24962507

24972508
private ClassNode getInferredTypeFromTempInfo(final Expression expression, final ClassNode expressionType) {
2498-
if (expression instanceof VariableExpression && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
2509+
if (expression instanceof VariableExpression) {
24992510
List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
2511+
/* GRECLIPSE edit -- GROOVY-11290
25002512
if (tempTypes != null && !tempTypes.isEmpty()) {
25012513
List<ClassNode> types = new ArrayList<>(tempTypes.size() + 1);
25022514
if (expressionType != null && !expressionType.equals(OBJECT_TYPE) // GROOVY-7333
@@ -2513,6 +2525,51 @@ private ClassNode getInferredTypeFromTempInfo(final Expression expression, final
25132525
return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
25142526
}
25152527
}
2528+
*/
2529+
if (!tempTypes.isEmpty()) {
2530+
ClassNode superclass;
2531+
ClassNode[] interfaces;
2532+
if (expressionType instanceof WideningCategories.LowestUpperBoundClassNode) {
2533+
superclass = expressionType.getSuperClass();
2534+
interfaces = expressionType.getInterfaces();
2535+
} else if (expressionType != null && expressionType.isInterface()) {
2536+
superclass = OBJECT_TYPE;
2537+
interfaces = new ClassNode[]{expressionType};
2538+
} else {
2539+
superclass = expressionType;
2540+
interfaces = ClassNode.EMPTY_ARRAY;
2541+
}
2542+
2543+
List<ClassNode> types = new ArrayList<>();
2544+
if (superclass != null && !superclass.equals(OBJECT_TYPE) // GROOVY-7333
2545+
&& tempTypes.stream().noneMatch(t -> !t.equals(superclass) && t.isDerivedFrom(superclass))) { // GROOVY-9769
2546+
types.add(superclass);
2547+
}
2548+
for (ClassNode anInterface : interfaces) {
2549+
if (tempTypes.stream().noneMatch(t -> t.implementsInterface(anInterface))) { // GROOVY-9769
2550+
types.add(anInterface);
2551+
}
2552+
}
2553+
int tempTypesCount = tempTypes.size();
2554+
if (tempTypesCount == 1 && types.isEmpty()) {
2555+
types.add(tempTypes.get(0));
2556+
} else for (ClassNode tempType : tempTypes) {
2557+
if (!tempType.isInterface() // GROOVY-11290: keep most-specific types
2558+
? (superclass == null || !superclass.isDerivedFrom(tempType))
2559+
&& (tempTypesCount == 1 || tempTypes.stream().noneMatch(t -> !t.equals(tempType) && t.isDerivedFrom(tempType)))
2560+
: (expressionType == null || !isOrImplements(expressionType, tempType))
2561+
&& (tempTypesCount == 1 || tempTypes.stream().noneMatch(t -> t != tempType && t.implementsInterface(tempType)))) {
2562+
types.add(tempType);
2563+
}
2564+
}
2565+
2566+
if (types.size() == 1) {
2567+
return types.get(0);
2568+
} else if (types.size() > 1) {
2569+
return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
2570+
}
2571+
}
2572+
// GRECLIPSE end
25162573
}
25172574
return expressionType;
25182575
}
@@ -4052,6 +4109,7 @@ protected void checkForbiddenSpreadArgument(final ArgumentListExpression argumen
40524109
}
40534110

40544111
protected List<ClassNode> getTemporaryTypesForExpression(final Expression objectExpression) {
4112+
/* GRECLIPSE edit -- GROOVY-11290
40554113
List<ClassNode> classNodes = null;
40564114
int depth = typeCheckingContext.temporaryIfBranchTypeInformation.size();
40574115
while (classNodes == null && depth > 0) {
@@ -4062,6 +4120,17 @@ protected List<ClassNode> getTemporaryTypesForExpression(final Expression object
40624120
classNodes = tempo.get(key);
40634121
}
40644122
return classNodes;
4123+
*/
4124+
Object key = extractTemporaryTypeInfoKey(objectExpression);
4125+
List<ClassNode> tempTypes = typeCheckingContext.temporaryIfBranchTypeInformation.stream().flatMap(tti ->
4126+
tti.getOrDefault(key, Collections.emptyList()).stream()
4127+
).collect(Collectors.toList());
4128+
int i = tempTypes.lastIndexOf(VOID_TYPE);
4129+
if (i != -1) { // assignment overwrites instanceof
4130+
tempTypes = tempTypes.subList(i + 1, tempTypes.size());
4131+
}
4132+
return tempTypes.size() <= 1 ? tempTypes : DefaultGroovyMethods.unique(tempTypes); // GROOVY-6429
4133+
// GRECLIPSE end
40654134
}
40664135

40674136
protected void storeTargetMethod(final Expression call, final MethodNode directMethodCallCandidate) {
@@ -4622,12 +4691,17 @@ protected void storeType(final Expression exp, ClassNode cn) {
46224691
assignedTypes.add(cn);
46234692
}
46244693
if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
4694+
/* GRECLIPSE edit -- GROOVY-11290
46254695
List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(var);
46264696
if (temporaryTypesForExpression != null && !temporaryTypesForExpression.isEmpty()) {
46274697
// a type inference has been made on a variable whose type was defined in an instanceof block
46284698
// erase available information with the new type
46294699
temporaryTypesForExpression.clear();
46304700
}
4701+
*/
4702+
if (!var.isThisExpression() && !var.isSuperExpression())
4703+
pushInstanceOfTypeInfo(var, new ClassExpression(VOID_TYPE));
4704+
// GRECLIPSE end
46314705
}
46324706
}
46334707
}

‎base/org.codehaus.groovy40/src/org/codehaus/groovy/ast/ClassNode.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@
5050
import static java.util.stream.Collectors.toList;
5151
import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
5252
import static org.codehaus.groovy.ast.ClassHelper.SEALED_TYPE;
53-
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
54-
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
55-
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid;
5653
import static org.codehaus.groovy.ast.tools.ParameterUtils.parametersEqual;
5754
import static org.codehaus.groovy.transform.RecordTypeASTTransformation.recordNative;
5855
import static groovyjarjarasm.asm.Opcodes.ACC_ABSTRACT;
@@ -985,12 +982,19 @@ public MethodNode getMethod(String name, Parameter[] parameters) {
985982
* @return true if this node is derived from the given ClassNode
986983
*/
987984
public boolean isDerivedFrom(ClassNode type) {
988-
if (isPrimitiveVoid(this)) {
989-
return isPrimitiveVoid(type);
985+
if (ClassHelper.isPrimitiveVoid(this)) {
986+
return ClassHelper.isPrimitiveVoid(type);
990987
}
991-
if (isObjectType(type)) {
988+
if (ClassHelper.isObjectType(type)) {
992989
return true;
993990
}
991+
// GRECLIPSE add -- GROOVY-11290
992+
if (this.isArray() && type.isArray()
993+
&& ClassHelper.isObjectType(type.getComponentType())
994+
&& !ClassHelper.isPrimitiveType(this.getComponentType())) {
995+
return true;
996+
}
997+
// GRECLIPSE end
994998
ClassNode node = this;
995999
while (node != null) {
9961000
if (type.equals(node)) {
@@ -1212,7 +1216,7 @@ public MethodNode getGetterMethod(String getterName, boolean searchSuperClasses)
12121216
boolean booleanReturnOnly = getterName.startsWith("is");
12131217
for (MethodNode method : getDeclaredMethods(getterName)) {
12141218
if (method.getName().equals(getterName) && method.getParameters().length == 0
1215-
&& (booleanReturnOnly ? isPrimitiveBoolean(method.getReturnType()) : !method.isVoidMethod())) {
1219+
&& (booleanReturnOnly ? ClassHelper.isPrimitiveBoolean(method.getReturnType()) : !method.isVoidMethod())) {
12161220
// GROOVY-7363: There can be multiple matches for a getter returning a generic parameter type, due to
12171221
// the generation of a bridge method. The real getter is really the non-bridge, non-synthetic one as it
12181222
// has the most specific and exact return type of the two. Picking the bridge method results in loss of

0 commit comments

Comments
 (0)