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 2022/02/19 19:08:53 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-9281: allow package-private for resolved nested type
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new b920a58 GROOVY-9281: allow package-private for resolved nested type
b920a58 is described below
commit b920a581756fb627fd9f82da719d7eb0339ae8a2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Feb 19 13:00:05 2022 -0600
GROOVY-9281: allow package-private for resolved nested type
2_5_X backport
---
.../codehaus/groovy/control/ResolveVisitor.java | 87 +++++++++++-----------
.../groovy/bugs/groovy8531/Groovy8531Bug.groovy | 73 ++++++++++++------
src/test/groovy/bugs/groovy8531/Reducer.java | 12 ++-
3 files changed, 103 insertions(+), 69 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index d41a73c..cce4f72 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -20,6 +20,7 @@ package org.codehaus.groovy.control;
import groovy.lang.Tuple2;
import org.apache.groovy.ast.tools.ExpressionUtils;
+import org.apache.groovy.internal.util.Predicate;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotatedNode;
@@ -71,10 +72,11 @@ import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Modifier;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
+import java.util.ListIterator;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import static org.codehaus.groovy.ast.CompileUnit.ConstructedOuterNestedClassNode;
@@ -470,21 +472,11 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private boolean resolveNestedClass(ClassNode type) {
if (type instanceof ConstructedNestedClass || type instanceof ConstructedClassWithPackage) return false;
+
// we have for example a class name A, are in class X
// and there is a nested class A$X. we want to be able
// to access that class directly, so A becomes a valid
// name in X.
- // GROOVY-4043: Do this check up the hierarchy, if needed
- Map<String, ClassNode> hierClasses = new LinkedHashMap<String, ClassNode>();
- for(ClassNode classToCheck = currentClass; classToCheck != ClassHelper.OBJECT_TYPE;
- classToCheck = classToCheck.getSuperClass()) {
- if(classToCheck == null || hierClasses.containsKey(classToCheck.getName())) break;
- hierClasses.put(classToCheck.getName(), classToCheck);
- }
-
- for (ClassNode classToCheck : hierClasses.values()) {
- if (setRedirect(type, classToCheck)) 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
@@ -492,10 +484,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
// is the qualified (minus package) name of that class
// anyway.
- // That means if the current class is not an InnerClassNode
- // there is nothing to be done.
- if (!(currentClass instanceof InnerClassNode)) return false;
-
// since we have B and want to get A we start with the most
// outer class, put them together and then see if that does
// already exist. In case of B from within A$B we are done
@@ -503,35 +491,50 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
// A.B.C.D.E.F and accessing E from F we test A$E=failed,
// A$B$E=failed, A$B$C$E=fail, A$B$C$D$E=success
- LinkedList<ClassNode> outerClasses = new LinkedList<ClassNode>();
- ClassNode outer = currentClass.getOuterClass();
- while (outer!=null) {
- outerClasses.addFirst(outer);
- outer = outer.getOuterClass();
+ Set<ClassNode> cycleCheck = new HashSet<>(); cycleCheck.add(ClassHelper.OBJECT_TYPE);
+ for (ClassNode cn = currentClass; cn != null && cycleCheck.add(cn); cn = cn.getSuperClass()) {
+ if (setRedirect(type, cn)) return true;
}
- // most outer class is now element 0
- for (ClassNode testNode : outerClasses) {
- if (setRedirect(type, testNode)) return true;
+ List<ClassNode> outerClasses = currentClass.getOuterClasses();
+ if (!outerClasses.isEmpty()) {
+ for (ListIterator<ClassNode> it = outerClasses.listIterator(outerClasses.size()); it.hasPrevious(); ) {
+ if (setRedirect(type, it.previous())) return true;
+ }
}
-
return false;
}
- private boolean setRedirect(ClassNode type, ClassNode classToCheck) {
- ClassNode val = new ConstructedNestedClass(classToCheck, type.getName());
- if (resolveFromCompileUnit(val)) {
- type.setRedirect(val);
- return true;
- }
- // also check interfaces in case we have interfaces with nested classes
- for (ClassNode next : classToCheck.getAllInterfaces()) {
- if (type.getName().contains(next.getName())) continue;
- val = new ConstructedNestedClass(next, type.getName());
- if (resolve(val, false, false, false)) {
- type.setRedirect(val);
+ private boolean setRedirect(final ClassNode type, ClassNode classToCheck) {
+ final String typeName = type.getName();
+
+ Predicate<ClassNode> resolver = new Predicate<ClassNode>() {
+ @Override
+ public boolean test(ClassNode maybeOuter) {
+ if (!typeName.equals(maybeOuter.getName())) {
+ ClassNode maybeNested = new ConstructedNestedClass(maybeOuter, typeName);
+ if (resolveFromCompileUnit(maybeNested) || resolveToOuter(maybeNested)) {
+ type.setRedirect(maybeNested);
+ return true;
+ }
+ }
+ return false;
+ }
+ };
+
+ if (resolver.test(classToCheck)) {
+ if (currentClass != classToCheck && !currentClass.getOuterClasses().contains(classToCheck) && !isVisibleNestedClass(type.redirect(), currentClass)) {
+ type.setRedirect(null);
+ } else {
return true;
}
}
+ if (classToCheck.getInterfaces().length > 0) {
+ for (ClassNode face : classToCheck.getAllInterfaces()) {
+ if (resolver.test(face)) {
+ return true;
+ }
+ }
+ }
return false;
}
@@ -1040,12 +1043,10 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
return ret;
}
- private boolean isVisibleNestedClass(ClassNode type, ClassNode ceType) {
- if (!type.isRedirectNode()) return false;
- ClassNode redirect = type.redirect();
- if (Modifier.isPublic(redirect.getModifiers()) || Modifier.isProtected(redirect.getModifiers())) return true;
- // package local
- return isDefaultVisibility(redirect.getModifiers()) && inSamePackage(ceType, redirect);
+ private static boolean isVisibleNestedClass(ClassNode innerType, ClassNode outerType) {
+ int modifiers = innerType.getModifiers();
+ return Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers)
+ || (!Modifier.isPrivate(modifiers) && Objects.equals(innerType.getPackageName(), outerType.getPackageName()));
}
private boolean directlyImplementsTrait(ClassNode trait) {
diff --git a/src/test/groovy/bugs/groovy8531/Groovy8531Bug.groovy b/src/test/groovy/bugs/groovy8531/Groovy8531Bug.groovy
index 53a71f2..d5650ee 100644
--- a/src/test/groovy/bugs/groovy8531/Groovy8531Bug.groovy
+++ b/src/test/groovy/bugs/groovy8531/Groovy8531Bug.groovy
@@ -19,60 +19,87 @@
package groovy.bugs.groovy8531
class Groovy8531Bug extends GroovyTestCase {
+
void testPublicAndProtectedInnerType() {
assertScript '''
package groovy.bugs.groovy8531
+
class Example extends Reducer {
- public void reduce(PublicContext context) {}
+ public void reduce1(PublicContext context) {}
public void reduce2(ProtectedContext context) {}
- public void reduce3(PublicStaticContext context) {}
- public void reduce4(ProtectedStaticContext context) {}
-
- public void reduce5(PublicBaseContext context) {}
- public void reduce6(ProtectedBaseContext context) {}
- public void reduce7(PublicStaticBaseContext context) {}
- public void reduce8(ProtectedStaticBaseContext context) {}
-
- public void reduce9(InterfaceContext context) {}
-
+ public void reduce3(PackagePrivateContext context) {}
+
+ public void reduce4(PublicStaticContext context) {}
+ public void reduce5(ProtectedStaticContext context) {}
+ public void reduce6(PackagePrivateStaticContext context) {}
+
+ public void reduce7(PublicBaseContext context) {}
+ public void reduce8(ProtectedBaseContext context) {}
+ public void reduce9(PackagePrivateBaseContext context) {}
+
+ public void reduce10(PublicStaticBaseContext context) {}
+ public void reduce11(ProtectedStaticBaseContext context) {}
+ public void reduce12(PackagePrivateStaticBaseContext context) {}
+
+ public void reduce13(InterfaceContext context) {}
+
public boolean isDynamic(Type type) {
return Type.DYNAMIC == type
}
}
-
- new Example().reduce(null)
+
+ new Example().reduce1(null)
+ new Example().reduce2(null)
new Example().reduce2(null)
new Example().reduce3(null)
new Example().reduce4(null)
-
new Example().reduce5(null)
new Example().reduce6(null)
new Example().reduce7(null)
new Example().reduce8(null)
-
new Example().reduce9(null)
-
+ new Example().reduce10(null)
+ new Example().reduce11(null)
+ new Example().reduce12(null)
+ new Example().reduce13(null)
+
assert new Example().isDynamic(Reducer.Type.DYNAMIC)
'''
}
- void testPrivateInnerType() {
- def errMsg = shouldFail '''
+ void testPrivateInnerType1() {
+ def err = shouldFail '''
package groovy.bugs.groovy8531
+
class Example extends Reducer {
- public void reduce3(PrivateContext context) {}
+ void reduce(PrivateContext context) {}
}
'''
- assert errMsg.contains('unable to resolve class PrivateContext')
+ assert err.contains('unable to resolve class PrivateContext')
}
void testPrivateInnerType2() {
- def errMsg = shouldFail '''
+ def err = shouldFail '''
package groovy.bugs.groovy8531
+
+ class Example extends Reducer {
+ void reduce(PrivateBaseContext context) {}
+ }
+ '''
+ assert err.contains('unable to resolve class PrivateBaseContext')
+ }
+
+ void testPackagePrivateInnerType() {
+ def err = shouldFail '''
+ package groovy.bugs.groovy9281
+
+ import groovy.bugs.groovy8531.Reducer
+
class Example extends Reducer {
- public void reduce3(PrivateBaseContext context) {}
+ void reduce(PackagePrivateContext context) {}
}
'''
- assert errMsg.contains('unable to resolve class PrivateBaseContext')
+
+ assert err.contains('unable to resolve class PackagePrivateContext')
}
}
diff --git a/src/test/groovy/bugs/groovy8531/Reducer.java b/src/test/groovy/bugs/groovy8531/Reducer.java
index 8652196..64a12c4 100644
--- a/src/test/groovy/bugs/groovy8531/Reducer.java
+++ b/src/test/groovy/bugs/groovy8531/Reducer.java
@@ -25,19 +25,25 @@ interface Reducable {
class BaseReducer {
public abstract class PublicBaseContext {}
protected abstract class ProtectedBaseContext {}
+ /*package*/ abstract class PackagePrivateBaseContext {}
+
public static abstract class PublicStaticBaseContext {}
protected static abstract class ProtectedStaticBaseContext {}
+ /*package*/ static abstract class PackagePrivateStaticBaseContext {}
+
private abstract class PrivateBaseContext {}
}
public class Reducer extends BaseReducer implements Reducable {
public abstract class PublicContext {}
protected abstract class ProtectedContext {}
+ /*package*/ abstract class PackagePrivateContext {}
+
public static abstract class PublicStaticContext {}
protected static abstract class ProtectedStaticContext {}
+ /*package*/ static abstract class PackagePrivateStaticContext {}
+
private abstract class PrivateContext {}
- public enum Type {
- DYNAMIC, STATIC
- }
+ public enum Type { DYNAMIC, STATIC }
}