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 2023/01/26 18:07:02 UTC
[groovy] 01/04: GROOVY-10002: STC: fix checks for list literal assignment
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 9e9843fcbc6b8519f1829b3619c5c70f704f8307
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
---
.../transform/stc/StaticTypeCheckingVisitor.java | 49 +++++----
src/test/groovy/bugs/Groovy6086Bug.groovy | 69 ------------
.../stc/ArraysAndCollectionsSTCTest.groovy | 61 ++++++++++-
.../InheritConstructorsTransformTest.groovy | 119 +++++++++++----------
4 files changed, 144 insertions(+), 154 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 38ad511812..bfa0849d30 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -312,7 +312,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};
@@ -329,8 +328,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
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 NAMED_PARAM_CLASSNODE = ClassHelper.make(NamedParam.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.make(Iterable.class);
+ private static final ClassNode SET_TYPE = ClassHelper.make(Set.class);
public static final Statement GENERATED_EMPTY_STATEMENT = EmptyStatement.INSTANCE;
@@ -1272,11 +1273,10 @@ 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);
}
@@ -1289,9 +1289,8 @@ 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())
- || (isWildcardLeftHandSide(leftRedirect) && !CLASS_Type.equals(leftRedirect)) // GROOVY-6802, GROOVY-6803
+ if ((leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
+ || (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type)) // GROOVY-6802, GROOVY-6803
|| implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
return;
}
@@ -1321,6 +1320,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;
@@ -1342,9 +1346,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else {
ClassNode lTypeRedirect = leftExpressionType.redirect();
addPrecisionErrors(lTypeRedirect, leftExpressionType, rTypeAdjusted, rightExpression);
- addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
- addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
- if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)) {
+ if (rightExpression instanceof ListExpression) {
+ addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
+ } else if (rightExpression instanceof MapExpression) {
+ addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+ }
+ if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)
+ && !isConstructorAbbreviation(leftExpressionType, rightExpression)) {
checkTypeGenerics(leftExpressionType, rTypeAdjusted, rightExpression);
}
}
@@ -4501,22 +4509,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(rightRedirect, Collection_TYPE)) {
- if (leftRedirect.isArray()) {
- return leftRedirect;
- }
- if (isOrImplements(leftRedirect, Collection_TYPE) &&
- rightExpression instanceof ListExpression && isEmptyCollection(rightExpression)) {
- 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 49eb6f4692..0000000000
--- 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 8cd4a3bbb2..3d7381db7d 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -247,7 +247,6 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
}
void testInlineMap() {
-
assertScript '''
Map map = [a:1, b:2]
'''
@@ -397,7 +396,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 {
@@ -749,4 +748,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', ' to variable of type java.util.Deque <String>'
+
+ shouldFailWithMessages '''
+ Deque<String> deque = []
+ ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
+
+ shouldFailWithMessages '''
+ Queue<String> queue = []
+ ''', 'Cannot assign value of type java.util.List', ' 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 c3f1aea32e..91f0caa547 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,88 +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 <RoundingMode>#<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 <RoundingMode>#<init>(java.util.Set <RoundingMode>) with arguments [java.util.HashSet <Date>]')
}