Skip to content

Commit

Permalink
GROOVY-11309: SC: skip ScriptBytecodeAdapter.createList for empty list
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed Oct 6, 2024
1 parent 485e089 commit 1bb575f
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1352,23 +1352,30 @@ private void addPrecisionErrors(final ClassNode leftRedirect, final ClassNode lh
}

private void addListAssignmentConstructorErrors(final ClassNode leftRedirect, final ClassNode leftExpressionType, final ClassNode inferredRightExpressionType, final Expression rightExpression, final Expression assignmentExpression) {
if (isWildcardLeftHandSide(leftRedirect) && !isClassType(leftRedirect)) 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[] types = getArgumentTypes(args(((ListExpression) rightExpression).getExpressions()));
ClassNode concreteType;
List<Expression> expressions = ((ListExpression) rightExpression).getExpressions();
if (implementsInterfaceOrIsSubclassOf((concreteType = ArrayList_TYPE), leftRedirect)
|| implementsInterfaceOrIsSubclassOf((concreteType = LinkedHashSet_TYPE), leftRedirect)) {
// GROOVY-11309: (? super ArrayList or LinkedHashSet) x = [ ]
if (expressions.isEmpty()) { // skip SBA.createList for empty
rightExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET,
concreteType.getDeclaredConstructor(Parameter.EMPTY_ARRAY));
}
} else if (isWildcardLeftHandSide(leftRedirect) && !isClassType(leftRedirect)) {
// GROOVY-6802, GROOVY-6803: ([Bb]oolean or String) x = [ ]
} else if (leftRedirect.isArray() || !leftRedirect.isAbstract()) {
// 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
ClassNode[] types = getArgumentTypes(args(expressions));
MethodNode constructor = checkGroovyStyleConstructor(leftRedirect, types, assignmentExpression);
if (constructor != null) {
rightExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, constructor);
}
} else if (implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, LIST_TYPE)
&& !implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, leftRedirect)) {
if (!extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression);
}
&& !implementsInterfaceOrIsSubclassOf(inferredRightExpressionType, leftRedirect)
&& !extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression);
}
}

Expand Down
36 changes: 23 additions & 13 deletions src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
'Cannot assign value of type java.lang.String into array of type int[]'
}

// GROOVY-8566
// GROOVY-5683, GROOVY-8566
void testMultiDimensionalArray4() {
assertScript '''
int[][] arrays = [ [], [1], [2,3] ]
Expand Down Expand Up @@ -846,17 +846,6 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
'''
}

// GROOVY-5683
void testArrayLengthOnMultidimensionalArray() {
assertScript '''
@ASTTest(phase=INSTRUCTION_SELECTION, value={
assert node.getNodeMetaData(INFERRED_TYPE) == int_TYPE.makeArray().makeArray()
})
int[][] array = [[1]] as int[][]
assert array[0].length == 1
'''
}

// GROOVY-10319
void testArrayClone() {
assertScript '''
Expand Down Expand Up @@ -1048,15 +1037,35 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
void testAbstractTypeInitializedByListLiteral() {
shouldFailWithMessages '''
abstract class A {
A(int n) {}
A(int n) {
}
}
A a = [1]
''',
'Cannot assign value of type java.util.ArrayList<java.lang.Integer> to variable of type A'
}

void testConcreteTypeInitializedByListLiteral() {
assertScript '''
class C {
int i
C(int i) {
this.i = i
}
}
C c = [42]
assert c.i == 42
'''
}

// GROOVY-6912
void testArrayListTypeInitializedByListLiteral() {
assertScript '''
ArrayList list = []
assert list.isEmpty()
assert list.size() == 0
'''

assertScript '''
ArrayList list = [1,2,3]
assert list.size() == 3
Expand All @@ -1066,6 +1075,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
assertScript '''
ArrayList list = [[1,2,3]]
assert list.size() == 1
assert list.last() instanceof List
'''

assertScript '''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,10 @@ final class GenericsUtilsTest {
def classNodeList = compile '''
@groovy.transform.CompileStatic
void test() {
def list = []
def list = Collections.emptyList()
}
'''
// get the intermediate type of the list literal (ArrayList<#E>)
// get the intermediate type of the list (List<#T>)
def node = classNodeList[0].getDeclaredMethod('test').code.statements[0]
def type = new StaticTypeCheckingVisitor(classNodeList[0].module.context,
classNodeList[0]).getType(node.expression.rightExpression)
Expand All @@ -378,8 +378,8 @@ final class GenericsUtilsTest {

assert listType == ClassHelper.LIST_TYPE
assert listType.genericsTypes.length == 1
assert listType.genericsTypes[0].name == '#E'
assert listType.genericsTypes[0].type.unresolvedName == '#E'
assert listType.genericsTypes[0].name == '#T'
assert listType.genericsTypes[0].type.unresolvedName == '#T'
assert listType.genericsTypes[0].type.name == 'java.lang.Object'
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,51 @@ final class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsST
assert !script.contains('createList')
}

// GROOVY-11309
void testListLiteralToSetAssignmentSC() {
for (t in ['LinkedHashSet','HashSet','Set']) {
astTrees.clear()
assertScript """
$t <String> set = []
assert set.isEmpty()
assert set.size() == 0
assert set instanceof LinkedHashSet
"""
String script = astTrees.values()[0][1]
assert script.contains('LinkedHashSet.<init> ()V')
assert !script.contains('ScriptBytecodeAdapter.createList')
}
}

// GROOVY-11309
void testListLiteralToListAssignmentSC() {
for (t in ['ArrayList','List','Collection','Iterable']) {
astTrees.clear()
assertScript """
$t <String> list = []
assert list.isEmpty()
assert list.size() == 0
assert list instanceof ArrayList
"""
String script = astTrees.values()[0][1]
assert script.contains('ArrayList.<init> ()V')
assert !script.contains('ScriptBytecodeAdapter.createList')
}

for (t in ['Object','Cloneable','Serializable','RandomAccess']) {
astTrees.clear()
assertScript """
$t list = []
assert list.isEmpty()
assert list.size() == 0
assert list instanceof ArrayList
"""
String script = astTrees.values()[0][1]
assert script.contains('ArrayList.<init> ()V')
assert !script.contains('ScriptBytecodeAdapter.createList')
}
}

// GROOVY-10029
void testCollectionToArrayAssignmentSC() {
assertScript '''
Expand Down

0 comments on commit 1bb575f

Please sign in to comment.