You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/06/15 04:14:32 UTC
[groovy] 01/02: minor fix-ups
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit a3b9ab2bd44f65a9e229bbd307707a5462250c5e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 14 07:49:36 2020 -0500
minor fix-ups
---
.../transform/stc/StaticTypeCheckingVisitor.java | 32 +++++++++-------------
.../groovy/transform/stc/ClosuresSTCTest.groovy | 32 ++++++++++------------
2 files changed, 28 insertions(+), 36 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 1deff6a..f5485fa 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -96,7 +96,6 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement;
import org.codehaus.groovy.ast.stmt.WhileStatement;
import org.codehaus.groovy.ast.tools.GenericsUtils;
import org.codehaus.groovy.ast.tools.WideningCategories;
-import org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
import org.codehaus.groovy.classgen.ReturnAdder;
import org.codehaus.groovy.classgen.asm.InvocationWriter;
import org.codehaus.groovy.control.CompilationUnit;
@@ -793,11 +792,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
// GROOVY-5874: if left expression is a closure shared variable, a second pass should be done
- if (leftExpression instanceof VariableExpression) {
- VariableExpression leftVar = (VariableExpression) leftExpression;
- if (leftVar.isClosureSharedVariable()) {
- typeCheckingContext.secondPassExpressions.add(new SecondPassExpression<>(expression));
- }
+ if (leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isClosureSharedVariable()) {
+ typeCheckingContext.secondPassExpressions.add(new SecondPassExpression(expression));
}
boolean isAssignment = isAssignment(expression.getOperation().getType());
@@ -1937,13 +1933,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
protected boolean isSecondPassNeededForControlStructure(final Map<VariableExpression, ClassNode> varOrigType, final Map<VariableExpression, List<ClassNode>> oldTracker) {
- Map<VariableExpression, ClassNode> assignedVars = popAssignmentTracking(oldTracker);
- for (Map.Entry<VariableExpression, ClassNode> entry : assignedVars.entrySet()) {
+ for (Map.Entry<VariableExpression, ClassNode> entry : popAssignmentTracking(oldTracker).entrySet()) {
Variable key = findTargetVariable(entry.getKey());
- if (key instanceof VariableExpression) {
+ if (key instanceof VariableExpression && varOrigType.containsKey(key)) {
ClassNode origType = varOrigType.get(key);
ClassNode newType = entry.getValue();
- if (varOrigType.containsKey(key) && (!newType.equals(origType))) {
+ if (!newType.equals(origType)) {
return true;
}
}
@@ -2396,12 +2391,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
protected void restoreVariableExpressionMetadata(final Map<VariableExpression, Map<StaticTypesMarker, Object>> typesBeforeVisit) {
if (typesBeforeVisit != null) {
for (Map.Entry<VariableExpression, Map<StaticTypesMarker, Object>> entry : typesBeforeVisit.entrySet()) {
- VariableExpression ve = entry.getKey();
- Map<StaticTypesMarker, Object> metadata = entry.getValue();
for (StaticTypesMarker marker : StaticTypesMarker.values()) {
- ve.removeNodeMetaData(marker);
- Object value = metadata.get(marker);
- if (value != null) ve.setNodeMetaData(marker, value);
+ Object value = entry.getValue().get(marker);
+ if (value == null) {
+ entry.getKey().removeNodeMetaData(marker);
+ } else {
+ entry.getKey().putNodeMetaData(marker, value);
+ }
}
}
}
@@ -3397,7 +3393,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
if (mn.isEmpty() && typeCheckingContext.getEnclosingClosure() != null && args.length == 0) {
- // add special handling of getDelegate() and getOwner()
+ // add special handling of "delegate", "owner", and "this" in a closure
switch (name) {
case "getDelegate":
mn = Collections.singletonList(GET_DELEGATE);
@@ -3603,7 +3599,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
nodes.add(arg);
}
}
- return new LowestUpperBoundClassNode(returnType.getName() + "Composed", OBJECT_TYPE, nodes.toArray(ClassNode.EMPTY_ARRAY));
+ return new WideningCategories.LowestUpperBoundClassNode(returnType.getName() + "Composed", OBJECT_TYPE, nodes.toArray(ClassNode.EMPTY_ARRAY));
}
}
return returnType;
@@ -5494,8 +5490,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (expression instanceof BinaryExpression) {
Expression left = ((BinaryExpression) expression).getLeftExpression();
if (left instanceof VariableExpression) {
- // should always be the case
- // this should always be the case, but adding a test is safer
Variable target = findTargetVariable((VariableExpression) left);
if (target instanceof VariableExpression) {
VariableExpression var = (VariableExpression) target;
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 17b7a33..11ce461 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -35,10 +35,8 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
void testClosureWithoutArgumentsExplicit() {
// GROOVY-9079: no params to statically type check but shouldn't get NPE
assertScript '''
- import groovy.transform.CompileStatic
import java.util.concurrent.Callable
- @CompileStatic
String makeFoo() {
Callable<String> call = { -> 'foo' }
call()
@@ -157,19 +155,19 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testClosureShouldNotChangeInferredType() {
+ void testClosureSharedVariable1() {
assertScript '''
def x = '123';
{ -> x = new StringBuffer() }
- x.charAt(0)
+ x.charAt(0) // available in String and StringBuffer
'''
}
- void testClosureSharedVariableWithIncompatibleType() {
+ void testClosureSharedVariable2() {
shouldFailWithMessages '''
def x = '123';
- { -> x = 1 }
- x.charAt(0)
+ { -> x = 123 }
+ x.charAt(0) // available in String but not available in Integer
''', 'A closure shared variable [x] has been assigned with various types and the method [charAt(int)] does not exist in the lowest upper bound'
}
@@ -216,7 +214,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
fib.fib(2)
'''
}
-
+
void testClosureRecursionWithoutClosureTypeArgument() {
shouldFailWithMessages '''
Closure fib
@@ -262,6 +260,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
}
'''
}
+
// a case in Grails
void testShouldNotThrowClosureSharedVariableError2() {
assertScript '''
@@ -336,9 +335,9 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
}
''', 'Cannot find matching method'
}
-
- //GROOVY-6189
- void testSAMsInMethodSelection(){
+
+ // GROOVY-6189
+ void testSAMsInMethodSelection() {
// simple direct case
assertScript """
interface MySAM {
@@ -347,7 +346,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
def foo(MySAM sam) {sam.someMethod()}
assert foo {1} == 1
"""
-
+
// overloads with classes implemented by Closure
["java.util.concurrent.Callable", "Object", "Closure", "GroovyObjectSupport", "Cloneable", "Runnable", "GroovyCallable", "Serializable", "GroovyObject"].each {
className ->
@@ -361,7 +360,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
"""
}
}
-
+
void testSAMVariable() {
assertScript """
interface SAM { def foo(); }
@@ -384,7 +383,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
assert s.accept(1) == -1
"""
}
-
+
void testSAMProperty() {
assertScript """
interface SAM { def foo(); }
@@ -395,7 +394,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
assert x.s.foo() == 1
"""
}
-
+
void testSAMAttribute() {
assertScript """
interface SAM { def foo(); }
@@ -476,7 +475,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
}
''', 'Reference to method is ambiguous. Cannot choose between'
}
-
+
void testSAMType() {
assertScript """
interface Foo {int foo()}
@@ -578,4 +577,3 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
'''
}
}
-