This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new c523c6d GROOVY-9653: SC: improve AST visit for multiple setter method
candidates
c523c6d is described below
commit c523c6d9ffe4983521ff97bc209893a05f336e1b
Author: Eric Milles <[email protected]>
AuthorDate: Sun Jul 26 11:19:54 2020 -0500
GROOVY-9653: SC: improve AST visit for multiple setter method candidates
---
.../groovy/classgen/AsmClassGenerator.java | 3 +-
.../transformers/BinaryExpressionTransformer.java | 14 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 17 +++--
.../stc/FieldsAndPropertiesSTCTest.groovy | 88 +++++++++++++---------
4 files changed, 72 insertions(+), 50 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 326821d..93c2395 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -124,8 +124,8 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
-import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
+import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
@@ -1311,6 +1311,7 @@ public class AsmClassGenerator extends ClassGenerator {
PropertyExpression pexp = thisPropX(true, variableName);
pexp.getObjectExpression().setSourcePosition(expression);
pexp.getProperty().setSourcePosition(expression);
+ pexp.copyNodeMetaData(expression);
pexp.visit(this);
}
diff --git
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index 8523424..2955577 100644
---
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -98,15 +98,17 @@ public class BinaryExpressionTransformer {
}
}
}
- if (operationType == Types.EQUAL && leftExpression instanceof
PropertyExpression) {
+ if (operationType == Types.ASSIGN) {
MethodNode directMCT =
leftExpression.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
if (directMCT != null) {
- PropertyExpression left = (PropertyExpression) leftExpression;
- Expression right =
staticCompilationTransformer.transform(rightExpression);
- return transformPropertyAssignmentToSetterCall(left, right,
directMCT);
+ Expression left =
staticCompilationTransformer.transform(leftExpression);
+ if (left instanceof PropertyExpression) {
+ Expression right =
staticCompilationTransformer.transform(rightExpression);
+ return
transformPropertyAssignmentToSetterCall((PropertyExpression) left, right,
directMCT);
+ }
+ // TODO: Handle left instanceof VariableExpression and has
DIRECT_METHOD_CALL_TARGET?
}
- }
- if (operationType == Types.COMPARE_EQUAL || operationType ==
Types.COMPARE_NOT_EQUAL) {
+ } else if (operationType == Types.COMPARE_EQUAL || operationType ==
Types.COMPARE_NOT_EQUAL) {
// let's check if one of the operands is the null constant
CompareToNullExpression compareToNullExpression = null;
if (isNullConstant(leftExpression)) {
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 f29513c..6b69ee6 100644
---
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -949,7 +949,7 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
* @param leftExpression left expression of the assignment
* @param rightExpression right expression of the assignment
* @param setterInfo possible setters
- * @return true if type checking passed
+ * @return {@code false} if valid setter found or {@code true} if type
checking error created
*/
private boolean ensureValidSetter(final Expression expression, final
Expression leftExpression, final Expression rightExpression, final SetterInfo
setterInfo) {
// for expressions like foo = { ... }
@@ -985,16 +985,17 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
for (MethodNode setter : setterInfo.setters) {
if (setter == directSetterCandidate) {
leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET,
directSetterCandidate);
+ leftExpression.removeNodeMetaData(INFERRED_TYPE); // clear
assumption
storeType(leftExpression, getType(newRightExpression));
break;
}
}
+ return false;
} else {
- ClassNode firstSetterType =
setterInfo.setters.iterator().next().getParameters()[0].getOriginType();
+ ClassNode firstSetterType =
setterInfo.setters.get(0).getParameters()[0].getOriginType();
addAssignmentError(firstSetterType, getType(newRightExpression),
expression);
return true;
}
- return false;
}
private boolean isCompoundAssignment(final Expression exp) {
@@ -1468,8 +1469,9 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
if (receiverType.isArray() && "length".equals(propertyName)) {
storeType(pexp, int_TYPE);
if (visitor != null) {
- PropertyNode length = new PropertyNode("length",
Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null, null,
null);
- visitor.visitProperty(length);
+ FieldNode length = new FieldNode("length",
Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null);
+ length.setDeclaringClass(receiverType);
+ visitor.visitField(length);
}
return true;
}
@@ -1554,8 +1556,9 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
visitor.visitField(field);
} else {
for (MethodNode setter : setters) {
- ClassNode setterType =
setter.getParameters()[0].getOriginType();
- FieldNode virtual = new
FieldNode(propertyName, 0, setterType, current, EmptyExpression.INSTANCE);
+ // visiting setter will not infer the
property type since return type is void, so visit a dummy field instead
+ FieldNode virtual = new
FieldNode(propertyName, 0, setter.getParameters()[0].getOriginType(), current,
null);
+
virtual.setDeclaringClass(setter.getDeclaringClass());
visitor.visitField(virtual);
}
}
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 5db8505..ed0d16f 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -642,6 +642,26 @@ class FieldsAndPropertiesSTCTest extends
StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-9653
+ void testSetterInWithUsingPropertyNotation_DelegateAndOwnerHaveSetter() {
+ assertScript '''
+ class C {
+ final result = new D().with {
+ something = 'value' // ClassCastException: D cannot be
cast to C
+ return object
+ }
+ void setSomething(value) { }
+ }
+
+ class D {
+ void setSomething(value) { }
+ Object getObject() { 'works' }
+ }
+
+ assert new C().result == 'works'
+ '''
+ }
+
// GROOVY-6230
void testAttributeWithGetterOfDifferentType() {
assertScript '''import java.awt.Dimension
@@ -778,56 +798,52 @@ import org.codehaus.groovy.ast.stmt.AssertStatement
void testPropertyAssignmentInSubClassAndMultiSetter() {
10.times {
assertScript '''
+ class A {
+ int which
- public class Activity {
- int debug
+ A() {
+ contentView = 42L
+ assert which == 2
+ }
- Activity() {
- contentView = 1
+ void setContentView(Date value) { which = 1 }
+ void setContentView(Long value) { which = 2 }
}
- public void setContentView(Date layoutResID) { debug = 2 }
- public void setContentView(int layoutResID) { debug = 3 }
- }
-
- class MyActivity extends Activity {
- void foo() {
- contentView = 1
- assert debug == 3
- contentView = new Date()
- assert debug == 2
+ class B extends A {
+ void m() {
+ contentView = 42L
+ assert which == 2
+ contentView = new Date()
+ assert which == 1
+ }
}
- }
- new MyActivity().foo()
- '''
+
+ new B().m()
+ '''
}
}
void testPropertyAssignmentInSubClassAndMultiSetterThroughDelegation() {
10.times {
- assertScript '''
-
- public class Activity {
- int debug
+ assertScript '''\
+ class A {
+ int which
- Activity() {
- contentView = 1
+ void setContentView(Date value) { which = 1 }
+ void setContentView(Long value) { which = 2 }
}
- public void setContentView(Date layoutResID) { debug = 2 }
- public void setContentView(int layoutResID) { debug = 3 }
- }
+ class B extends A {
+ }
- class MyActivity extends Activity {
- }
- def activity = new MyActivity()
- activity.with {
- contentView = 1
- assert debug == 3
- contentView = new Date()
- assert debug == 2
- }
- '''
+ new B().with {
+ contentView = 42L
+ assert which == 2
+ contentView = new Date()
+ assert which == 1
+ }
+ '''
}
}
