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 ade7ecb  GROOVY-7812: Static inner classes cannot be accessed from 
other files when running by 'groovy' command(closes #853)
ade7ecb is described below

commit ade7ecb915ece738193deaa667c54b8a483093a2
Author: Daniel Sun <sun...@apache.org>
AuthorDate: Wed Jan 16 21:54:22 2019 +0800

    GROOVY-7812: Static inner classes cannot be accessed from other files when 
running by 'groovy' command(closes #853)
---
 .../java/org/codehaus/groovy/ast/CompileUnit.java  | 38 +++++++++++
 .../codehaus/groovy/control/ResolveVisitor.java    | 74 +++++++++++++++++++++-
 .../groovy/bugs/groovy7812/Main.groovy             |  7 ++
 .../{Main.groovy => MainWithErrors.groovy}         |  2 +-
 .../groovy/bugs/groovy7812/Outer.groovy            | 12 ++++
 src/test/groovy/bugs/Groovy7812Bug.groovy          | 30 +++++++--
 6 files changed, 156 insertions(+), 7 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/CompileUnit.java 
b/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
index 2909569..137d079 100644
--- a/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
+++ b/src/main/java/org/codehaus/groovy/ast/CompileUnit.java
@@ -19,11 +19,13 @@
 package org.codehaus.groovy.ast;
 
 import groovy.lang.GroovyClassLoader;
+import groovy.transform.Internal;
 import org.codehaus.groovy.control.CompilerConfiguration;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
 import org.codehaus.groovy.syntax.SyntaxException;
 import org.codehaus.groovy.util.ListHashMap;
+import org.objectweb.asm.Opcodes;
 
 import java.security.CodeSource;
 import java.util.ArrayList;
@@ -51,6 +53,7 @@ public class CompileUnit implements NodeMetaDataHandler {
     private final Map<String, ClassNode> classesToCompile = new 
HashMap<String, ClassNode>();
     private final Map<String, SourceUnit> classNameToSource = new 
HashMap<String, SourceUnit>();
     private final Map<String, InnerClassNode> generatedInnerClasses = new 
HashMap();
+    private final Map<String, ConstructedOuterNestedClassNode> 
classesToResolve = new HashMap<>();
     private Map metaDataMap = null;
 
     public CompileUnit(GroovyClassLoader classLoader, CompilerConfiguration 
config) {
@@ -191,6 +194,23 @@ public class CompileUnit implements NodeMetaDataHandler {
         return Collections.unmodifiableMap(generatedInnerClasses);
     }
 
+    public Map<String, ClassNode> getClassesToCompile() {
+        return classesToCompile;
+    }
+
+    public Map<String, ConstructedOuterNestedClassNode> getClassesToResolve() {
+        return classesToResolve;
+    }
+
+    /**
+     * Add a constructed class node as a placeholder to resolve outer nested 
class further.
+     *
+     * @param cn the constructed class node
+     */
+    public void addClassNodeToResolve(ConstructedOuterNestedClassNode cn) {
+        classesToResolve.put(cn.getUnresolvedName(), cn);
+    }
+
     @Override
     public ListHashMap getMetaDataMap() {
         return (ListHashMap) metaDataMap;
@@ -200,4 +220,22 @@ public class CompileUnit implements NodeMetaDataHandler {
     public void setMetaDataMap(Map<?, ?> metaDataMap) {
         this.metaDataMap = metaDataMap;
     }
+
+    /**
+     * Represents a resolved type as a placeholder, SEE GROOVY-7812
+     */
+    @Internal
+    public static class ConstructedOuterNestedClassNode extends ClassNode {
+        private final ClassNode enclosingClassNode;
+
+        public ConstructedOuterNestedClassNode(ClassNode outer, String 
innerClassName) {
+            super(innerClassName, Opcodes.ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
+            this.enclosingClassNode = outer;
+            this.isPrimaryNode = false;
+        }
+
+        public ClassNode getEnclosingClassNode() {
+            return enclosingClassNode;
+        }
+    }
 }
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java 
b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index c5bb144..c6bd079 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -79,6 +79,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static 
org.codehaus.groovy.ast.CompileUnit.ConstructedOuterNestedClassNode;
 import static org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.inSamePackage;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.isDefaultVisibility;
@@ -147,6 +148,7 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
     }
 
 
+
     private static String replacePoints(String name) {
         return name.replace('.','$');
     }
@@ -341,9 +343,44 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
     private void resolveOrFail(ClassNode type, String msg, ASTNode node) {
         if (resolve(type)) return;
         if (resolveToInner(type)) return;
+        if (resolveToOuterNested(type)) return;
+
         addError("unable to resolve class " + type.getName() + " " + msg, 
node);
     }
 
+    // GROOVY-7812(#1): Static inner classes cannot be accessed from other 
files when running by 'groovy' command
+    // if the type to resolve is an inner class and it is in an outer class 
which is not resolved,
+    // we set the resolved type to a placeholder class node, i.e. a 
ConstructedOuterNestedClass instance
+    // when resolving the outer class later, we set the resolved type of 
ConstructedOuterNestedClass instance to the actual inner class node(SEE 
GROOVY-7812(#2))
+    private boolean resolveToOuterNested(ClassNode type) {
+        CompileUnit compileUnit = currentClass.getCompileUnit();
+        Map<String, ClassNode> classesToCompile = 
compileUnit.getClassesToCompile();
+
+        for (Map.Entry<String, ClassNode> entry : classesToCompile.entrySet()) 
{
+            String enclosingClassName = entry.getKey();
+            ClassNode enclosingClassNode = entry.getValue();
+
+            String outerNestedClassName = 
tryToConstructOuterNestedClassName(type, enclosingClassName);
+            if (null != outerNestedClassName) {
+                compileUnit.addClassNodeToResolve(new 
ConstructedOuterNestedClassNode(enclosingClassNode, outerNestedClassName));
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private String tryToConstructOuterNestedClassName(ClassNode type, String 
enclosingClassName) {
+        for (String typeName = type.getName(), ident = typeName; 
ident.contains("."); ) {
+            ident = ident.substring(0, ident.lastIndexOf("."));
+            if (enclosingClassName.endsWith(ident)) {
+                return enclosingClassName + 
typeName.substring(ident.length()).replace(".", "$");
+            }
+        }
+
+        return null;
+    }
+
     private void resolveOrFail(ClassNode type, ASTNode node, boolean 
prefereImports) {
         resolveGenericsTypes(type.getGenericsTypes());
         if (prefereImports && resolveAliasFromModule(type)) return;
@@ -794,10 +831,10 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
             }
             return true;
         }
+
         return false;
     }
 
-
     public Expression transform(Expression exp) {
         if (exp == null) return null;
         Expression ret;
@@ -1421,9 +1458,42 @@ public class ResolveVisitor extends 
ClassCodeExpressionTransformer {
 
         super.visitClass(node);
 
+        resolveOuterNestedClassFurther(node);
+
         currentClass = oldNode;
     }
-    
+
+    // GROOVY-7812(#2): Static inner classes cannot be accessed from other 
files when running by 'groovy' command
+    private void resolveOuterNestedClassFurther(ClassNode node) {
+        CompileUnit compileUnit = currentClass.getCompileUnit();
+
+        if (null == compileUnit) return;
+
+        Map<String, ConstructedOuterNestedClassNode> classesToResolve = 
compileUnit.getClassesToResolve();
+        List<String> resolvedInnerClassNameList = new LinkedList<>();
+
+        for (Map.Entry<String, ConstructedOuterNestedClassNode> entry : 
classesToResolve.entrySet()) {
+            String innerClassName = entry.getKey();
+            ConstructedOuterNestedClassNode constructedOuterNestedClass = 
entry.getValue();
+
+            // When the outer class is resolved, all inner classes are 
resolved too
+            if 
(node.getName().equals(constructedOuterNestedClass.getEnclosingClassNode().getName()))
 {
+                ClassNode innerClassNode = 
compileUnit.getClass(innerClassName); // find the resolved inner class
+
+                if (null == innerClassNode) {
+                    return; // "unable to resolve class" error can be thrown 
already, no need to `addError`, so just return
+                }
+
+                constructedOuterNestedClass.setRedirect(innerClassNode);
+                resolvedInnerClassNameList.add(innerClassName);
+            }
+        }
+
+        for (String innerClassName : resolvedInnerClassNameList) {
+            classesToResolve.remove(innerClassName);
+        }
+    }
+
     private void checkCyclicInheritance(ClassNode originalNode, ClassNode 
parentToCompare, ClassNode[] interfacesToCompare) {
         if(!originalNode.isInterface()) {
             if(parentToCompare == null) return;
diff --git a/src/test-resources/groovy/bugs/groovy7812/Main.groovy 
b/src/test-resources/groovy/bugs/groovy7812/Main.groovy
index 4242743..16d2fa4 100644
--- a/src/test-resources/groovy/bugs/groovy7812/Main.groovy
+++ b/src/test-resources/groovy/bugs/groovy7812/Main.groovy
@@ -20,3 +20,10 @@ package groovy.bugs.groovy7812
 
 assert new Outer()
 assert new Outer.Inner()
+assert new groovy.bugs.groovy7812.Outer.Inner()
+assert new Outer.Inner.Innest()
+assert new groovy.bugs.groovy7812.Outer.Inner.Innest()
+assert "2" == new Outer.Inner2().innerName
+assert "3" == new Outer.Inner3().innerName
+assert "1.Innest" == new Outer.Inner.Innest().name
+assert "3.Innest" == new Outer.Inner3.Innest().name
diff --git a/src/test-resources/groovy/bugs/groovy7812/Main.groovy 
b/src/test-resources/groovy/bugs/groovy7812/MainWithErrors.groovy
similarity index 96%
copy from src/test-resources/groovy/bugs/groovy7812/Main.groovy
copy to src/test-resources/groovy/bugs/groovy7812/MainWithErrors.groovy
index 4242743..091a59e 100644
--- a/src/test-resources/groovy/bugs/groovy7812/Main.groovy
+++ b/src/test-resources/groovy/bugs/groovy7812/MainWithErrors.groovy
@@ -19,4 +19,4 @@
 package groovy.bugs.groovy7812
 
 assert new Outer()
-assert new Outer.Inner()
+assert new Outer.Inner123()
diff --git a/src/test-resources/groovy/bugs/groovy7812/Outer.groovy 
b/src/test-resources/groovy/bugs/groovy7812/Outer.groovy
index ec2103b..753bbe5 100644
--- a/src/test-resources/groovy/bugs/groovy7812/Outer.groovy
+++ b/src/test-resources/groovy/bugs/groovy7812/Outer.groovy
@@ -20,5 +20,17 @@ package groovy.bugs.groovy7812
 
 class Outer {
     static class Inner {
+        static class Innest {
+            def name = "1.Innest"
+        }
+    }
+    static class Inner2 {
+        def innerName = "2"
+    }
+    static class Inner3 {
+        def innerName = "3"
+        static class Innest {
+            def name = "3.Innest"
+        }
     }
 }
\ No newline at end of file
diff --git a/src/test/groovy/bugs/Groovy7812Bug.groovy 
b/src/test/groovy/bugs/Groovy7812Bug.groovy
index 2960af6..ac94ee6 100644
--- a/src/test/groovy/bugs/Groovy7812Bug.groovy
+++ b/src/test/groovy/bugs/Groovy7812Bug.groovy
@@ -19,12 +19,34 @@
 package groovy.bugs
 
 import org.codehaus.groovy.tools.GroovyStarter
-import org.junit.Ignore
 
-@Ignore('To be fixed')
 class Groovy7812Bug extends GroovyTestCase {
-    void test() {
+    void testResolvingOuterNestedClass() {
         def mainScriptPath = new 
File(this.getClass().getResource('/groovy/bugs/groovy7812/Main.groovy').toURI()).absolutePath
-        new GroovyStarter().main(["--main", "groovy.ui.GroovyMain", 
mainScriptPath] as String[])
+        runScript(mainScriptPath)
+    }
+
+//   Even if try to catch `Throwable`, the expected error is thrown all the 
same..., as a result, the test fails due to the weired problem...
+//
+//    org.codehaus.groovy.control.MultipleCompilationErrorsException: startup 
failed:
+//D:\_APPS\git_apps\groovy\out\test\resources\groovy\bugs\groovy7812\MainWithErrors.groovy:
 22: unable to resolve class Outer.Inner123
+// @ line 22, column 8.
+//   assert new Outer.Inner123()
+//          ^
+//
+//1 error
+//
+//    void testUnexistingInnerClass() {
+//        try {
+//            def mainScriptPath = new 
File(this.getClass().getResource('/groovy/bugs/groovy7812/MainWithErrors.groovy').toURI()).absolutePath
+//            runScript(mainScriptPath)
+//        } catch (Throwable t) {
+//            assert t.getMessage().contains('unable to resolve class 
Outer.Inner123')
+//        }
+//    }
+
+
+    static void runScript(String path) {
+        GroovyStarter.main(new String[] { "--main", "groovy.ui.GroovyMain", 
path })
     }
 }

Reply via email to