Skip to content

Commit

Permalink
GROOVY-7812: Static inner classes cannot be accessed from other files…
Browse files Browse the repository at this point in the history
… when running by 'groovy' command(closes #853)
  • Loading branch information
daniellansun committed Jan 16, 2019
1 parent 50b77b2 commit ade7ecb
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 6 deletions.
38 changes: 38 additions & 0 deletions src/main/java/org/codehaus/groovy/ast/CompileUnit.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -191,6 +194,23 @@ public Map<String, InnerClassNode> getGeneratedInnerClasses() {
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;
Expand All @@ -200,4 +220,22 @@ public ListHashMap getMetaDataMap() {
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;
}
}
}
74 changes: 72 additions & 2 deletions src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
Expand Up @@ -79,6 +79,7 @@
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;
Expand Down Expand Up @@ -147,6 +148,7 @@ public String setName(String name) {
}



private static String replacePoints(String name) {
return name.replace('.','$');
}
Expand Down Expand Up @@ -341,9 +343,44 @@ private boolean checkInnerTypeVisibility(ClassNode enclosingType, ClassNode inne
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;
Expand Down Expand Up @@ -794,10 +831,10 @@ private boolean resolveToOuter(ClassNode type) {
}
return true;
}

return false;
}


public Expression transform(Expression exp) {
if (exp == null) return null;
Expression ret;
Expand Down Expand Up @@ -1421,9 +1458,42 @@ public void visitClass(ClassNode node) {

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;
Expand Down
7 changes: 7 additions & 0 deletions src/test-resources/groovy/bugs/groovy7812/Main.groovy
Expand Up @@ -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
22 changes: 22 additions & 0 deletions src/test-resources/groovy/bugs/groovy7812/MainWithErrors.groovy
@@ -0,0 +1,22 @@
/*
* 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.groovy7812

assert new Outer()
assert new Outer.Inner123()
12 changes: 12 additions & 0 deletions src/test-resources/groovy/bugs/groovy7812/Outer.groovy
Expand Up @@ -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"
}
}
}
30 changes: 26 additions & 4 deletions src/test/groovy/bugs/Groovy7812Bug.groovy
Expand Up @@ -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 })
}
}

0 comments on commit ade7ecb

Please sign in to comment.