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/08/22 16:09:04 UTC

[groovy] branch GROOVY-8947 created (now bf3e88e)

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

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


      at bf3e88e  GROOVY-8947: convert "new A().new B()" into "new A.B(new A())"

This branch includes the following new commits:

     new bf3e88e  GROOVY-8947: convert "new A().new B()" into "new A.B(new A())"

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[groovy] 01/01: GROOVY-8947: convert "new A().new B()" into "new A.B(new A())"

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit bf3e88ee0bddbabbd2ed21f2a909dfc1ff873d93
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Aug 22 10:53:08 2021 -0500

    GROOVY-8947: convert "new A().new B()" into "new A.B(new A())"
---
 .../apache/groovy/parser/antlr4/AstBuilder.java    |  7 +-
 .../codehaus/groovy/control/ResolveVisitor.java    | 32 --------
 src/test/groovy/bugs/Groovy8947.groovy             | 86 +++++++++++-----------
 3 files changed, 49 insertions(+), 76 deletions(-)

diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 9a7aa5d..ae5a123 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -3448,7 +3448,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
             Expression arguments = this.visitArguments(ctx.arguments());
             Expression enclosingInstanceExpression = ctx.getNodeMetaData(ENCLOSING_INSTANCE_EXPRESSION);
 
-            if (null != enclosingInstanceExpression) {
+            if (enclosingInstanceExpression != null) {
                 if (arguments instanceof ArgumentListExpression) {
                     ((ArgumentListExpression) arguments).getExpressions().add(0, enclosingInstanceExpression);
                 } else if (arguments instanceof TupleExpression) {
@@ -3458,6 +3458,9 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
                 } else {
                     throw createParsingFailedException("Unsupported arguments", arguments); // should never reach here
                 }
+                if (enclosingInstanceExpression instanceof ConstructorCallExpression && classNode.getName().indexOf('.') < 0) {
+                    classNode.setName(enclosingInstanceExpression.getType().getName() + '.' + classNode.getName()); // GROOVY-8947
+                }
             }
 
             if (asBoolean(ctx.anonymousInnerClassDeclaration())) {
@@ -3465,7 +3468,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
                 InnerClassNode anonymousInnerClassNode = this.visitAnonymousInnerClassDeclaration(ctx.anonymousInnerClassDeclaration());
 
                 List<InnerClassNode> anonymousInnerClassList = anonymousInnerClassesDefinedInMethodStack.peek();
-                if (null != anonymousInnerClassList) { // if the anonymous class is created in a script, no anonymousInnerClassList is available.
+                if (anonymousInnerClassList != null) { // if the anonymous class is created in a script, no anonymousInnerClassList is available.
                     anonymousInnerClassList.add(anonymousInnerClassNode);
                 }
 
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 8d2c254..d4ce72e 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -22,7 +22,6 @@ import groovy.lang.Tuple2;
 import org.apache.groovy.ast.tools.ExpressionUtils;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
 import org.codehaus.groovy.ast.ClassHelper;
@@ -42,7 +41,6 @@ import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.Variable;
 import org.codehaus.groovy.ast.VariableScope;
 import org.codehaus.groovy.ast.expr.AnnotationConstantExpression;
-import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.CastExpression;
 import org.codehaus.groovy.ast.expr.ClassExpression;
@@ -113,7 +111,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
     private boolean inPropertyExpression = false;
     private boolean inClosure = false;
 
-    private final Map<ClassNode, ClassNode> possibleOuterClassNodeMap = new HashMap<>();
     private Map<GenericsTypeName, GenericsType> genericParameterNames = new HashMap<>();
     private final Set<FieldNode> fieldTypesChecked = new HashSet<>();
     private boolean checkingVariableTypeInDeclaration;
@@ -528,10 +525,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
             // GROOVY-9866: unresolvable interfaces
         }
 
-        // GROOVY-8947: non-static inner class outside of outer class
-        cn = possibleOuterClassNodeMap.get(type);
-        if (cn != null && setRedirect(type, cn)) return true;
-
         // Another case we want to check here is if we are in a
         // nested class A$B$C and want to access B without
         // qualifying it by A.B. A alone will work, since that
@@ -1235,8 +1228,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
     }
 
     protected Expression transformConstructorCallExpression(final ConstructorCallExpression cce) {
-        findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(cce);
-
         ClassNode type = cce.getType();
         if (cce.isUsingAnonymousInnerClass()) { // GROOVY-9642
             resolveOrFail(type.getUnresolvedSuperClass(false), type);
@@ -1250,28 +1241,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
         return cce.transformExpression(this);
     }
 
-    private void findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(final ConstructorCallExpression cce) {
-        // GROOVY-8947: Fail to resolve non-static inner class outside of outer class
-        // `new Computer().new Cpu(4)` will be parsed to `new Cpu(new Computer(), 4)`
-        // so non-static inner class instantiation expression's first argument is a constructor call of outer class
-        // but the first argument is constructor call can not be non-static inner class instantiation expression, e.g.
-        // `new HashSet(new ArrayList())`, so we add "possible" to the variable name
-        Expression argumentExpression = cce.getArguments();
-        if (argumentExpression instanceof ArgumentListExpression) {
-            ArgumentListExpression argumentListExpression = (ArgumentListExpression) argumentExpression;
-            List<Expression> expressionList = argumentListExpression.getExpressions();
-            if (!expressionList.isEmpty()) {
-                Expression firstExpression = expressionList.get(0);
-
-                if (firstExpression instanceof ConstructorCallExpression) {
-                    ConstructorCallExpression constructorCallExpression = (ConstructorCallExpression) firstExpression;
-                    ClassNode possibleOuterClassNode = constructorCallExpression.getType();
-                    possibleOuterClassNodeMap.put(cce.getType(), possibleOuterClassNode);
-                }
-            }
-        }
-    }
-
     private static String getDescription(final ClassNode node) {
         return (node.isInterface() ? "interface" : "class") + " '" + node.getName() + "'";
     }
@@ -1680,6 +1649,5 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
             genericsType.setResolved(genericsType.getType().isResolved());
         }
         return genericsType.isResolved();
-
     }
 }
diff --git a/src/test/groovy/bugs/Groovy8947.groovy b/src/test/groovy/bugs/Groovy8947.groovy
index f11e565..cba5a65 100644
--- a/src/test/groovy/bugs/Groovy8947.groovy
+++ b/src/test/groovy/bugs/Groovy8947.groovy
@@ -30,73 +30,75 @@ final class Groovy8947 {
     @Test
     void testResolvingNonStaticInnerClass() {
         assertScript '''
-            public class Computer {
-                public class Cpu {
-                    int coreNumber
-                    public Cpu(int coreNumber) {
-                        this.coreNumber = coreNumber
-                    }
+            class Foo {
+                @groovy.transform.TupleConstructor(defaults=false)
+                class Bar {
+                    def baz
                 }
-                public static newCpuInstance(int coreNumber) {
-                    return new Computer().new Cpu(coreNumber)
+                static newBar(def baz) {
+                    new Foo().new Bar(baz)
                 }
             }
 
-            assert 4 == new Computer().new Cpu(4).coreNumber
-            assert 4 == Computer.newCpuInstance(4).coreNumber
-            assert 0 == new HashSet(new ArrayList()).size()
-            
-            def coreNumber = new Computer().new Cpu(4).coreNumber
-            assert 4 == coreNumber
-            def cpu = new Computer().new Cpu(4)
-            assert 4 == cpu.coreNumber
+            assert Foo.newBar('baz').baz == 'baz'
+            assert new Foo().new Bar('baz').baz == 'baz'
         '''
     }
 
     @Test
     void testResolvingNonStaticInnerClass2() {
         assertScript '''
-            public class Computer {
-                public class Cpu {
-                    int coreNumber
-                    public Cpu(int coreNumber) {
-                        this.coreNumber = coreNumber
-                    }
+            class Foo {
+                @groovy.transform.TupleConstructor(defaults=false)
+                class Bar {
+                    def baz
                 }
-                static newComputerInstance() {
-                    return new Computer()
+                static newFoo() {
+                    new Foo()
                 }
-                static newCpuInstance(int coreNumber) {
-                    // `new Cpu(coreNumber)` is inside of the outer class `Computer`, so we can resolve `Cpu`
-                    return newComputerInstance().new Cpu(coreNumber)
+                static newBar(def baz) {
+                    return newFoo().new Bar(baz)
                 }
             }
 
-            assert 4 == Computer.newCpuInstance(4).coreNumber
+            assert Foo.newBar('baz').baz == 'baz'
         '''
     }
 
     @Test
     void testResolvingNonStaticInnerClass3() {
-        def err = shouldFail '''
-            public class Computer {
-                public class Cpu {
-                    int coreNumber
-
-                    public Cpu(int coreNumber) {
-                        this.coreNumber = coreNumber
-                    }
+        assertScript '''
+            class Foo {
+                @groovy.transform.TupleConstructor(defaults=false)
+                class Bar {
+                    def baz
+                }
+                static newFoo() {
+                    new Foo()
                 }
             }
-            def newComputerInstance() {
-                return new Computer()
+
+            assert Foo.newFoo().new Foo.Bar('baz').baz == 'baz'
+        '''
+    }
+
+    @Test
+    void testResolvingNonStaticInnerClass4() {
+        def err = shouldFail '''
+            class Foo {
+                @groovy.transform.TupleConstructor(defaults=false)
+                class Bar {
+                    def baz
+                }
+                static newFoo() {
+                    new Foo()
+                }
             }
 
-            // `new Cpu(4)` is outside of outer class `Computer` and the return type of `newComputerInstance()` can not be resolved,
-            // so we does not support this syntax outside of outer class
-            assert 4 == newComputerInstance().new Cpu(4).coreNumber
+            // this form isn't supported outside of enclosing class
+            Foo.newFoo().new Bar('baz')
         '''
 
-        assert err =~ 'unable to resolve class Cpu'
+        assert err =~ 'unable to resolve class Bar'
     }
 }