groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [1/4] groovy git commit: GROOVY-7315: CompileStatic/TypeChecked cannot create non-static nested inner class using named-arg short-hand syntax (closes #634)
Date Mon, 13 Nov 2017 02:50:49 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X e704a7ce9 -> 35838a514


GROOVY-7315: CompileStatic/TypeChecked cannot create non-static nested inner class using named-arg
short-hand syntax (closes #634)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/becb9cea
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/becb9cea
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/becb9cea

Branch: refs/heads/GROOVY_2_4_X
Commit: becb9ceab308aa620b6f8d289150d3b9cc8d9fc5
Parents: e704a7c
Author: paulk <paulk@asert.com.au>
Authored: Sun Nov 12 23:07:27 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Mon Nov 13 12:48:41 2017 +1000

----------------------------------------------------------------------
 .../ConstructorCallTransformer.java             | 44 +++++++++++++-------
 .../stc/StaticTypeCheckingVisitor.java          | 27 ++++++++----
 .../groovy/transform/stc/BugsSTCTest.groovy     | 31 +++++++++++++-
 3 files changed, 79 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/becb9cea/src/main/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
b/src/main/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
index 1a04299..569a5ab 100644
--- a/src/main/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
+++ b/src/main/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
@@ -22,6 +22,8 @@ import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.GroovyCodeVisitor;
+import org.codehaus.groovy.ast.InnerClassNode;
+import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
@@ -55,15 +57,16 @@ public class ConstructorCallTransformer {
     Expression transformConstructorCall(final ConstructorCallExpression expr) {
         ConstructorNode node = (ConstructorNode) expr.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
         if (node == null) return expr;
-        if (node.getParameters().length == 1
-                && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node.getParameters()[0].getType(),
ClassHelper.MAP_TYPE)
+        Parameter[] params = node.getParameters();
+        if ((params.length == 1 || params.length == 2) // 2 is for inner class case
+                && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(params[params.length
- 1].getType(), ClassHelper.MAP_TYPE)
                 && node.getCode() == StaticTypeCheckingVisitor.GENERATED_EMPTY_STATEMENT)
{
             Expression arguments = expr.getArguments();
             if (arguments instanceof TupleExpression) {
                 TupleExpression tupleExpression = (TupleExpression) arguments;
                 List<Expression> expressions = tupleExpression.getExpressions();
-                if (expressions.size() == 1) {
-                    Expression expression = expressions.get(0);
+                if (expressions.size() == 1 || expressions.size() == 2) { // 2 = inner class
case
+                    Expression expression = expressions.get(expressions.size() - 1);
                     if (expression instanceof MapExpression) {
                         MapExpression map = (MapExpression) expression;
                         // check that the node doesn't belong to the list of declared constructors
@@ -73,7 +76,8 @@ public class ConstructorCallTransformer {
                                 return staticCompilationTransformer.superTransform(expr);
                             }
                         }
-                        // replace this call with a call to <init>() + appropriate
setters
+                        // replace call to <init>(Map) or <init>(this, Map)
+                        // with a call to <init>() or <init>(this) + appropriate
setters
                         // for example, foo(x:1, y:2) is replaced with:
                         // { def tmp = new Foo(); tmp.x = 1; tmp.y = 2; return tmp }()
                         MapStyleConstructorCall result = new MapStyleConstructorCall(
@@ -95,21 +99,25 @@ public class ConstructorCallTransformer {
     private static class MapStyleConstructorCall extends BytecodeExpression implements Opcodes
{
         private StaticCompilationTransformer staticCompilationTransformer;
         private AsmClassGenerator acg;
-        private ClassNode declaringClass;
-        private MapExpression map;
-        private ConstructorCallExpression orginalCall;
+        private final ClassNode declaringClass;
+        private final MapExpression map;
+        private final ConstructorCallExpression originalCall;
+        private final boolean innerClassCall;
 
         public MapStyleConstructorCall(
                 final StaticCompilationTransformer transformer,
                 final ClassNode declaringClass,
                 final MapExpression map,
-                ConstructorCallExpression orginalCall) {
+                final ConstructorCallExpression originalCall) {
             this.staticCompilationTransformer = transformer;
             this.declaringClass = declaringClass;
             this.map = map;
-            this.orginalCall = orginalCall;
-            this.setSourcePosition(orginalCall);
-            this.copyNodeMetaData(orginalCall);
+            this.originalCall = originalCall;
+            this.setSourcePosition(originalCall);
+            this.copyNodeMetaData(originalCall);
+            List<Expression> originalExpressions = originalCall.getArguments() instanceof
TupleExpression ?
+                    ((TupleExpression)originalCall.getArguments()).getExpressions() : null;
+            this.innerClassCall = originalExpressions != null && originalExpressions.size()
== 2;
         }
 
         @Override
@@ -117,7 +125,7 @@ public class ConstructorCallTransformer {
             if (visitor instanceof AsmClassGenerator) {
                 acg = (AsmClassGenerator) visitor;
             } else {
-                orginalCall.visit(visitor);
+                originalCall.visit(visitor);
             } 
             super.visit(visitor);
         }
@@ -137,7 +145,15 @@ public class ConstructorCallTransformer {
             String classInternalName = BytecodeHelper.getClassInternalName(declaringClass);
             mv.visitTypeInsn(NEW, classInternalName);
             mv.visitInsn(DUP);
-            mv.visitMethodInsn(INVOKESPECIAL, classInternalName, "<init>", "()V", false);
+            String desc = "()V";
+            if (innerClassCall && declaringClass.isRedirectNode() && declaringClass.redirect()
instanceof InnerClassNode) {
+                // load "this"
+                mv.visitVarInsn(ALOAD, 0);
+                InnerClassNode icn = (InnerClassNode) declaringClass.redirect();
+                Parameter[] params = { new Parameter(icn.getOuterClass(), "$p$") };
+                desc = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, params);
+            }
+            mv.visitMethodInsn(INVOKESPECIAL, classInternalName, "<init>", desc, false);
             mv.visitVarInsn(ASTORE, tmpObj); // store it into tmp variable
 
             // load every field

http://git-wip-us.apache.org/repos/asf/groovy/blob/becb9cea/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index a9b0a0d..d3bfbdc 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1929,9 +1929,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
 
 
         MethodNode node;
-        if (args.length == 1
-                && implementsInterfaceOrIsSubclassOf(args[0], MAP_TYPE)
-                && findMethod(receiver, "<init>", ClassNode.EMPTY_ARRAY).size()
== 1
+        if (looksLikeNamedArgConstructor(receiver, args)
+                && findMethod(receiver, "<init>", DefaultGroovyMethods.init(args)).size()
== 1
                 && findMethod(receiver, "<init>", args).isEmpty()) {
             // bean-style constructor
             node = typeCheckMapConstructor(call, receiver, arguments);
@@ -1943,7 +1942,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         }
         node = findMethodOrFail(call, receiver, "<init>", args);
         if (node != null) {
-            if (node.getParameters().length == 0 && args.length == 1 && implementsInterfaceOrIsSubclassOf(args[0],
MAP_TYPE)) {
+            if (looksLikeNamedArgConstructor(receiver, args) && node.getParameters().length
+ 1 == args.length) {
                 node = typeCheckMapConstructor(call, receiver, arguments);
             } else {
                 typeCheckMethodsWithGenericsOrFail(receiver, args, node, call);
@@ -1953,17 +1952,31 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         extension.afterMethodCall(call);
     }
 
+    private boolean looksLikeNamedArgConstructor(ClassNode receiver, ClassNode[] args) {
+        return (args.length == 1 || args.length == 2 && isInnerConstructor(receiver,
args[0]))
+                && implementsInterfaceOrIsSubclassOf(args[args.length - 1], MAP_TYPE);
+    }
+
+    private boolean isInnerConstructor(ClassNode receiver, ClassNode parent) {
+        return receiver.isRedirectNode() && receiver.redirect() instanceof InnerClassNode
&&
+                receiver.redirect().getOuterClass().equals(parent);
+    }
+
     protected MethodNode typeCheckMapConstructor(final ConstructorCallExpression call, final
ClassNode receiver, final Expression arguments) {
         MethodNode node = null;
         if (arguments instanceof TupleExpression) {
             TupleExpression texp = (TupleExpression) arguments;
             List<Expression> expressions = texp.getExpressions();
-            if (expressions.size() == 1) {
-                Expression expression = expressions.get(0);
+            // should only get here with size = 2 when inner class constructor
+            if (expressions.size() == 1 || expressions.size() == 2) {
+                Expression expression = expressions.get(expressions.size() - 1);
                 if (expression instanceof MapExpression) {
                     MapExpression argList = (MapExpression) expression;
                     checkGroovyConstructorMap(call, receiver, argList);
-                    node = new ConstructorNode(Opcodes.ACC_PUBLIC, new Parameter[]{new Parameter(MAP_TYPE,
"map")}, ClassNode.EMPTY_ARRAY, GENERATED_EMPTY_STATEMENT);
+                    Parameter[] params = expressions.size() == 1
+                            ? new Parameter[]{new Parameter(MAP_TYPE, "map")}
+                            : new Parameter[]{new Parameter(receiver.redirect().getOuterClass(),
"$p$"), new Parameter(MAP_TYPE, "map")};
+                    node = new ConstructorNode(Opcodes.ACC_PUBLIC, params, ClassNode.EMPTY_ARRAY,
GENERATED_EMPTY_STATEMENT);
                     node.setDeclaringClass(receiver);
                 }
             }

http://git-wip-us.apache.org/repos/asf/groovy/blob/becb9cea/src/test/groovy/transform/stc/BugsSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 7f09833..07b3ba4 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -20,8 +20,6 @@ package groovy.transform.stc
 
 /**
  * Unit tests for static type checking : bug fixes.
- *
- * @author Cedric Champeau
  */
 class BugsSTCTest extends StaticTypeCheckingTestCase {
     // GROOVY-5456
@@ -717,4 +715,33 @@ Printer
             assert Child.name == 'Child'
         '''
     }
+
+    // GROOVY-7315
+    void testNamedArgConstructorSupportWithInnerClassesAndCS() {
+        assertScript '''
+            import groovy.transform.*
+            @ToString
+            class X {
+                int a
+                static X makeX() { new X(a:1) }
+                Y makeY() {
+                    new Y(b:2)
+                }
+                @ToString
+                private class Y {
+                    int b
+                    @ToString
+                    private class Z {
+                        int c
+                    }
+                    Z makeZ() {
+                        new Z(c:3)
+                    }
+                }
+            }
+            assert X.makeX().toString() == 'X(1)'
+            assert X.makeX().makeY().toString() == 'X$Y(2)'
+            assert X.makeX().makeY().makeZ().toString() == 'X$Y$Z(3)'
+        '''
+    }
 }


Mime
View raw message