Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 493E9200D5F for ; Mon, 13 Nov 2017 03:44:40 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 464CE160C08; Mon, 13 Nov 2017 02:44:40 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 3D3C0160BF1 for ; Mon, 13 Nov 2017 03:44:39 +0100 (CET) Received: (qmail 98550 invoked by uid 500); 13 Nov 2017 02:44:38 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 98541 invoked by uid 99); 13 Nov 2017 02:44:38 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Nov 2017 02:44:38 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 4D126E0014; Mon, 13 Nov 2017 02:44:36 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: paulk@apache.org To: commits@groovy.apache.org Date: Mon, 13 Nov 2017 02:44:36 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [1/5] groovy git commit: GROOVY-7315: CompileStatic/TypeChecked cannot create non-static nested inner class using named-arg short-hand syntax (closes #634) archived-at: Mon, 13 Nov 2017 02:44:40 -0000 Repository: groovy Updated Branches: refs/heads/GROOVY_2_6_X ff4eb69dd -> 412c6441c 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/45aa4b31 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/45aa4b31 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/45aa4b31 Branch: refs/heads/GROOVY_2_6_X Commit: 45aa4b31bcd3b09ad700b122a57b33040cb15d06 Parents: ff4eb69 Author: paulk Authored: Sun Nov 12 23:07:27 2017 +1000 Committer: paulk Committed: Mon Nov 13 12:44:06 2017 +1000 ---------------------------------------------------------------------- .../ConstructorCallTransformer.java | 40 ++++++++++++++------ .../stc/StaticTypeCheckingVisitor.java | 27 +++++++++---- .../groovy/transform/stc/BugsSTCTest.groovy | 31 ++++++++++++++- 3 files changed, 77 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/45aa4b31/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 854959b..6c42e78 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 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 () + appropriate setters + // replace call to (Map) or (this, Map) + // with a call to () or (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( @@ -97,19 +101,23 @@ public class ConstructorCallTransformer { private AsmClassGenerator acg; private final ClassNode declaringClass; private final MapExpression map; - private final ConstructorCallExpression orginalCall; + 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 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, "", "()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, "", desc, false); mv.visitVarInsn(ASTORE, tmpObj); // store it into tmp variable // load every field http://git-wip-us.apache.org/repos/asf/groovy/blob/45aa4b31/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 e650473..bd2a44f 100644 --- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -1927,9 +1927,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { MethodNode node; - if (args.length == 1 - && implementsInterfaceOrIsSubclassOf(args[0], MAP_TYPE) - && findMethod(receiver, "", ClassNode.EMPTY_ARRAY).size() == 1 + if (looksLikeNamedArgConstructor(receiver, args) + && findMethod(receiver, "", DefaultGroovyMethods.init(args)).size() == 1 && findMethod(receiver, "", args).isEmpty()) { // bean-style constructor node = typeCheckMapConstructor(call, receiver, arguments); @@ -1941,7 +1940,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } node = findMethodOrFail(call, receiver, "", 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); @@ -1951,17 +1950,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 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/45aa4b31/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)' + ''' + } }