You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2021/03/28 12:51:09 UTC
[groovy] 01/03: GROOVY-10002: STC: fix checks for list literal
assignment
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 017e0a7141b7940c606e0fa53653219302dcadbf
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Mar 27 00:29:54 2021 -0500
GROOVY-10002: STC: fix checks for list literal assignment
- empty and non-empty list literals can initialize Set variables
- list literal generics checking skipped when constructor may be used
- list literal as constructor arguments does not apply to abstract types
---
.../java/org/codehaus/groovy/ast/ClassHelper.java | 3 +
.../transform/stc/StaticTypeCheckingVisitor.java | 53 ++++-----
src/test/groovy/bugs/Groovy6086Bug.groovy | 69 ------------
.../stc/ArraysAndCollectionsSTCTest.groovy | 62 ++++++++++-
.../InheritConstructorsTransformTest.groovy | 118 +++++++++++----------
5 files changed, 150 insertions(+), 155 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index 7d1c7d5..7c76c02 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -67,6 +67,7 @@ import java.math.BigInteger;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.regex.Pattern;
/**
@@ -140,10 +141,12 @@ public class ClassHelper {
// uncached constants
MAP_TYPE = makeWithoutCaching(Map.class),
+ SET_TYPE = makeWithoutCaching(Set.class),
LIST_TYPE = makeWithoutCaching(List.class),
Enum_Type = makeWithoutCaching(Enum.class),
CLASS_Type = makeWithoutCaching(Class.class),
TUPLE_TYPE = makeWithoutCaching(Tuple.class),
+ ITERABLE_TYPE = makeWithoutCaching(Iterable.class),
REFERENCE_TYPE = makeWithoutCaching(Reference.class),
COMPARABLE_TYPE = makeWithoutCaching(Comparable.class),
GROOVY_OBJECT_TYPE = makeWithoutCaching(GroovyObject.class),
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 b33a7f7..29359fe 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -159,6 +159,7 @@ import static org.codehaus.groovy.ast.ClassHelper.Number_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.PATTERN_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.RANGE_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.SET_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Short_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.TUPLE_CLASSES;
@@ -305,7 +306,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
private static final AtomicLong UNIQUE_LONG = new AtomicLong();
protected static final Object ERROR_COLLECTOR = ErrorCollector.class;
- protected static final ClassNode ITERABLE_TYPE = ClassHelper.make(Iterable.class);
protected static final List<MethodNode> EMPTY_METHODNODE_LIST = Collections.emptyList();
protected static final ClassNode TYPECHECKED_CLASSNODE = ClassHelper.make(TypeChecked.class);
protected static final ClassNode[] TYPECHECKING_ANNOTATIONS = new ClassNode[]{TYPECHECKED_CLASSNODE};
@@ -321,8 +321,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
protected static final ClassNode LINKEDHASHMAP_CLASSNODE = ClassHelper.make(LinkedHashMap.class);
protected static final ClassNode CLOSUREPARAMS_CLASSNODE = ClassHelper.make(ClosureParams.class);
protected static final ClassNode NAMED_PARAMS_CLASSNODE = ClassHelper.make(NamedParams.class);
- protected static final ClassNode MAP_ENTRY_TYPE = ClassHelper.make(Map.Entry.class);
protected static final ClassNode ENUMERATION_TYPE = ClassHelper.make(Enumeration.class);
+ protected static final ClassNode MAP_ENTRY_TYPE = ClassHelper.make(Map.Entry.class);
+ protected static final ClassNode ITERABLE_TYPE = ClassHelper.ITERABLE_TYPE;
public static final Statement GENERATED_EMPTY_STATEMENT = EmptyStatement.INSTANCE;
@@ -1216,16 +1217,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
// if left type is not a list but right type is a list, then we're in the case of a groovy
// constructor type : Dimension d = [100,200]
// In that case, more checks can be performed
- if (rightExpression instanceof ListExpression && !implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)) {
- ArgumentListExpression argList = args(((ListExpression) rightExpression).getExpressions());
- ClassNode[] args = getArgumentTypes(argList);
- MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, args, assignmentExpression);
+ if (!implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)
+ && (!leftRedirect.isAbstract() || leftRedirect.isArray())) {
+ ClassNode[] types = getArgumentTypes(args(((ListExpression) rightExpression).getExpressions()));
+ MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, types, assignmentExpression);
if (methodNode != null) {
rightExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, methodNode);
}
- } else if (!implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, leftRedirect)
- && implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, LIST_TYPE)
- && !isWildcardLeftHandSide(leftExpressionType)) {
+ } else if (implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, LIST_TYPE)
+ && !implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, leftRedirect)
+ && !isWildcardLeftHandSide(leftRedirect)) {
if (!extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression);
}
@@ -1233,7 +1234,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
- if (!(rightExpression instanceof MapExpression) || (leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
+ if ((leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
|| leftRedirect.equals(OBJECT_TYPE) || implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
return;
}
@@ -1271,6 +1272,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return false;
}
+ private static boolean isConstructorAbbreviation(final ClassNode leftType, final Expression rightExpression) {
+ return (rightExpression instanceof ListExpression && !(leftType.equals(LIST_TYPE) || leftType.equals(SET_TYPE)
+ || leftType.equals(ITERABLE_TYPE) || leftType.equals(Collection_TYPE)));
+ }
+
protected void typeCheckAssignment(final BinaryExpression assignmentExpression, final Expression leftExpression, final ClassNode leftExpressionType, final Expression rightExpression, final ClassNode rightExpressionType) {
if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
@@ -1292,9 +1298,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else {
ClassNode lTypeRedirect = leftExpressionType.redirect();
addPrecisionErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression);
- addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
- addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
- if (!hasGStringStringError(leftExpressionType, rTypeWrapped, rightExpression)) {
+ if (rightExpression instanceof ListExpression) {
+ addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
+ } else if (rightExpression instanceof MapExpression) {
+ addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+ }
+ if (!hasGStringStringError(leftExpressionType, rTypeWrapped, rightExpression)
+ && !isConstructorAbbreviation(leftExpressionType, rightExpression)) {
checkTypeGenerics(leftExpressionType, rTypeWrapped, rightExpression);
}
}
@@ -4332,7 +4342,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
if (leftExpression instanceof VariableExpression) {
- ClassNode initialType = getOriginalDeclarationType(leftExpression).redirect();
+ ClassNode initialType = getOriginalDeclarationType(leftExpression);
if (isPrimitiveType(right) && initialType.isDerivedFrom(Number_TYPE)) {
return getWrapper(right);
@@ -4343,22 +4353,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
// as anything can be assigned to a String, Class or [Bb]oolean, return the left type instead
- if (STRING_TYPE.equals(initialType)
- || CLASS_Type.equals(initialType)
- || Boolean_TYPE.equals(initialType)
- || boolean_TYPE.equals(initialType)) {
+ if (isWildcardLeftHandSide(initialType) && !initialType.equals(OBJECT_TYPE)) {
return initialType;
}
}
- if (isOrImplements(leftRedirect, Collection_TYPE) && isEmptyList(rightExpression)) {
- return left;
- }
- if (isOrImplements(leftRedirect, MAP_TYPE) && isEmptyMap(rightExpression)) {
- return left;
- }
- if (leftRedirect.isArray() && isOrImplements(right, Collection_TYPE)) {
- return left;
+ if (rightExpression instanceof ListExpression && leftRedirect.equals(SET_TYPE)) {
+ return GenericsUtils.parameterizeType(right, leftRedirect.getPlainNodeReference());
}
return right;
diff --git a/src/test/groovy/bugs/Groovy6086Bug.groovy b/src/test/groovy/bugs/Groovy6086Bug.groovy
deleted file mode 100644
index 49eb6f4..0000000
--- a/src/test/groovy/bugs/Groovy6086Bug.groovy
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package groovy.bugs
-
-import groovy.test.GroovyTestCase
-import org.codehaus.groovy.control.CompilePhase
-import org.codehaus.groovy.control.CompilerConfiguration
-import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
-
-class Groovy6086Bug extends GroovyTestCase {
-
- // Note that this unit test reproduces the code that we can
- // see on the Grails build. However, it never managed to reproduce
- // the issue so the latter has been fixed independently.
- void testGroovy6086() {
-
- def config = new CompilerConfiguration()
- config.with {
- targetDirectory = File.createTempDir()
- jointCompilationOptions = [stubDir: File.createTempDir()]
- }
-
- try {
- def unit = new JavaAwareCompilationUnit(config, null, new GroovyClassLoader(getClass().classLoader))
-
- unit.addSource('Boo.java', 'interface Boo {}')
- unit.addSource('Wrapper.groovy', '''
- import groovy.transform.CompileStatic
-
- @CompileStatic
- class Wrapper {
- private Map cache
- Boo[] boos() {
- Boo[] locations = (Boo[]) cache.a
- if (locations == null) {
- if (true) {
- locations = [].collect { it }
- }
- else {
- locations = [] as Boo[]
- }
- }
- return locations
- }
- }
- ''')
- unit.compile(CompilePhase.INSTRUCTION_SELECTION.phaseNumber)
- } finally {
- config.targetDirectory.deleteDir()
- config.jointCompilationOptions.stubDir.deleteDir()
- }
- }
-}
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index fb0d2ab..7f58531 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -18,7 +18,6 @@
*/
package groovy.transform.stc
-
/**
* Unit tests for static type checking : arrays and collections.
*/
@@ -207,7 +206,6 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
}
void testInlineMap() {
-
assertScript '''
Map map = [a:1, b:2]
'''
@@ -319,7 +317,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
class Foo {
def say() {
- FooAnother foo1 = new Foo[13] // but FooAnother foo1 = new Foo() reports a STC Error
+ FooAnother foo1 = new Foo[13] // but FooAnother foo1 = new Foo() reports a STC error
}
}
class FooAnother {
@@ -593,4 +591,62 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
assert meth().toSet() == ['pls', 'bar'].toSet()
'''
}
+
+ void testAbstractTypeInitializedByListLiteral() {
+ shouldFailWithMessages '''
+ abstract class A {
+ A(int n) {}
+ }
+ A a = [1]
+ ''', 'Cannot assign value of type java.util.List <java.lang.Integer> to variable of type A'
+ }
+
+ void testCollectionTypesInitializedByListLiteral1() {
+ assertScript '''
+ Set<String> set = []
+ set << 'foo'
+ set << 'bar'
+ set << 'foo'
+ assert set.size() == 2
+ '''
+ }
+
+ // GROOVY-10002
+ void testCollectionTypesInitializedByListLiteral2() {
+ assertScript '''
+ Set<String> set = ['foo', 'bar', 'foo']
+ assert set.size() == 2
+ '''
+
+ assertScript '''
+ ArrayDeque<String> deque = [123] // ArrayDeque(int numElements)
+ '''
+ }
+
+ // GROOVY-10002
+ void testCollectionTypesInitializedByListLiteral3() {
+ shouldFailWithMessages '''
+ Set<String> set = [1,2,3]
+ ''', 'Cannot assign java.util.Set <java.lang.Integer> to: java.util.Set <String>'
+
+ shouldFailWithMessages '''
+ Iterable<String> iter = [1,2,3]
+ ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.lang.Iterable <String>'
+
+ shouldFailWithMessages '''
+ Collection<String> coll = [1,2,3]
+ ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.util.Collection <String>'
+
+ shouldFailWithMessages '''
+ Deque<String> deque = [""]
+ ''', 'Cannot assign value of type java.util.List <java.lang.String> to variable of type java.util.Deque <String>'
+
+ shouldFailWithMessages '''
+ Deque<String> deque = []
+ ''', 'Cannot assign value of type java.util.List <java.lang.String> to variable of type java.util.Deque <String>'
+
+ shouldFailWithMessages '''
+ Queue<String> queue = []
+ ''', 'Cannot assign value of type java.util.List <java.lang.String> to variable of type java.util.Queue <String>'
+ }
}
diff --git a/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy b/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
index 30c52fa..7090068 100644
--- a/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
@@ -23,16 +23,16 @@ import groovy.test.GroovyShellTestCase
class InheritConstructorsTransformTest extends GroovyShellTestCase {
void testStandardCase() {
- assertScript """
+ assertScript '''
import groovy.transform.InheritConstructors
@InheritConstructors class CustomException extends RuntimeException { }
def ce = new CustomException('foo')
assert ce.message == 'foo'
- """
+ '''
}
void testOverrideCase() {
- assertScript """
+ assertScript '''
import groovy.transform.InheritConstructors
@InheritConstructors
class CustomException2 extends RuntimeException {
@@ -42,11 +42,11 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
assert ce.message == 'bar'
ce = new CustomException2('foo')
assert ce.message == 'foo'
- """
+ '''
}
void testChainedCase() {
- assertScript """
+ assertScript '''
import groovy.transform.InheritConstructors
@InheritConstructors
class CustomException5 extends CustomException4 {}
@@ -56,11 +56,12 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
class CustomException4 extends CustomException3 {}
def ce = new CustomException5('baz')
assert ce.message == 'baz'
- """
+ '''
}
- void testCopyAnnotations_groovy7059() {
- assertScript """
+ // GROOVY-7059
+ void testCopyAnnotations() {
+ assertScript '''
import java.lang.annotation.*
import groovy.transform.InheritConstructors
@@ -109,11 +110,11 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
break
}
}
- """
+ '''
}
void testInnerClassUsage() {
- assertScript """
+ assertScript '''
import groovy.transform.InheritConstructors
@InheritConstructors
class Outer extends RuntimeException {
@@ -138,87 +139,90 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
assert o.message == 'baz'
o.test()
new Outer2().test()
- """
+ '''
}
- void testParametersWithGenericsAndCompileStatic_GROOVY6874() {
- assertScript """
+ // GROOVY-6874
+ void testParametersWithGenericsAndCompileStatic() {
+ assertScript '''
import groovy.transform.*
import java.math.RoundingMode
@CompileStatic
abstract class BasePublisher<T, U> {
- final Deque<T> items
- private U mode
- BasePublisher(Deque<T> items) { this.items = items }
- BasePublisher(U mode) {
- this.mode = mode
- this.items = []
- }
- BasePublisher(Set<U> modes) { this(modes[0]) }
- void publish(T item) { items.addFirst(item) }
- void init(U mode) { this.mode = mode }
- String toString() { items.join('|') + "|" + mode.toString() }
+ final Deque<T> items
+ private U mode
+ BasePublisher(Deque<T> items) { this.items = items }
+ BasePublisher(U mode) {
+ this.mode = mode
+ this.items = new ArrayDeque<>()
+ }
+ BasePublisher(Set<U> modes) { this(modes[0]) }
+ void publish(T item) { items.addFirst(item) }
+ void init(U mode) { this.mode = mode }
+ String toString() { items.join('|') + "|" + mode.toString() }
}
@CompileStatic @InheritConstructors
class OrderPublisher<V> extends BasePublisher<Integer, V> {
- static OrderPublisher make() {
- new OrderPublisher<RoundingMode>(new LinkedList<Integer>())
- }
- void foo() { publish(3) }
- void bar(V mode) { init(mode) }
- void baz() {
- new OrderPublisher<RoundingMode>(RoundingMode.UP)
- new OrderPublisher<RoundingMode>(new HashSet<RoundingMode>())
- }
+ static OrderPublisher make() {
+ new OrderPublisher<RoundingMode>(new LinkedList<Integer>())
+ }
+ void foo() { publish(3) }
+ void bar(V mode) { init(mode) }
+ void baz() {
+ new OrderPublisher<RoundingMode>(RoundingMode.UP)
+ new OrderPublisher<RoundingMode>(new HashSet<RoundingMode>())
+ }
}
def op = OrderPublisher.make()
op.foo()
op.bar(RoundingMode.DOWN)
+ op.baz()
assert op.toString() == '3|DOWN'
- """
+ '''
}
- void testParametersWithGenericsAndCompileStatic_errors_GROOVY6874() {
- def message = shouldFail """
+ // GROOVY-6874
+ void testParametersWithGenericsAndCompileStatic_errors() {
+ def message = shouldFail '''
import groovy.transform.*
import java.math.RoundingMode
@CompileStatic
abstract class BasePublisher<T, U> {
- final Deque<T> items
- private U mode
- BasePublisher(Deque<T> items) { this.items = items }
- BasePublisher(U mode) {
- this.mode = mode
- this.items = []
- }
- BasePublisher(Set<U> modes) { this(modes[0]) }
- void publish(T item) { items.addFirst(item) }
- void init(U mode) { this.mode = mode }
- String toString() { items.join('|') + "|" + mode.toString() }
+ final Deque<T> items
+ private U mode
+ BasePublisher(Deque<T> items) { this.items = items }
+ BasePublisher(U mode) {
+ this.mode = mode
+ this.items = new ArrayDeque<>()
+ }
+ BasePublisher(Set<U> modes) { this(modes[0]) }
+ void publish(T item) { items.addFirst(item) }
+ void init(U mode) { this.mode = mode }
+ String toString() { items.join('|') + "|" + mode.toString() }
}
@CompileStatic @InheritConstructors
class OrderPublisher<V> extends BasePublisher<Integer, V> {
- static OrderPublisher make() {
- new OrderPublisher<RoundingMode>(new LinkedList<String>())
- }
- void foo() { publish(3) }
- void bar(V mode) { init(mode) }
- void baz() {
- new OrderPublisher<RoundingMode>(new Date())
- new OrderPublisher<RoundingMode>(new HashSet<Date>())
- }
+ static OrderPublisher make() {
+ new OrderPublisher<RoundingMode>(new LinkedList<String>())
+ }
+ void foo() { publish(3) }
+ void bar(V mode) { init(mode) }
+ void baz() {
+ new OrderPublisher<RoundingMode>(new Date())
+ new OrderPublisher<RoundingMode>(new HashSet<Date>())
+ }
}
def op = OrderPublisher.make()
op.foo()
op.bar(RoundingMode.DOWN)
assert op.toString() == '3|DOWN'
- """
+ '''
assert message.contains('Cannot call OrderPublisher#<init>(java.util.Deque <java.lang.Integer>) with arguments [java.util.LinkedList <String>]')
assert message.contains('Cannot find matching method OrderPublisher#<init>(java.util.Date)')
assert message.contains('Cannot call OrderPublisher#<init>(java.util.Set <RoundingMode>) with arguments [java.util.HashSet <Date>]')