Skip to content

Commit 5255a65

Browse files
eric-milleseric-milles
committed
GROOVY-10107, GROOVY-10220, GROOVY-10235, GROOVY-10256
1 parent d498406 commit 5255a65

File tree

8 files changed

+279
-44
lines changed

8 files changed

+279
-44
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java

+13-9
Original file line numberDiff line numberDiff line change
@@ -1805,17 +1805,21 @@ public void testCompileStatic8176() {
18051805
String[] sources = {
18061806
"Main.groovy",
18071807
"@groovy.transform.CompileStatic\n" +
1808-
"static <M extends Map> M merge(M to, Map from) {\n" +
1809-
" !from ? to : to.with {\n" +
1810-
" one = from['one']\n" +
1811-
" two = from['two']\n" +
1812-
" return it\n" +
1808+
"static <P extends Pogo> P merge(P pogo, Map spec) {\n" +
1809+
" !spec ? pogo : pogo.tap {\n" +
1810+
" one = spec['one']\n" +
1811+
" two = spec['two']\n" +
18131812
" }\n" +
18141813
"}\n" +
1815-
"def map = [:]\n" +
1816-
"def result = merge(map, [one: 1, two: 2.0])\n" +
1817-
"assert result == [one: 1, two: 2.0]\n" +
1818-
"assert result.is(map)\n",
1814+
"def pogo = new Pogo()\n" +
1815+
"def result = merge(pogo, [one: 1, two: 2.0])\n" +
1816+
"assert result.one == 1 && result.two == 2.0\n" +
1817+
"assert result.is(pogo)\n",
1818+
1819+
"Pogo.groovy",
1820+
"class Pogo {\n" +
1821+
" def one, two\n" +
1822+
"}\n",
18191823
};
18201824
//@formatter:on
18211825

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java

+194-6
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,26 @@ public void testTypeChecked2() {
6666
"Main.groovy",
6767
"@groovy.transform.TypeChecked\n" +
6868
"void method(String message) {\n" +
69-
" List<Integer> ls = new ArrayList<Integer>()\n" +
70-
" ls.add(123)\n" +
71-
" ls.add('abc')\n" +
69+
" List<Integer> ints = new ArrayList<>()\n" +
70+
" ints.add(12345)\n" +
71+
" ints.add('abc')\n" +
72+
" ints << 'def'\n" +
7273
"}\n",
7374
};
7475
//@formatter:on
7576

7677
runNegativeTest(sources,
7778
"----------\n" +
7879
"1. ERROR in Main.groovy (at line 5)\n" +
79-
"\tls.add(\'abc\')\n" +
80+
"\tints.add(\'abc\')\n" +
81+
"\t^^^^^^^^^^^^^^^\n" +
82+
"Groovy:[Static type checking] - Cannot find matching method java.util.ArrayList#add(java.lang.String)." +
83+
" Please check if the declared type is correct and if the method exists.\n" +
84+
"----------\n" +
85+
"2. ERROR in Main.groovy (at line 6)\n" +
86+
"\tints << 'def'\n" +
8087
"\t^^^^^^^^^^^^^\n" +
81-
"Groovy:[Static type checking] - Cannot find matching method java.util.ArrayList#add(java.lang.String). Please check if the declared type is correct and if the method exists.\n" +
88+
"Groovy:[Static type checking] - Cannot call <T> java.util.ArrayList#leftShift(T) with arguments [java.lang.String]\n" +
8289
"----------\n");
8390
}
8491

@@ -483,6 +490,23 @@ public void testTypeChecked6240() {
483490
runConformTest(sources, "A1B2C3");
484491
}
485492

493+
@Test
494+
public void testTypeChecked6455() {
495+
//@formatter:off
496+
String[] sources = {
497+
"Main.groovy",
498+
"@groovy.transform.TypeChecked\n" +
499+
"class IntegerList {\n" +
500+
" @Delegate List<Integer> delegate = new ArrayList<Integer>()\n" +
501+
"}\n" +
502+
"def list = new IntegerList()\n" +
503+
"assert list == []\n",
504+
};
505+
//@formatter:on
506+
507+
runConformTest(sources);
508+
}
509+
486510
@Test
487511
public void testTypeChecked6786() {
488512
//@formatter:off
@@ -774,6 +798,35 @@ public void testTypeChecked7274() {
774798
runConformTest(sources);
775799
}
776800

801+
@Test
802+
public void testTypeChecked7316() {
803+
//@formatter:off
804+
String[] sources = {
805+
"Main.groovy",
806+
"def <T> T blank() {\n" +
807+
"}\n" +
808+
"def <T extends Iterable<?>> T iter() {\n" +
809+
"}\n" +
810+
"def <T extends CharSequence> T seq() {\n" +
811+
"}\n" +
812+
"@groovy.transform.TypeChecked\n" +
813+
"List<?> test() {\n" +
814+
" blank()\n" +
815+
" iter()\n" +
816+
" seq()\n" +
817+
"}\n",
818+
};
819+
//@formatter:on
820+
821+
runNegativeTest(sources,
822+
"----------\n" +
823+
"1. ERROR in Main.groovy (at line 11)\n" +
824+
"\tseq()\n" +
825+
"\t^^^^^\n" +
826+
"Groovy:[Static type checking] - Cannot return value of type #T on method returning type java.util.List<?>\n" +
827+
"----------\n");
828+
}
829+
777830
@Test
778831
public void testTypeChecked7333() {
779832
//@formatter:off
@@ -1856,7 +1909,7 @@ public void testTypeChecked9907() {
18561909

18571910
@Test
18581911
public void testTypeChecked9915() {
1859-
for (String type : new String[] {"List", "Iterable", "Collection"}) {
1912+
for (String type : new String[] {"List", "Collection", "Iterable"}) {
18601913
//@formatter:off
18611914
String[] sources = {
18621915
"Main.groovy",
@@ -1877,6 +1930,24 @@ public void testTypeChecked9915() {
18771930
}
18781931
}
18791932

1933+
@Test
1934+
public void testTypeChecked9915a() {
1935+
for (String type : new String[] {"Set", "Collection", "Iterable"}) {
1936+
//@formatter:off
1937+
String[] sources = {
1938+
"Main.groovy",
1939+
"@groovy.transform.TypeChecked\n" +
1940+
"class C {\n" +
1941+
type + "<String> strings = Collections.emptySet()\n" +
1942+
"}\n" +
1943+
"new C()\n",
1944+
};
1945+
//@formatter:on
1946+
1947+
runConformTest(sources);
1948+
}
1949+
}
1950+
18801951
@Test
18811952
public void testTypeChecked9935() {
18821953
for (String type : new String[] {"def", "int", "Integer", "BigInteger", "BigDecimal"}) {
@@ -2556,6 +2627,34 @@ public void testTypeChecked9998b() {
25562627
runConformTest(sources, "null");
25572628
}
25582629

2630+
@Test
2631+
public void testTypeChecked10002() {
2632+
//@formatter:off
2633+
String[] sources = {
2634+
"Main.groovy",
2635+
"@groovy.transform.TypeChecked\n" +
2636+
"void test() {\n" +
2637+
" List<String> list = ['a','b',3]\n" +
2638+
" Deque<String> deque = ['x','y']\n" +
2639+
"}\n",
2640+
};
2641+
//@formatter:on
2642+
2643+
runNegativeTest(sources,
2644+
"----------\n" +
2645+
"1. ERROR in Main.groovy (at line 3)\n" +
2646+
"\tList<String> list = ['a','b',3]\n" +
2647+
"\t ^^^^^^^^^^^\n" +
2648+
"Groovy:[Static type checking] - Incompatible generic argument types." +
2649+
" Cannot assign java.util.ArrayList<java.io.Serializable<? extends java.lang.Object>> to: java.util.List<java.lang.String>\n" +
2650+
"----------\n" +
2651+
"2. ERROR in Main.groovy (at line 4)\n" +
2652+
"\tDeque<String> deque = ['x','y']\n" +
2653+
"\t^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
2654+
"Groovy:[Static type checking] - Cannot assign value of type java.util.List<java.lang.String> to variable of type java.util.Deque<java.lang.String>\n" +
2655+
"----------\n");
2656+
}
2657+
25592658
@Test
25602659
public void testTypeChecked10006() {
25612660
//@formatter:off
@@ -3281,6 +3380,95 @@ public void testTypeChecked10217() {
32813380
runConformTest(sources, "11");
32823381
}
32833382

3383+
@Test
3384+
public void testTypeChecked10220() {
3385+
//@formatter:off
3386+
String[] sources = {
3387+
"Main.groovy",
3388+
"class C<S, T extends Number> {\n" +
3389+
"}\n" +
3390+
"@groovy.transform.TypeChecked\n" +
3391+
"class D<T> {\n" +
3392+
" C<? extends T, Integer> f\n" +
3393+
" D(C<? extends T, Integer> p) {\n" +
3394+
" f = p\n" +
3395+
" }\n" +
3396+
"}\n" +
3397+
"print(new D<String>(null).f)\n",
3398+
};
3399+
//@formatter:on
3400+
3401+
runConformTest(sources, "null");
3402+
}
3403+
3404+
@Test
3405+
public void testTypeChecked10222() {
3406+
//@formatter:off
3407+
String[] sources = {
3408+
"Main.groovy",
3409+
"@groovy.transform.TypeChecked\n" +
3410+
"class C<T> {\n" +
3411+
" def <X> X m() {\n" +
3412+
" }\n" +
3413+
" void test() {\n" +
3414+
" T x = m()\n" + // Cannot assign value of type #X to variable of type T
3415+
" print x\n" +
3416+
" }\n" +
3417+
"}\n" +
3418+
"new C().test()\n",
3419+
};
3420+
//@formatter:on
3421+
3422+
runConformTest(sources, "null");
3423+
}
3424+
3425+
@Test
3426+
public void testTypeChecked10222a() {
3427+
//@formatter:off
3428+
String[] sources = {
3429+
"Main.groovy",
3430+
"class Task {\n" +
3431+
" def <T> T exec(args) {\n" +
3432+
" args\n" +
3433+
" }\n" +
3434+
"}\n" +
3435+
"class Test {\n" +
3436+
" Task task\n" +
3437+
" @groovy.transform.TypeChecked\n" +
3438+
" def <T> T exec(args) {\n" +
3439+
" task.exec(args)\n" + // Cannot return value of type #T on method returning type T
3440+
" }\n" +
3441+
"}\n" +
3442+
"print(new Test(task: new Task()).exec('works'))\n",
3443+
};
3444+
//@formatter:on
3445+
3446+
runConformTest(sources, "works");
3447+
}
3448+
3449+
@Test
3450+
public void testTypeChecked10235() {
3451+
if (Float.parseFloat(System.getProperty("java.specification.version")) > 8)
3452+
vmArguments = new String[] {"--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED"};
3453+
3454+
//@formatter:off
3455+
String[] sources = {
3456+
"Main.groovy",
3457+
"@groovy.transform.TypeChecked\n" +
3458+
"void test() {\n" +
3459+
" Set<Integer> integers = java.util.concurrent.ConcurrentHashMap.newKeySet()\n" +
3460+
" printSet(integers)\n" + // Cannot call printSet(Set<Integer>) with arguments [KeySetView<Object,Object>]
3461+
"}\n" +
3462+
"void printSet(Set<Integer> integers) {\n" +
3463+
" println(integers)\n" +
3464+
"}\n" +
3465+
"test()\n",
3466+
};
3467+
//@formatter:on
3468+
3469+
runConformTest(sources, "[]");
3470+
}
3471+
32843472
@Test
32853473
public void testTypeChecked10239() {
32863474
assumeTrue(isParrotParser());

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878

7979
import static java.lang.Math.min;
8080
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
81+
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
8182
import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
8283
import static org.codehaus.groovy.ast.ClassHelper.BigInteger_TYPE;
8384
import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
@@ -695,6 +696,7 @@ public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode r
695696
}
696697

697698
public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode right, Expression rightExpression, boolean allowConstructorCoercion) {
699+
/* GRECLIPSE edit -- GROOVY-8983, GROOVY-10107
698700
ClassNode leftRedirect = left.redirect();
699701
ClassNode rightRedirect = right.redirect();
700702
if (leftRedirect == rightRedirect) return true;
@@ -703,13 +705,14 @@ public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode r
703705
return checkCompatibleAssignmentTypes(leftRedirect.getComponentType(), rightRedirect.getComponentType(), rightExpression, false);
704706
}
705707
706-
/* GRECLIPSE edit -- GROOVY-8983
707708
if (right == VOID_TYPE || right == void_WRAPPER_TYPE) {
708709
return left == VOID_TYPE || left == void_WRAPPER_TYPE;
709710
}
710711
*/
711-
if (rightRedirect == void_WRAPPER_TYPE) return leftRedirect == VOID_TYPE;
712-
if (rightRedirect == VOID_TYPE) return leftRedirect == void_WRAPPER_TYPE;
712+
boolean rightExpressionIsNull = isNullConstant(rightExpression);
713+
if (rightExpressionIsNull && !isPrimitiveType(left)) {
714+
return true;
715+
}
713716

714717
if (left.isArray()) {
715718
if (right.isArray()) {
@@ -722,6 +725,13 @@ public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode r
722725
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ GROOVY-8984: "? super T" is only compatible with an Object[] target
723726
}
724727
}
728+
729+
ClassNode leftRedirect = left.redirect();
730+
ClassNode rightRedirect = right.redirect();
731+
if (leftRedirect == rightRedirect) return true;
732+
733+
if (rightRedirect == void_WRAPPER_TYPE) return leftRedirect == VOID_TYPE;
734+
if (rightRedirect == VOID_TYPE) return leftRedirect == void_WRAPPER_TYPE;
725735
// GRECLIPSE end
726736

727737
if (isNumberType(rightRedirect) || WideningCategories.isNumberCategory(rightRedirect)) {
@@ -734,13 +744,13 @@ public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode r
734744
rightRedirect.isDerivedFrom(BigInteger_TYPE);
735745
}
736746
}
737-
747+
/* GRECLIPSE edit -- GROOVY-10107
738748
// if rightExpression is null and leftExpression is not a primitive type, it's ok
739749
boolean rightExpressionIsNull = rightExpression instanceof ConstantExpression && ((ConstantExpression) rightExpression).getValue() == null;
740750
if (rightExpressionIsNull && !isPrimitiveType(left)) {
741751
return true;
742752
}
743-
753+
*/
744754
// on an assignment everything that can be done by a GroovyCast is allowed
745755

746756
// anything can be assigned to an Object, String, Boolean
@@ -802,9 +812,11 @@ public static boolean checkCompatibleAssignmentTypes(ClassNode left, ClassNode r
802812
}
803813
}
804814

805-
// GROOVY-7316 : it is an apparently legal thing to allow this. It's not type safe,
806-
// but it is allowed...
815+
/* GRECLIPSE edit -- GROOVY-7316, GROOVY-10256
807816
return right.isGenericsPlaceHolder();
817+
*/
818+
return right.isGenericsPlaceHolder() && right.asGenericsType().isCompatibleWith(left);
819+
// GRECLIPSE end
808820
}
809821

810822
private static boolean isGroovyConstructorCompatible(final Expression rightExpression) {

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

+9-2
Original file line numberDiff line numberDiff line change
@@ -984,8 +984,15 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType())) {
984984
if (lType.isUsingGenerics() && missesGenericsTypes(resultType)) {
985985
// the inferred type of the binary expression is the type of the RHS
986986
// "completed" with generics type information available from the LHS
987-
if (!resultType.isGenericsPlaceHolder()) // plain reference loses information
988-
resultType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference());
987+
if (lType.equals(resultType)) {
988+
if (!lType.isGenericsPlaceHolder()) resultType = lType;
989+
} else {
990+
Map<GenericsTypeName, GenericsType> gt = new HashMap<>();
991+
extractGenericsConnections(gt, resultType, resultType.redirect());
992+
extractGenericsConnections(gt, lType, getNextSuperClass(resultType, lType));
993+
994+
resultType = applyGenericsContext(gt, resultType.redirect());
995+
}
989996
}
990997
// GRECLIPSE end
991998
ClassNode originType = getOriginalDeclarationType(leftExpression);

0 commit comments

Comments
 (0)