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

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 0917bfc  GROOVY-8947: Fail to resolve non-static inner class outside 
of outer class(closes #850)
0917bfc is described below

commit 0917bfcfbdbd7cbcd6dba0ba903e6d43b5221953
Author: Daniel Sun <sun...@apache.org>
AuthorDate: Mon Jan 7 21:56:34 2019 +0800

    GROOVY-8947: Fail to resolve non-static inner class outside of outer 
class(closes #850)
---
 .../java/org/codehaus/groovy/ast/CompileUnit.java  |  2 +-
 .../codehaus/groovy/control/ResolveVisitor.java    | 34 +++++++-
 src/test/groovy/bugs/Groovy8947Bug.groovy          | 92 ++++++++++++++++++++++
 3 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/CompileUnit.java 
b/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
index a453bb1..2909569 100644
--- a/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
+++ b/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
@@ -90,7 +90,7 @@ public class CompileUnit implements NodeMetaDataHandler {
     /**
      * @return a list of all the classes in each module in the compilation unit
      */
-    public List getClasses() {
+    public List<ClassNode> getClasses() {
         List<ClassNode> answer = new ArrayList<ClassNode>();
         for (ModuleNode module : modules) {
             answer.addAll(module.getClasses());
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java 
b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index aabcaf3..c5bb144 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -40,6 +40,7 @@ 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;
@@ -108,6 +109,7 @@ 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<GenericsTypeName, GenericsType>();
     private final Set<FieldNode> fieldTypesChecked = new HashSet<FieldNode>();
     private boolean checkingVariableTypeInDeclaration = false;
@@ -413,6 +415,12 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
             if (setRedirect(type, classToCheck)) return true;
         }
 
+        // GROOVY-8947: Fail to resolve non-static inner class outside of 
outer class
+        ClassNode possibleOuterClassNode = possibleOuterClassNodeMap.get(type);
+        if (null != possibleOuterClassNode) {
+            if (setRedirect(type, possibleOuterClassNode)) 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
@@ -1172,6 +1180,8 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
     }
 
     protected Expression 
transformConstructorCallExpression(ConstructorCallExpression cce) {
+        findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(cce);
+
         ClassNode type = cce.getType();
         resolveOrFail(type, cce);
         if (Modifier.isAbstract(type.getModifiers())) {
@@ -1181,6 +1191,28 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
         return cce.transformExpression(this);
     }
 
+    private void 
findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(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(ClassNode node) {
         return (node.isInterface() ? "interface" : "class") + " '" + 
node.getName() + "'";
     }
@@ -1386,7 +1418,7 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
         }
 
         checkCyclicInheritance(node, node.getUnresolvedSuperClass(), 
node.getInterfaces());
-        
+
         super.visitClass(node);
 
         currentClass = oldNode;
diff --git a/src/test/groovy/bugs/Groovy8947Bug.groovy 
b/src/test/groovy/bugs/Groovy8947Bug.groovy
new file mode 100644
index 0000000..5c858b3
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8947Bug.groovy
@@ -0,0 +1,92 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+class Groovy8947Bug extends GroovyTestCase {
+    void testResolvingNonStaticInnerClass() {
+        assertScript '''
+            public class Computer {
+                public class Cpu {
+                    int coreNumber
+            
+                    public Cpu(int coreNumber) {
+                        this.coreNumber = coreNumber
+                    }
+                }
+                public static newCpuInstance(int coreNumber) {
+                    return new Computer().new Cpu(coreNumber)
+                }
+            }
+            
+            assert 4 == new Computer().new Cpu(4).coreNumber
+            
+            assert 4 == Computer.newCpuInstance(4).coreNumber
+            assert 0 == new HashSet(new ArrayList()).size()
+        '''
+    }
+
+    void testResolvingNonStaticInnerClass2() {
+        assertScript '''
+            public class Computer {
+                public class Cpu {
+                    int coreNumber
+            
+                    public Cpu(int coreNumber) {
+                        this.coreNumber = coreNumber
+                    }
+                }
+                
+                static newComputerInstance() {
+                    return new Computer()
+                }
+                
+                static newCpuInstance(int coreNumber) {
+                    // `new Cpu(coreNumber)` is inside of the outer class 
`Computer`, so we can resolve `Cpu`
+                    return newComputerInstance().new Cpu(coreNumber)
+                } 
+            }
+            
+            assert 4 == Computer.newCpuInstance(4).coreNumber 
+        '''
+    }
+
+    void testResolvingNonStaticInnerClass3() {
+        def errMsg = shouldFail '''
+            public class Computer {
+                public class Cpu {
+                    int coreNumber
+            
+                    public Cpu(int coreNumber) {
+                        this.coreNumber = coreNumber
+                    }
+                }
+            }
+            
+            def newComputerInstance() {
+                return new Computer()
+            }
+            
+            // `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 
+        '''
+
+        assert errMsg.contains('unable to resolve class Cpu')
+    }
+}

Reply via email to