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 22:21:10 UTC
[groovy] 04/04: GROOVY-6802, GROOVY-6803, GROOVY-8136: STC: more list/map assign checks
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
commit 500ebf9708381a5b697fa6fa8005ea272f6645fd
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jan 26 15:46:23 2023 -0600
GROOVY-6802, GROOVY-6803, GROOVY-8136: STC: more list/map assign checks
2_5_X backport
---
.../transform/stc/StaticTypeCheckingVisitor.java | 68 +++++++++--------
.../stc/ArraysAndCollectionsSTCTest.groovy | 9 +++
.../groovy/transform/stc/CoercionSTCTest.groovy | 88 +++++++++++++++++++++-
.../transform/stc/ConstructorsSTCTest.groovy | 4 +-
.../asm/sc/CoercionStaticCompileTests.groovy | 27 +++++++
5 files changed, 157 insertions(+), 39 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 29f1a86174..e560554a7a 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1262,38 +1262,38 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void addListAssignmentConstructorErrors(final ClassNode leftRedirect, final ClassNode leftExpressionType, final ClassNode inferredRightExpressionType, final Expression rightExpression, final Expression assignmentExpression) {
+ if (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type)) return; // GROOVY-6802, GROOVY-6803
// 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 (!implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)
&& (!leftRedirect.isAbstract() || leftRedirect.isArray())
&& !ArrayList_TYPE.isDerivedFrom(leftRedirect) && !LinkedHashSet_TYPE.isDerivedFrom(leftRedirect)) {
- ClassNode[] args = getArgumentTypes(args(((ListExpression) rightExpression).getExpressions()));
- MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, args, assignmentExpression);
+ ClassNode[] types = getArgumentTypes(args(((ListExpression)rightExpression).getExpressions()));
+ MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, types, assignmentExpression);
if (methodNode != null) {
rightExpression.putNodeMetaData(StaticTypesMarker.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)) {
if (!extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression);
}
}
}
- private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
- if ((leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
- || leftRedirect.equals(OBJECT_TYPE) || implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
+ private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final MapExpression rightExpression) {
+ if (!isConstructorAbbreviation(leftRedirect, rightExpression)
+ // GROOVY-6802, GROOVY-6803: Object, String or [Bb]oolean target
+ || (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type))) {
return;
}
// groovy constructor shorthand: A a = [x:2, y:3]
- ClassNode[] argTypes = getArgumentTypes(args(rightExpression));
+ ClassNode[] argTypes = {getType(rightExpression)};
checkGroovyStyleConstructor(leftRedirect, argTypes, rightExpression);
// perform additional type checking on arguments
- MapExpression mapExpression = (MapExpression) rightExpression;
- checkGroovyConstructorMap(leftExpression, leftRedirect, mapExpression);
+ checkGroovyConstructorMap(leftExpression, leftRedirect, rightExpression);
}
private void checkTypeGenerics(final ClassNode leftExpressionType, final ClassNode rightExpressionType, final Expression rightExpression) {
@@ -1316,7 +1316,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
private static boolean isConstructorAbbreviation(final ClassNode leftType, final Expression rightExpression) {
if (rightExpression instanceof ListExpression) {
return !(ArrayList_TYPE.isDerivedFrom(leftType) || ArrayList_TYPE.implementsInterface(leftType)
- || LinkedHashSet_TYPE.isDerivedFrom(leftType) || LinkedHashSet_TYPE.implementsInterface(leftType));
+ || LinkedHashSet_TYPE.isDerivedFrom(leftType) || LinkedHashSet_TYPE.implementsInterface(leftType));
+ }
+ if (rightExpression instanceof MapExpression) {
+ return !(LinkedHashMap_TYPE.isDerivedFrom(leftType) || LinkedHashMap_TYPE.implementsInterface(leftType));
}
return false;
}
@@ -1345,7 +1348,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (rightExpression instanceof ListExpression) {
addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
} else if (rightExpression instanceof MapExpression) {
- addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+ addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, (MapExpression)rightExpression);
}
if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)
&& !isConstructorAbbreviation(leftExpressionType, rightExpression)) {
@@ -1419,29 +1422,28 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
* @param arguments the constructor arguments
*/
protected MethodNode checkGroovyStyleConstructor(final ClassNode node, final ClassNode[] arguments, final ASTNode source) {
- if (node.equals(ClassHelper.OBJECT_TYPE) || node.equals(ClassHelper.DYNAMIC_TYPE)) {
- // in that case, we are facing a list constructor assigned to a def or object
- return null;
+ if (node.equals(DYNAMIC_TYPE) || node.equals(OBJECT_TYPE)) {
+ return null; // list or map assigned to def or Object
}
- List<ConstructorNode> constructors = node.getDeclaredConstructors();
+ List<? extends MethodNode> constructors = node.getDeclaredConstructors();
if (constructors.isEmpty() && arguments.length == 0) {
return null;
}
- List<MethodNode> constructorList = findMethod(node, "<init>", arguments);
- if (constructorList.isEmpty()) {
- if (isBeingCompiled(node) && arguments.length == 1 && LinkedHashMap_TYPE.equals(arguments[0])) {
+ constructors = findMethod(node, "<init>", arguments);
+ if (constructors.isEmpty()) {
+ if (isBeingCompiled(node) && !node.isInterface() && arguments.length == 1 && arguments[0].equals(LinkedHashMap_TYPE)) {
// there will be a default map constructor added later
Parameter[] params = {new Parameter(LinkedHashMap_TYPE, "args")};
return new ConstructorNode(Opcodes.ACC_PUBLIC, params, ClassNode.EMPTY_ARRAY, EmptyStatement.INSTANCE);
} else {
- addStaticTypeError("No matching constructor found: " + node + toMethodParametersString("<init>", arguments), source);
+ addStaticTypeError("No matching constructor found: " + prettyPrintTypeName(node) + toMethodParametersString("", arguments), source);
return null;
}
- } else if (constructorList.size() > 1) {
- addStaticTypeError("Ambiguous constructor call " + node + toMethodParametersString("<init>", arguments), source);
+ } else if (constructors.size() > 1) {
+ addStaticTypeError("Ambiguous constructor call " + prettyPrintTypeName(node) + toMethodParametersString("", arguments), source);
return null;
}
- return constructorList.get(0);
+ return constructors.get(0);
}
/**
@@ -1992,13 +1994,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
ClassNode forLoopVariableType = forLoop.getVariableType();
ClassNode componentType;
- if (Character_TYPE.equals(ClassHelper.getWrapper(forLoopVariableType)) && STRING_TYPE.equals(collectionType)) {
+ if (Character_TYPE.equals(getWrapper(forLoopVariableType)) && STRING_TYPE.equals(collectionType)) {
// we allow auto-coercion here
componentType = forLoopVariableType;
} else {
componentType = inferLoopElementType(collectionType);
}
- if (ClassHelper.getUnwrapper(componentType) == forLoopVariableType) {
+ if (getUnwrapper(componentType) == forLoopVariableType) {
// prefer primitive type over boxed type
componentType = forLoopVariableType;
}
@@ -2214,7 +2216,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode type = getType(expression);
ClassNode typeRe = type.redirect();
ClassNode resultType;
- if (isDoubleCategory(ClassHelper.getUnwrapper(typeRe))) {
+ if (isDoubleCategory(getUnwrapper(typeRe))) {
resultType = type;
} else if (typeRe == ArrayList_TYPE) {
resultType = ArrayList_TYPE;
@@ -2446,7 +2448,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
if (tempTypes != null && !tempTypes.isEmpty()) {
List<ClassNode> types = new ArrayList<>(tempTypes.size() + 1);
- if (expressionType != null && !expressionType.equals(ClassHelper.OBJECT_TYPE) // GROOVY-7333
+ if (expressionType != null && !expressionType.equals(OBJECT_TYPE) // GROOVY-7333
&& noneMatch(tempTypes, expressionType)) { // GROOVY-9769
types.add(expressionType);
}
@@ -2506,7 +2508,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
super.visitClosureExpression(expression);
typeCheckingContext.delegationMetadata = typeCheckingContext.delegationMetadata.getParent();
- returnAdder.visitMethod(new MethodNode("dummy", 0, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, expression.getCode()));
+ returnAdder.visitMethod(new MethodNode("dummy", 0, OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, expression.getCode()));
TypeCheckingContext.EnclosingClosure enclosingClosure = typeCheckingContext.getEnclosingClosure();
if (!enclosingClosure.getReturnTypes().isEmpty()) {
@@ -2933,7 +2935,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void checkNamedParamsAnnotation(Parameter param, MapExpression args) {
- if (!param.getType().isDerivedFrom(ClassHelper.MAP_TYPE)) return;
+ if (!param.getType().isDerivedFrom(MAP_TYPE)) return;
List<MapEntryExpression> entryExpressions = args.getMapEntryExpressions();
Map<Object, Expression> entries = new LinkedHashMap<Object, Expression>();
for (MapEntryExpression entry : entryExpressions) {
@@ -3339,7 +3341,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
Expression type = annotation.getMember("type");
Integer stInt = Closure.OWNER_FIRST;
if (strategy != null) {
- stInt = (Integer) evaluateExpression(castX(ClassHelper.Integer_TYPE, strategy), typeCheckingContext.source.getConfiguration());
+ stInt = (Integer) evaluateExpression(castX(Integer_TYPE, strategy), typeCheckingContext.source.getConfiguration());
}
if (value instanceof ClassExpression && !value.getType().equals(DELEGATES_TO_TARGET)) {
if (genericTypeIndex != null) {
@@ -4696,7 +4698,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
protected boolean areCategoryMethodCalls(final List<MethodNode> foundMethods, final String name, final ClassNode[] args) {
boolean category = false;
- if ("use".equals(name) && args != null && args.length == 2 && args[1].equals(ClassHelper.CLOSURE_TYPE)) {
+ if ("use".equals(name) && args != null && args.length == 2 && args[1].equals(CLOSURE_TYPE)) {
category = true;
for (MethodNode method : foundMethods) {
if (!(method instanceof ExtensionMethodNode) || !((ExtensionMethodNode) method).getExtensionMethodNode().getDeclaringClass().equals(DGM_CLASSNODE)) {
@@ -5087,7 +5089,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return ((Variable) exp).getOriginType();
}
if (exp instanceof RangeExpression) {
- ClassNode plain = ClassHelper.RANGE_TYPE.getPlainNodeReference();
+ ClassNode plain = RANGE_TYPE.getPlainNodeReference();
RangeExpression re = (RangeExpression) exp;
ClassNode fromType = getType(re.getFrom());
ClassNode toType = getType(re.getTo());
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index 608fa048b1..4bf8c427df 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -799,4 +799,13 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
assert set.last() == 3
'''
}
+
+ // GROOVY-8136
+ void testInterfaceThatExtendsMapInitializedByMapLiteral() {
+ shouldFailWithMessages '''
+ interface MVM<K, V> extends Map<K, List<V>> { }
+ MVM map = [:] // no STC error; fails at runtime
+ ''',
+ 'No matching constructor found'
+ }
}
diff --git a/src/test/groovy/transform/stc/CoercionSTCTest.groovy b/src/test/groovy/transform/stc/CoercionSTCTest.groovy
index 0bbad5657d..a8fef776f7 100644
--- a/src/test/groovy/transform/stc/CoercionSTCTest.groovy
+++ b/src/test/groovy/transform/stc/CoercionSTCTest.groovy
@@ -22,9 +22,9 @@ package groovy.transform.stc
* Unit tests for static type checking : coercions.
*/
class CoercionSTCTest extends StaticTypeCheckingTestCase {
+
void testCoerceToArray() {
assertScript '''
- def m() {
try {
throw new Exception()
} catch (Throwable t) {
@@ -38,10 +38,90 @@ class CoercionSTCTest extends StaticTypeCheckingTestCase {
// ...
clean = newTrace.toArray(newTrace as StackTraceElement[])
}
- }
+ '''
+ }
- m()
+ // GROOVY-6802
+ void testCoerceToBool1() {
+ assertScript '''
+ boolean b = [new Object()]
+ assert b
+ '''
+ assertScript '''
+ boolean b = [false]
+ assert b
+ '''
+ assertScript '''
+ boolean b = [true]
+ assert b
+ '''
+ assertScript '''
+ boolean b = ['x']
+ assert b
+ '''
+ assertScript '''
+ boolean b = [:]
+ assert !b
+ '''
+ assertScript '''
+ boolean b = []
+ assert !b
'''
}
-}
+ void testCoerceToBool2() {
+ assertScript '''
+ Boolean b = [new Object()]
+ assert b
+ '''
+ assertScript '''
+ Boolean b = [false]
+ assert b
+ '''
+ assertScript '''
+ Boolean b = [true]
+ assert b
+ '''
+ assertScript '''
+ Boolean b = ['x']
+ assert b
+ '''
+ assertScript '''
+ Boolean b = [:]
+ assert !b
+ '''
+ assertScript '''
+ Boolean b = []
+ assert !b
+ '''
+ }
+
+ void testCoerceToClass() {
+ assertScript '''
+ Class c = 'java.lang.String'
+ assert String.class
+ '''
+ shouldFailWithMessages '''
+ Class c = []
+ ''', 'No matching constructor found: java.lang.Class()'
+ shouldFailWithMessages '''
+ Class c = [:]
+ ''', 'No matching constructor found: java.lang.Class(java.util.LinkedHashMap)'
+ }
+
+ // GROOVY-6803
+ void testCoerceToString() {
+ assertScript '''
+ String s = ['x']
+ assert s == '[x]'
+ '''
+ assertScript '''
+ String s = [:]
+ assert s == '{}' || s == '[:]'
+ '''
+ assertScript '''
+ String s = []
+ assert s == '[]'
+ '''
+ }
+}
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index dacfebd309..2b4056de5e 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -37,7 +37,7 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
import java.awt.Dimension
Dimension d = [100]
- ''', 'No matching constructor found: java.awt.Dimension<init>(int)'
+ ''', 'No matching constructor found: java.awt.Dimension(int)'
}
void testWrongNumberOfArgumentsWithDefaultConstructor() {
@@ -62,7 +62,7 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
import java.awt.Dimension
Dimension d = ['100','200']
- ''', 'No matching constructor found: java.awt.Dimension<init>(java.lang.String, java.lang.String)'
+ ''', 'No matching constructor found: java.awt.Dimension(java.lang.String, java.lang.String)'
}
void testConstructFromListAndVariables() {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/CoercionStaticCompileTests.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/CoercionStaticCompileTests.groovy
new file mode 100644
index 0000000000..7506ceb415
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/CoercionStaticCompileTests.groovy
@@ -0,0 +1,27 @@
+/*
+ * 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 org.codehaus.groovy.classgen.asm.sc
+
+import groovy.transform.stc.CoercionSTCTest
+
+/**
+ * Unit tests for static compilation : coercions.
+ */
+final class CoercionStaticCompileTests extends CoercionSTCTest implements StaticCompilationTestSupport {
+}