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 2022/01/30 21:40:08 UTC

[groovy] 03/04: GROOVY-10419: STC: fix overflow for elvis assignment using setter method

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 8ce16783623a8a71df835b7a8fb7274c00f1290e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Dec 16 17:57:40 2021 -0600

    GROOVY-10419: STC: fix overflow for elvis assignment using setter method
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 37 ++++++-----
 .../groovy/transform/stc/STCAssignmentTest.groovy  | 73 +++++++++++++++-------
 2 files changed, 69 insertions(+), 41 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 453040d..01bf8cf 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -189,6 +189,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.elvisX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
@@ -965,10 +966,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         // because we need to check if a setter uses @DelegatesTo
         VariableExpression ve = varX("%", setterInfo.receiverType);
         // for compound assignment "x op= y" find type as if it was "x = (x op y)"
-        Expression newRightExpression = isCompoundAssignment(expression)
-                ? binX(leftExpression, getOpWithoutEqual(expression), rightExpression)
-                : rightExpression;
-        MethodCallExpression call = callX(ve, setterInfo.name, newRightExpression);
+        Expression valueExpression = rightExpression;
+        if (isCompoundAssignment(expression)) {
+            Token op = ((BinaryExpression) expression).getOperation();
+            if (op.getType() == ELVIS_EQUAL) { // GROOVY-10419: "x ?= y"
+                valueExpression = elvisX(leftExpression, rightExpression);
+            } else {
+                op = Token.newSymbol(TokenUtil.removeAssignment(op.getType()), op.getStartLine(), op.getStartColumn());
+                valueExpression = binX(leftExpression, op, rightExpression);
+            }
+        }
+        MethodCallExpression call = callX(ve, setterInfo.name, valueExpression);
         call.setImplicitThis(false);
         visitMethodCallExpression(call);
         MethodNode directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
@@ -978,7 +986,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             for (MethodNode setter : setterInfo.setters) {
                 ClassNode type = getWrapper(setter.getParameters()[0].getOriginType());
                 if (Boolean_TYPE.equals(type) || STRING_TYPE.equals(type) || CLASS_Type.equals(type)) {
-                    call = callX(ve, setterInfo.name, castX(type, newRightExpression));
+                    call = callX(ve, setterInfo.name, castX(type, valueExpression));
                     call.setImplicitThis(false);
                     visitMethodCallExpression(call);
                     directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
@@ -993,14 +1001,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 if (setter == directSetterCandidate) {
                     leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, directSetterCandidate);
                     leftExpression.removeNodeMetaData(INFERRED_TYPE); // clear assumption
-                    storeType(leftExpression, getType(newRightExpression));
+                    storeType(leftExpression, getType(valueExpression));
                     break;
                 }
             }
             return false;
         } else {
             ClassNode firstSetterType = setterInfo.setters.get(0).getParameters()[0].getOriginType();
-            addAssignmentError(firstSetterType, getType(newRightExpression), expression);
+            addAssignmentError(firstSetterType, getType(valueExpression), expression);
             return true;
         }
     }
@@ -1010,16 +1018,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private static boolean isCompoundAssignment(final Expression exp) {
-        if (!(exp instanceof BinaryExpression)) return false;
-        int type = ((BinaryExpression) exp).getOperation().getType();
-        return isAssignment(type) && type != ASSIGN;
-    }
-
-    private static Token getOpWithoutEqual(final Expression exp) {
-        if (!(exp instanceof BinaryExpression)) return null; // should never happen
-        Token op = ((BinaryExpression) exp).getOperation();
-        int typeWithoutEqual = TokenUtil.removeAssignment(op.getType());
-        return new Token(typeWithoutEqual, op.getText() /* will do */, op.getStartLine(), op.getStartColumn());
+        if (exp instanceof BinaryExpression) {
+            Token op = ((BinaryExpression) exp).getOperation();
+            return isAssignment(op.getType()) && op.getType() != EQUAL;
+        }
+        return false;
     }
 
     protected ClassNode getOriginalDeclarationType(final Expression lhs) {
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index 462334c..13fb434 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -1015,16 +1015,16 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
 
     void testMultiAssign() {
         assertScript '''
-        def m() {
-            def row = ['', '', '']
-            def (left, right) = [row[0], row[1]]
-            left.toUpperCase()
-        }
+            def m() {
+                def row = ['', '', '']
+                def (left, right) = [row[0], row[1]]
+                left.toUpperCase()
+            }
         '''
     }
 
     // GROOVY-8220
-    void testFlowTypingParameterTempTypeAssignmentTracking() {
+    void testFlowTypingParameterTempTypeAssignmentTracking1() {
         assertScript '''
             class Foo {
                 CharSequence makeEnv( env, StringBuilder result = new StringBuilder() ) {
@@ -1039,7 +1039,10 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
             }
             assert new Foo().makeEnv('X=1') == 'export X=1;'
         '''
-        // GROOVY-8237
+    }
+
+    // GROOVY-8237
+    void testFlowTypingParameterTempTypeAssignmentTracking2() {
         assertScript '''
             class Foo {
                 String parse(Reader reader) {
@@ -1053,7 +1056,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    void testFlowTypingParameterTempTypeAssignmentTrackingWithGenerics() {
+    void testFlowTypingParameterTempTypeAssignmentTracking3() {
         assertScript '''
             class M {
                 Map<String, List<Object>> mvm = new HashMap<String, List<Object>>()
@@ -1083,27 +1086,49 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
 
     void testNarrowingConversion() {
         assertScript '''
-        interface A1{}
-        interface A2 extends A1{}
-
-        class C1 implements A1{}
-
-        def m(A2 a2) {
-            C1 c1 = (C1) a2
-        }
+            interface A {
+            }
+            interface B extends A {
+            }
+            class C implements B {
+            }
+            def m(B b) {
+                C c = (C) b
+            }
         '''
     }
 
     void testFinalNarrowingConversion() {
         shouldFailWithMessages '''
-        interface A1{}
-        interface A2 extends A1{}
-
-        final class C1 implements A1{}
+            interface I {
+            }
+            interface B extends I {
+            }
+            final class C implements I {
+            }
+            def m(B b) {
+                C c = (C) b
+            }
+        ''',
+        'Inconvertible types: cannot cast B to C'
+    }
 
-        def m(A2 a2) {
-            C1 c1 = (C1) a2
-        }
-        ''', 'Inconvertible types: cannot cast A2 to C1'
+    // GROOVY-10419
+    void testElvisAssignmentAndSetter() {
+        assertScript '''
+            class C {
+                def p
+                void setP(p) {
+                    this.p = p
+                }
+            }
+            @groovy.transform.TypeChecked
+            void test(C c) {
+                c.p ?= 'x'
+            }
+            def c = new C()
+            test(c)
+            assert c.p == 'x'
+        '''
     }
 }