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 2020/11/04 00:05:00 UTC
[groovy] branch GROOVY_3_0_X updated (5591f06 -> e878e00)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 5591f06 GROOVY-9790: bootstrap method initialization exception raised when lambda parameter type is wrong
new bce2f3c GROOVY-9779: don't chain call properties together
new e878e00 GROOVY-9786: STC: propagate assignments to outer if-else for if-else-if
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/main/java/groovy/lang/MetaClassImpl.java | 2 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 85 ++++++++++----------
src/test/groovy/bugs/Groovy9779.groovy | 91 ++++++++++++++++++++--
.../groovy/transform/stc/STCAssignmentTest.groovy | 44 +++++++++--
4 files changed, 165 insertions(+), 57 deletions(-)
[groovy] 01/02: GROOVY-9779: don't chain call properties together
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit bce2f3cea4a36be0d12fb527221866b95205e2dc
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Oct 21 09:47:53 2020 -0500
GROOVY-9779: don't chain call properties together
(cherry picked from commit 0c0cc6b7e1c8573f2c267a1148bb3859040ad9e0)
---
src/main/java/groovy/lang/MetaClassImpl.java | 2 +-
src/test/groovy/bugs/Groovy9779.groovy | 91 +++++++++++++++++++++++++---
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index aebd79c..ec2cd8f 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1331,7 +1331,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return metaClass.invokeMethod(closure.getClass(), closure, DO_CALL_METHOD, originalArguments, false, fromInsideClass);
}
- if (value != null && !(value instanceof Map)) {
+ if (value != null && !(value instanceof Map) && !methodName.equals(CALL_METHOD)) {
try {
MetaClass metaClass = ((MetaClassRegistryImpl) registry).getMetaClass(value);
return metaClass.invokeMethod(value, CALL_METHOD, originalArguments); // delegate to call method of property value
diff --git a/src/test/groovy/bugs/Groovy9779.groovy b/src/test/groovy/bugs/Groovy9779.groovy
index c2b0623..eb383d3 100644
--- a/src/test/groovy/bugs/Groovy9779.groovy
+++ b/src/test/groovy/bugs/Groovy9779.groovy
@@ -22,22 +22,97 @@ import groovy.transform.CompileStatic
import org.junit.Test
import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
@CompileStatic
final class Groovy9779 {
@Test
- void testCallOperatorOnDynamicProperties() {
+ void testCallOperatorOnDynamicProperties1() {
assertScript '''
- class C {
+ class A {
def call() { return 42 }
}
- class D {
- static final x = new C()
- final y = new C()
+ class C {
+ static final x = new A()
+ final y = new A()
+ }
+ assert C.x() == 42
+ assert new C().x() == 42
+ assert new C().y() == 42
+ '''
+ }
+
+ @Test
+ void testCallOperatorOnDynamicProperties2() {
+ assertScript '''
+ class A {
+ def call = { -> return 42 }
+ }
+ class C {
+ static final x = new A()
+ final y = new A()
+ }
+ assert C.x() == 42
+ assert new C().x() == 42
+ assert new C().y() == 42
+ '''
+ }
+
+ @Test // don't chain call properties together
+ void testCallOperatorOnDynamicProperties3() {
+ def err = shouldFail '''
+ class A {
+ def call = { -> return 42 }
+ }
+ class B {
+ def call = new A()
+ }
+ class C {
+ static final x = new B()
+ final y = new B()
+ }
+ C.x()
+ '''
+ assert err.message.contains('No signature of method: B.call() is applicable')
+ }
+
+ @Test // don't chain call properties together
+ void testCallOperatorOnDynamicProperties4() {
+ def err = shouldFail '''
+ class A {
+ def call(x) { assert x == 1; return 42 }
+ }
+ class B {
+ def call = new A()
+ }
+ class C {
+ def plus = new B()
+ }
+ assert new C() + 1 == 42
+ '''
+ assert err.message.contains('No signature of method: C.plus() is applicable')
+ }
+
+ @Test
+ void testOperatorOverloadViaCallable() {
+ assertScript '''
+ class A {
+ def call(x) { return x + 1 }
+ }
+ class C {
+ def plus = new A()
+ }
+ assert new C() + 1 == 2
+ '''
+ }
+
+ @Test
+ void testOperatorOverloadViaClosure() {
+ assertScript '''
+ class C {
+ def plus = { x -> x + 1 }
}
- assert D.x() == 42
- assert new D().x() == 42
- assert new D().y() == 42
+ assert new C() + 1 == 2
'''
}
}
[groovy] 02/02: GROOVY-9786: STC: propagate assignments to outer
if-else for if-else-if
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e878e001eeda58659da39211eb8af047c2e48584
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Oct 22 11:32:04 2020 -0500
GROOVY-9786: STC: propagate assignments to outer if-else for if-else-if
(cherry picked from commit 024ffdc1a3e8ad67f9dc7ebcb2d247c9a0902010)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 85 +++++++++++-----------
.../groovy/transform/stc/STCAssignmentTest.groovy | 44 +++++++++--
2 files changed, 81 insertions(+), 48 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 141dffd..923346d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -855,28 +855,22 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
resultType = originType;
}
- // if we are in an if/else branch, keep track of assignment
- if (typeCheckingContext.ifElseForWhileAssignmentTracker != null && leftExpression instanceof VariableExpression
- && !isNullConstant(rightExpression)) {
+ // track conditional assignment
+ if (!isNullConstant(rightExpression)
+ && leftExpression instanceof VariableExpression
+ && typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
accessedVariable = new ParameterVariableExpression((Parameter) accessedVariable);
}
if (accessedVariable instanceof VariableExpression) {
- VariableExpression var = (VariableExpression) accessedVariable;
- List<ClassNode> types = typeCheckingContext.ifElseForWhileAssignmentTracker.get(var);
- if (types == null) {
- types = new LinkedList<>();
- ClassNode type = var.getNodeMetaData(INFERRED_TYPE);
- types.add(type);
- typeCheckingContext.ifElseForWhileAssignmentTracker.put(var, types);
- }
- types.add(resultType);
+ recordAssignment((VariableExpression) accessedVariable, resultType);
}
}
+
storeType(leftExpression, resultType);
- // if right expression is a ClosureExpression, store parameter type information
+ // propagate closure parameter type information
if (leftExpression instanceof VariableExpression) {
if (rightExpression instanceof ClosureExpression) {
leftExpression.putNodeMetaData(CLOSURE_ARGUMENTS, ((ClosureExpression) rightExpression).getParameters());
@@ -3863,7 +3857,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
@Override
public void visitIfElse(final IfStatement ifElse) {
Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
-
try {
// create a new temporary element in the if-then-else type info
typeCheckingContext.pushTemporaryTypeInfo();
@@ -3878,15 +3871,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
restoreTypeBeforeConditional();
ifElse.getElseBlock().visit(this);
+
+ // GROOVY-9786: if chaining: "if (...) x=?; else if (...) x=?;"
+ Map<VariableExpression, ClassNode> updates =
+ ifElse.getElseBlock().getNodeMetaData("assignments");
+ if (updates != null) {
+ updates.forEach(this::recordAssignment);
+ }
} finally {
- popAssignmentTracking(oldTracker);
+ ifElse.putNodeMetaData("assignments", popAssignmentTracking(oldTracker));
}
- BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
- if (instanceOfExpression == null) {
- instanceOfExpression = findNotInstanceOfReturnExpression(ifElse);
- }
- if (instanceOfExpression != null) {
- if (!typeCheckingContext.enclosingBlocks.isEmpty()) {
+
+ if (!typeCheckingContext.enclosingBlocks.isEmpty()) {
+ BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
+ if (instanceOfExpression == null) {
+ instanceOfExpression = findNotInstanceOfReturnExpression(ifElse);
+ }
+ if (instanceOfExpression != null) {
visitInstanceofNot(instanceOfExpression);
}
}
@@ -4036,32 +4037,30 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
restoreTypeBeforeConditional();
}
+ private void recordAssignment(final VariableExpression lhsExpr, final ClassNode rhsType) {
+ typeCheckingContext.ifElseForWhileAssignmentTracker.computeIfAbsent(lhsExpr, lhs -> {
+ ClassNode lhsType = lhs.getNodeMetaData(INFERRED_TYPE);
+ List<ClassNode> types = new ArrayList<>(2);
+ types.add(lhsType);
+ return types;
+ }).add(rhsType);
+ }
+
private void restoreTypeBeforeConditional() {
- Set<Map.Entry<VariableExpression, List<ClassNode>>> entries = typeCheckingContext.ifElseForWhileAssignmentTracker.entrySet();
- for (Map.Entry<VariableExpression, List<ClassNode>> entry : entries) {
- VariableExpression var = entry.getKey();
- List<ClassNode> items = entry.getValue();
- ClassNode originValue = items.get(0);
- storeType(var, originValue);
- }
+ typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
+ ClassNode originType = types.get(0);
+ storeType(var, originType);
+ });
}
protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
- Map<VariableExpression, ClassNode> assignments = new HashMap<VariableExpression, ClassNode>();
- if (!typeCheckingContext.ifElseForWhileAssignmentTracker.isEmpty()) {
- for (Map.Entry<VariableExpression, List<ClassNode>> entry : typeCheckingContext.ifElseForWhileAssignmentTracker.entrySet()) {
- VariableExpression key = entry.getKey();
- List<ClassNode> allValues = entry.getValue();
- // GROOVY-6099: First element of the list may be null, if no assignment was made before the branch
- List<ClassNode> nonNullValues = new ArrayList<>(allValues.size());
- for (ClassNode value : allValues) {
- if (value != null) nonNullValues.add(value);
- }
- ClassNode cn = lowestUpperBound(nonNullValues);
- storeType(key, cn);
- assignments.put(key, cn);
- }
- }
+ Map<VariableExpression, ClassNode> assignments = new HashMap<>();
+ typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
+ ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
+ .reduce(WideningCategories::lowestUpperBound).get();
+ assignments.put(var, type);
+ storeType(var, type);
+ });
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index ace7882..1c6752a 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -549,18 +549,52 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
void testIfWithCommonInterface() {
assertScript '''
- interface Foo { void foo() }
- class A implements Foo { void foo() { println 'A' } }
- class B implements Foo { void foo() { println 'B' } }
+ interface I {
+ def foo()
+ }
+ class A implements I {
+ def foo() { 'A' }
+ }
+ class B implements I {
+ def foo() { 'B' }
+ }
+
def x = new A()
- def y = 'foo'
+ def y = true
if (y) {
x = new B()
}
- x.foo()
+ assert x.foo() == 'B'
'''
}
+ // GROOVY-9786
+ void testIfElseIfWithCommonInterface() {
+ ['I', 'def', 'var', 'Object'].each {
+ assertScript """
+ interface I {
+ def foo()
+ }
+ class A implements I {
+ def foo() { 'A' }
+ }
+ class B implements I {
+ def foo() { 'B' }
+ }
+
+ $it x
+ def y = false
+ def z = true
+ if (y) {
+ x = new A()
+ } else if (z) {
+ x = new B()
+ }
+ assert x.foo() == 'B'
+ """
+ }
+ }
+
void testForLoopWithNewAssignment() {
shouldFailWithMessages '''
def x = '123'