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 0D35A200D4B for ; Mon, 13 Nov 2017 03:50:53 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 0A2B7160C08; Mon, 13 Nov 2017 02:50:53 +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 0130E160BF1 for ; Mon, 13 Nov 2017 03:50:51 +0100 (CET) Received: (qmail 7202 invoked by uid 500); 13 Nov 2017 02:50:51 -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 7193 invoked by uid 99); 13 Nov 2017 02:50:50 -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:50:50 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DEB28DFF4D; Mon, 13 Nov 2017 02:50:49 +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:50:49 -0000 Message-Id: <240c23f93a894dcea0b33c3783a7fe34@git.apache.org> X-Mailer: ASF-Git Admin Mailer 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) archived-at: Mon, 13 Nov 2017 02:50:53 -0000 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 Authored: Sun Nov 12 23:07:27 2017 +1000 Committer: paulk 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 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( @@ -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 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/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, "", 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); @@ -1943,7 +1942,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); @@ -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 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)' + ''' + } }