You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/08/26 14:59:21 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof` LUB
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new a54700c71d GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof` LUB
a54700c71d is described below
commit a54700c71dec9a4810cfca5d7a1780d47b67d937
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Aug 26 09:15:51 2022 -0500
GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof` LUB
---
.../transform/stc/StaticTypeCheckingVisitor.java | 22 +-
src/test/groovy/transform/stc/BugsSTCTest.groovy | 46 ++--
.../stc/ClosureParamTypeInferenceSTCTest.groovy | 20 ++
.../transform/stc/TypeInferenceSTCTest.groovy | 274 ++++++++++++++++-----
.../asm/sc/TypeInferenceStaticCompileTest.groovy | 16 ++
5 files changed, 288 insertions(+), 90 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 9212345315..8b9549fc67 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3745,12 +3745,19 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
List<Receiver<String>> owners = new ArrayList<>();
if (typeCheckingContext.delegationMetadata != null
&& objectExpression instanceof VariableExpression
- && ((VariableExpression) objectExpression).getName().equals("owner")
+ && ((Variable) objectExpression).getName().equals("owner")
&& /*isNested:*/typeCheckingContext.delegationMetadata.getParent() != null) {
List<Receiver<String>> enclosingClass = Collections.singletonList(
Receiver.<String>make(typeCheckingContext.getEnclosingClassNode()));
addReceivers(owners, enclosingClass, typeCheckingContext.delegationMetadata.getParent(), "owner.");
} else {
+ List<ClassNode> temporaryTypes = getTemporaryTypesForExpression(objectExpression);
+ int temporaryTypesCount = (temporaryTypes != null ? temporaryTypes.size() : 0);
+ if (temporaryTypesCount > 0) {
+ // GROOVY-8965, GROOVY-10180, GROOVY-10668
+ ClassNode commonType = lowestUpperBound(temporaryTypes);
+ if (!commonType.equals(OBJECT_TYPE)) owners.add(Receiver.<String>make(commonType));
+ }
if (isClassClassNodeWrappingConcreteType(receiver)) {
ClassNode staticType = receiver.getGenericsTypes()[0].getType();
owners.add(Receiver.<String>make(staticType)); // Type from Class<Type>
@@ -3764,19 +3771,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
owners.add(Receiver.<String>make(OBJECT_TYPE));
}
}
- if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
- List<ClassNode> potentialReceiverType = getTemporaryTypesForExpression(objectExpression);
- if (potentialReceiverType != null && !potentialReceiverType.isEmpty()) {
- for (ClassNode node : potentialReceiverType) {
- owners.add(Receiver.<String>make(node));
- }
- }
- }
if (typeCheckingContext.lastImplicitItType != null
&& objectExpression instanceof VariableExpression
- && ((VariableExpression) objectExpression).getName().equals("it")) {
+ && ((Variable) objectExpression).getName().equals("it")) {
owners.add(Receiver.<String>make(typeCheckingContext.lastImplicitItType));
}
+ if (temporaryTypesCount > 1) {
+ owners.add(Receiver.<String>make(new UnionTypeClassNode(temporaryTypes.toArray(ClassNode.EMPTY_ARRAY))));
+ }
}
return owners;
}
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index f2a9d1bd27..d4357d24bb 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -371,6 +371,7 @@ class BugsSTCTest extends StaticTypeCheckingTestCase {
GroovyObject obj = new A.B()
'''
}
+
void testCastInnerClassToGroovyObject() {
assertScript '''
class A { static class B {} }
@@ -684,18 +685,16 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- interface C extends A, B {}
-
- class D {
- D(C c) {
- c.m()
- }
+ interface C extends A,B {
}
- class CImpl implements C {
- void m() { }
+ class Impl implements C {
+ void m() {}
}
- new D(new CImpl())
+ void test(C c) {
+ c.m()
+ }
+ test(new Impl())
'''
}
@@ -703,16 +702,16 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- class C implements A,B {
- void m() {}
+ interface C extends A,B {
}
- class D {
- D(C c) {
- c.m()
- }
+ class Impl implements C,A,B {
+ void m() {}
}
- new D(new C())
+ void test(C c) {
+ c.m()
+ }
+ test(new Impl())
'''
}
@@ -720,17 +719,14 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- interface C extends A,B {}
- class CImpl implements C, A,B {
+ class C implements A,B {
void m() {}
}
- class D {
- D(C c) {
- c.m()
- }
- }
- new D(new CImpl())
+ void test(C c) {
+ c.m()
+ }
+ test(new C())
'''
}
@@ -953,6 +949,7 @@ Printer
}
'''
}
+
void testInnerClassImplementsInterfaceMethodWithTrait() {
assertScript '''
class Main {
@@ -978,6 +975,7 @@ Printer
}
'''
}
+
void testInnerClassImplementsInterfaceMethodWithDelegate() {
assertScript '''
class Main {
diff --git a/src/test/groovy/transform/stc/ClosureParamTypeInferenceSTCTest.groovy b/src/test/groovy/transform/stc/ClosureParamTypeInferenceSTCTest.groovy
index 579343bef6..fa7e98430c 100644
--- a/src/test/groovy/transform/stc/ClosureParamTypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosureParamTypeInferenceSTCTest.groovy
@@ -1440,4 +1440,24 @@ class ClosureParamTypeInferenceSTCTest extends StaticTypeCheckingTestCase {
assert iterable.collect { it.prop } == ['x', 'y', 'z']
'''
}
+
+ void testGroovy10180() {
+ assertScript '''
+ void test(args) {
+ if (args instanceof Map) {
+ args.each { e ->
+ def k = e.key, v = e.value
+ }
+ }
+ }
+ '''
+ assertScript '''
+ void test(args) {
+ if (args instanceof Map) {
+ args.each { k, v ->
+ }
+ }
+ }
+ '''
+ }
}
diff --git a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 7bf5d8077a..99cd7be03e 100644
--- a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -18,6 +18,7 @@
*/
package groovy.transform.stc
+import groovy.transform.NotYetImplemented
import org.codehaus.groovy.ast.ClassHelper
import org.codehaus.groovy.ast.ClassNode
import org.codehaus.groovy.ast.MethodNode
@@ -91,25 +92,16 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testInstanceOf() {
+ void testInstanceOf1() {
assertScript '''
Object o
- if (o instanceof String) o.toUpperCase()
- '''
- }
-
- void testEmbeddedInstanceOf() {
- assertScript '''
- Object o
- if (o instanceof Object) {
- if (o instanceof String) {
- o.toUpperCase()
- }
+ if (o instanceof String) {
+ o.toUpperCase()
}
'''
}
- void testEmbeddedInstanceOf2() {
+ void testInstanceOf2() {
assertScript '''
Object o
if (o instanceof String) {
@@ -120,40 +112,72 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testEmbeddedInstanceOf3() {
+ void testInstanceOf3() {
shouldFailWithMessages '''
Object o
if (o instanceof String) {
- if (o instanceof Object) { // causes the inferred type of 'o' to be overwritten
- o.toUpperCase()
- }
+ o.toUpperCase()
}
- ''', 'Cannot find matching method java.lang.Object#toUpperCase()'
+ o.toUpperCase() // ensure that type information is reset
+ ''',
+ 'Cannot find matching method java.lang.Object#toUpperCase()'
}
- void testInstanceOfAfterEach() {
+ void testInstanceOf4() {
shouldFailWithMessages '''
Object o
if (o instanceof String) {
o.toUpperCase()
+ } else {
+ o.toUpperCase() // ensure that type information is reset
}
- o.toUpperCase() // ensure that type information is lost after if()
- ''', 'Cannot find matching method java.lang.Object#toUpperCase()'
+ ''',
+ 'Cannot find matching method java.lang.Object#toUpperCase()'
}
- void testInstanceOfInElseBranch() {
- shouldFailWithMessages '''
- Object o
- if (o instanceof String) {
- o.toUpperCase()
- } else {
- o.toUpperCase() // ensure that type information is lost in else
+ void testInstanceOf5() {
+ assertScript '''
+ class A {
+ void bar() {
+ }
}
- ''', 'Cannot find matching method java.lang.Object#toUpperCase()'
+ class B {
+ void bar() {
+ }
+ }
+
+ void test(o) {
+ if (o instanceof A) {
+ o.bar()
+ }
+ if (o instanceof B) {
+ o.bar()
+ }
+ }
+ test(new A())
+ test(new B())
+ '''
+ }
+
+ @NotYetImplemented // GROOVY-9953
+ void testInstanceOf6() {
+ assertScript '''
+ class A {
+ }
+ A test(Object x) {
+ if (x instanceof A) {
+ def y = x
+ return y
+ } else {
+ new A()
+ }
+ }
+ new A().with { assert test(it) === it }
+ '''
}
// GROOVY-9454
- void testInstanceOfOnGenericProperty() {
+ void testInstanceOf7() {
assertScript '''
interface Face {
}
@@ -179,34 +203,52 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testMultipleInstanceOf() {
+ // GROOVY-10667
+ void testInstanceOf8() {
assertScript '''
- class A {
- void foo() { println 'ok' }
+ trait Tagged {
+ String tag
}
-
- class B {
- void foo() { println 'ok' }
- void foo2() { println 'ok 2' }
+ class TaggedException extends Exception implements Tagged {
}
- def o = new A()
-
- if (o instanceof A) {
- o.foo()
+ static void doSomething1(Exception e) {
+ if (e instanceof Tagged) {
+ //println e.tag
+ doSomething2(e) // Cannot find matching method #doSomething2(Tagged)
+ }
}
-
- if (o instanceof B) {
- o.foo()
+ static void doSomething2(Exception e) {
}
- if (o instanceof A || o instanceof B) {
- o.foo()
+ doSomething1(new TaggedException(tag:'Test'))
+ '''
+ }
+
+ void testNestedInstanceOf1() {
+ assertScript '''
+ Object o
+ if (o instanceof Object) {
+ if (o instanceof String) {
+ o.toUpperCase()
+ }
}
'''
}
- void testInstanceOfInTernaryOp() {
+ void testNestedInstanceOf2() {
+ shouldFailWithMessages '''
+ Object o
+ if (o instanceof String) {
+ if (o instanceof Object) { // causes the inferred type of 'o' to be overwritten
+ o.toUpperCase()
+ }
+ }
+ ''',
+ 'Cannot find matching method java.lang.Object#toUpperCase()'
+ }
+
+ void testNestedInstanceOf3() {
assertScript '''
class A {
int foo() { 1 }
@@ -215,7 +257,125 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
int foo2() { 2 }
}
def o = new A()
- int result = o instanceof A?o.foo():(o instanceof B?o.foo2():3)
+ int result = o instanceof A ? o.foo() : (o instanceof B ? o.foo2() : 3)
+ '''
+ }
+
+ @NotYetImplemented
+ void testMultipleInstanceOf1() {
+ assertScript '''
+ class A {
+ void bar() {
+ }
+ }
+ class B {
+ void bar() {
+ }
+ }
+
+ void test(o) {
+ if (o instanceof A || o instanceof B) {
+ o.bar()
+ }
+ }
+ test(new A())
+ test(new B())
+ '''
+ }
+
+ void testMultipleInstanceOf2() {
+ assertScript '''
+ int cardinality(o) {
+ (o instanceof List || o instanceof Map ? o.size() : 1)
+ }
+ assert cardinality('') == 1
+ assert cardinality(['foo','bar']) == 2
+ assert cardinality([foo:'',bar:'']) == 2
+ '''
+ }
+
+ void testMultipleInstanceOf3() {
+ assertScript '''
+ boolean empty(o) {
+ (o instanceof List || o instanceof Map) && o.isEmpty()
+ }
+ assert !empty('')
+ assert empty([])
+ assert empty([:])
+ assert !empty(['foo'])
+ assert !empty([foo:null])
+ '''
+ }
+
+ // GROOVY-8965
+ void testMultipleInstanceOf4() {
+ ['o', '((Number) o)'].each {
+ assertScript """
+ def foo(o) {
+ if (o instanceof Integer || o instanceof Double) {
+ ${it}.floatValue() // ClassCastException
+ }
+ }
+ def bar = foo(1.1d)
+ assert bar == 1.1f
+ def baz = foo(1)
+ assert baz == 1
+ """
+ }
+ }
+
+ @NotYetImplemented
+ void testMultipleInstanceOf5() {
+ assertScript '''
+ void test(thing) {
+ if (thing instanceof Deque) {
+ thing.addFirst(1) // 'addFirst' only in Deque
+ } else if (thing instanceof Stack) {
+ thing.addElement(2) // 'addElement' only in Stack
+ }
+ if (thing instanceof Deque || thing instanceof Stack) {
+ assert thing.peek() in 1..2 // 'peek' in Deque and Stack but not LUB
+ }
+ }
+ test(new Stack())
+ test(new ArrayDeque())
+ '''
+ }
+
+ // GROOVY-10668
+ void testMultipleInstanceOf6() {
+ ['(value as String)', 'value.toString()'].each { string ->
+ assertScript """
+ def toArray(Object value) {
+ def array
+ if (value instanceof List)
+ array = value.toArray()
+ else if (value instanceof String || value instanceof GString)
+ array = ${string}.split(',')
+ else
+ throw new Exception('not supported')
+
+ return array
+ }
+ toArray([1,2,3])
+ toArray('1,2,3')
+ """
+ }
+ }
+
+ @NotYetImplemented // GROOVY-8828
+ void testMultipleInstanceOf7() {
+ assertScript '''
+ interface Foo { }
+ interface Bar { String name() }
+
+ Map<String, Foo> map = [:]
+ map.values().each { foo ->
+ if (foo instanceof Bar) {
+ String name = foo.name() // method available through Bar
+ map.put(name, foo) // second parameter expects Foo
+ }
+ }
'''
}
@@ -315,13 +475,6 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
''', 'Cannot find matching method java.lang.Object#toUpperCase()'
}
- void testShouldNotAllowDynamicVariable() {
- shouldFailWithMessages '''
- String name = 'Guillaume'
- println naamme
- ''', 'The variable [naamme] is undeclared'
- }
-
void testInstanceOfInferenceWithImplicitIt() {
assertScript '''
['a', 'b', 'c'].each {
@@ -433,6 +586,14 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ void testShouldNotAllowDynamicVariable() {
+ shouldFailWithMessages '''
+ String name = 'Guillaume'
+ println naamme
+ ''',
+ 'The variable [naamme] is undeclared'
+ }
+
void testShouldNotFailWithWith() {
assertScript '''
class A {
@@ -515,7 +676,8 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
a.with { String it ->
it.x = 2 // should be recognized as a.x at compile time
}
- ''', 'Expected parameter of type A but got java.lang.String'
+ ''',
+ 'Expected parameter of type A but got java.lang.String'
}
void testShouldNotFailWithInheritanceAndWith() {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
index afe168980f..c14d7ba887 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
@@ -18,10 +18,26 @@
*/
package org.codehaus.groovy.classgen.asm.sc
+import groovy.transform.NotYetImplemented
import groovy.transform.stc.TypeInferenceSTCTest
/**
* Unit tests for static compilation : type inference.
*/
class TypeInferenceStaticCompileTest extends TypeInferenceSTCTest implements StaticCompilationTestSupport {
+
+ @Override @NotYetImplemented
+ void testMultipleInstanceOf2() {
+ super.testMultipleInstanceOf2()
+ }
+
+ @Override @NotYetImplemented
+ void testMultipleInstanceOf3() {
+ super.testMultipleInstanceOf3()
+ }
+
+ @Override @NotYetImplemented
+ void testMultipleInstanceOf4() {
+ super.testMultipleInstanceOf4()
+ }
}