[GitHub] asfgit closed pull request #850: GROOVY-8947: Fail to resolve non-static inner class outside of outer …

[GitBox <gi...@apache.org>]

asfgit closed pull request #850: GROOVY-8947: Fail to resolve non-static inner class outside of outer …
URL: https://github.com/apache/groovy/pull/850
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/src/main/java/org/codehaus/groovy/ast/CompileUnit.java b/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
index a453bb16c7..2909569eaf 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 ClassNode getClass(String name) {
     /**
      * @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 aabcaf32fe..c5bb144657 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.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 @@
     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 @@ private boolean resolveNestedClass(ClassNode type) {
             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 @@ protected Expression transformClosureExpression(ClosureExpression ce) {
     }
 
     protected Expression transformConstructorCallExpression(ConstructorCallExpression cce) {
+        findPossibleOuterClassNodeForNonStaticInnerClassInstantiation(cce);
+
         ClassNode type = cce.getType();
         resolveOrFail(type, cce);
         if (Modifier.isAbstract(type.getModifiers())) {
@@ -1181,6 +1191,28 @@ protected Expression transformConstructorCallExpression(ConstructorCallExpressio
         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 void visitClass(ClassNode node) {
         }
 
         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 0000000000..5c858b3e6e
--- /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')
+    }
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services