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 2021/02/24 02:15:25 UTC

[groovy] 01/01: GROOVY-9951: set implicit-this to false for method call expressions

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

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

commit 98ef337f1812eba402e6f7cc65e611057ba8187d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Feb 23 20:06:19 2021 -0600

    GROOVY-9951: set implicit-this to false for method call expressions
---
 .../groovy/ast/tools/ConstructorNodeUtils.java     | 30 +++++++++++++---------
 .../transform/ToStringASTTransformation.java       | 20 ++++++++-------
 src/test/groovy/transform/stc/BugsSTCTest.groovy   | 18 +++++++++++++
 3 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ConstructorNodeUtils.java b/src/main/java/org/apache/groovy/ast/tools/ConstructorNodeUtils.java
index a9cd0f8..5d7f251 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ConstructorNodeUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ConstructorNodeUtils.java
@@ -24,7 +24,7 @@ import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.ast.expr.ListExpression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.ExpressionStatement;
@@ -33,14 +33,17 @@ import org.codehaus.groovy.transform.ImmutableASTTransformation;
 
 import java.util.List;
 
+import static java.util.stream.Collectors.toList;
 import static org.codehaus.groovy.ast.ClassHelper.make;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
+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.constX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.forS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.listX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.notX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
@@ -88,17 +91,20 @@ public class ConstructorNodeUtils {
         if (!pojo) {
             return stmt(callX(IMMUTABLE_TYPE, "checkPropNames", args(varX("this"), namedArgs)));
         }
-        BlockStatement block = new BlockStatement();
-        ListExpression knownPropNames = new ListExpression();
-        for (PropertyNode pNode : props) {
-            knownPropNames.addExpression(constX(pNode.getName()));
-        }
-        VariableExpression validNames = localVarX("validNames", ClassHelper.LIST_TYPE);
+
+        Expression validNames = localVarX("validNames", ClassHelper.LIST_TYPE);
         Parameter name = param(ClassHelper.STRING_TYPE, "arg");
-        Statement loopS = ifS(notX(callX(validNames, "contains", varX(name))),
-                throwS(ctorX(EXCEPTION, plusX(constX("Unknown named argument: "), varX(name)))));
-        block.addStatement(declS(validNames, knownPropNames));
-        block.addStatement(forS(name, callX(namedArgs, "keySet"), loopS));
-        return block;
+
+        MethodCallExpression names = callX(namedArgs, "keySet");
+        names.setImplicitThis(false);
+
+        MethodCallExpression isNameValid = callX(validNames, "contains", varX(name));
+        isNameValid.setImplicitThis(false);
+
+        return block(
+            declS(validNames, listX(props.stream().map(p -> constX(p.getName())).collect(toList()))),
+            forS(name, names, ifS(notX(isNameValid),
+                    throwS(ctorX(EXCEPTION, plusX(constX("Unknown named argument: "), varX(name))))))
+        );
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
index f3c0328..a11cf57 100644
--- a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
@@ -76,7 +76,7 @@ import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
 @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
 public class ToStringASTTransformation extends AbstractASTTransformation {
 
-    static final Class MY_CLASS = ToString.class;
+    static final Class<?> MY_CLASS = ToString.class;
     static final ClassNode MY_TYPE = make(MY_CLASS);
     static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
     private static final ClassNode STRINGBUILDER_TYPE = make(StringBuilder.class);
@@ -245,9 +245,8 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
 
         // wrap up
         body.addStatement(appendS(result, constX(")")));
-        MethodCallExpression toString = callX(result, "toString");
-        toString.setImplicitThis(false);
-        return toString;
+
+        return toStringX(result);
     }
 
     private static void appendValue(BlockStatement body, Expression result, VariableExpression first, Expression value, String name, boolean includeNames, boolean ignoreNulls, boolean canBeSelf, boolean pojo) {
@@ -255,17 +254,14 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
         final Statement appendValue = ignoreNulls ? ifS(notNullX(value), thenBlock) : thenBlock;
         appendCommaIfNotFirst(thenBlock, result, first);
         appendPrefix(thenBlock, result, name, includeNames);
-        Expression toString = pojo
-                ? callX(value, "toString")
-                : callX(INVOKER_TYPE, "toString", value);
+        Expression toString = pojo ? toStringX(value) : callX(INVOKER_TYPE, "toString", value);
         if (canBeSelf) {
             thenBlock.addStatement(ifElseS(
-                    sameX(value, new VariableExpression("this")),
+                    sameX(value, varX("this")),
                     appendS(result, constX("(this)")),
                     appendS(result, toString)));
         } else {
             thenBlock.addStatement(appendS(result, toString));
-
         }
         body.addStatement(appendValue);
     }
@@ -297,4 +293,10 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
         append.setImplicitThis(false);
         return stmt(append);
     }
+
+    private static Expression toStringX(final Expression object) {
+        MethodCallExpression toString = callX(object, "toString");
+        toString.setImplicitThis(false);
+        return toString;
+    }
 }
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 2f5fe42..ea1c787 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -938,4 +938,22 @@ Printer
             }
         '''
     }
+
+    // GROOVY-9951
+    void testInnerImmutablePOJO() {
+        assertScript '''
+            import groovy.transform.Immutable
+            import groovy.transform.stc.POJO
+
+            class Outer {
+                @Immutable @POJO
+                static class Inner {
+                    String proper
+                }
+            }
+
+            def obj = new Outer.Inner('value')
+            assert obj.proper == 'value'
+        '''
+    }
 }