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/04/25 15:22:59 UTC
[groovy] branch master updated: GROOVY-10592: STC: add error for indirect static interface method access
This is an automated email from the ASF dual-hosted git repository.
emilles 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 8fa6d43a65 GROOVY-10592: STC: add error for indirect static interface method access
8fa6d43a65 is described below
commit 8fa6d43a6515e7ddfc419f259d247254ddbd72c2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Apr 25 10:16:21 2022 -0500
GROOVY-10592: STC: add error for indirect static interface method access
interface I {
static m() {}
}
I.m() // qualifier required
---
.../transform/stc/StaticTypeCheckingVisitor.java | 100 +++++++++++++--------
src/test/groovy/bugs/Groovy8579.groovy | 40 +++++++++
2 files changed, 103 insertions(+), 37 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 248e1d3617..5369c505df 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -557,9 +557,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
* Checks for private method call from inner or outer class.
*/
private void checkOrMarkPrivateAccess(final Expression source, final MethodNode mn) {
- if (mn == null) {
- return;
- }
ClassNode declaringClass = mn.getDeclaringClass();
ClassNode enclosingClassNode = typeCheckingContext.getEnclosingClassNode();
if (declaringClass != enclosingClassNode || typeCheckingContext.getEnclosingClosure() != null) {
@@ -569,7 +566,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (packageName == null) {
packageName = "";
}
- if ((Modifier.isPrivate(mods) && sameModule)) {
+ if (Modifier.isPrivate(mods) && sameModule) {
addPrivateFieldOrMethodAccess(source, declaringClass, PV_METHODS_ACCESS, mn);
} else if (Modifier.isProtected(mods) && !packageName.equals(enclosingClassNode.getPackageName())
&& !implementsInterfaceOrIsSubclassOf(enclosingClassNode, declaringClass)) {
@@ -1792,34 +1789,36 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
/**
- * This method is used to filter search results in which null means "no match",
- * to filter out illegal access to instance members from a static context.
+ * Filters search result to prevent access to instance members from a static
+ * context.
* <p>
- * Return null if the given member is not static, but we want to access in
- * a static way (staticOnly=true). If we want to access in a non-static way
- * we always return the member, since then access to static members and
+ * Return null if the given member is not static, but we want to access in a
+ * static way (staticOnly=true). If we want to access in a non-static way we
+ * always return the member, since then access to static members and
* non-static members is allowed.
+ *
+ * @return {@code member} or null
*/
- @SuppressWarnings("unchecked")
private <T> T allowStaticAccessToMember(final T member, final boolean staticOnly) {
- if (member == null) return null;
- if (!staticOnly) return member;
+ if (member == null || !staticOnly) return member;
+
+ if (member instanceof List) {
+ @SuppressWarnings("unchecked")
+ T list = (T) ((List<MethodNode>) member).stream()
+ .map(m -> allowStaticAccessToMember(m, true))
+ .filter(Objects::nonNull).collect(Collectors.toList());
+ return list;
+ }
+
boolean isStatic;
- if (member instanceof Variable) {
- Variable v = (Variable) member;
- isStatic = Modifier.isStatic(v.getModifiers());
- } else if (member instanceof List) {
- List<MethodNode> list = (List<MethodNode>) member;
- if (list.size() == 1) {
- return (T) Collections.singletonList(allowStaticAccessToMember(list.get(0), staticOnly));
- }
- return (T) Collections.emptyList();
+ if (member instanceof FieldNode) {
+ isStatic = ((FieldNode) member).isStatic();
+ } else if (member instanceof MethodNode) {
+ isStatic = ((MethodNode) member).isStatic();
} else {
- MethodNode mn = (MethodNode) member;
- isStatic = mn.isStatic();
+ isStatic = ((PropertyNode) member).isStatic();
}
- if (staticOnly && !isStatic) return null;
- return member;
+ return (isStatic ? member : null);
}
private boolean storeField(final FieldNode field, final PropertyExpression expressionToStoreOn, final ClassNode receiver, final ClassCodeVisitorSupport visitor, final String delegationData, final boolean lhsOfAssignment) {
@@ -3834,25 +3833,52 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return classNodes;
}
- protected void storeTargetMethod(final Expression call, final MethodNode directMethodCallCandidate) {
- call.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, directMethodCallCandidate);
+ protected void storeTargetMethod(final Expression call, final MethodNode target) {
+ if (target == null) {
+ call.removeNodeMetaData(DIRECT_METHOD_CALL_TARGET); return;
+ }
+ call.putNodeMetaData(DIRECT_METHOD_CALL_TARGET,target);
- checkOrMarkPrivateAccess(call, directMethodCallCandidate);
- checkSuperCallFromClosure(call, directMethodCallCandidate);
- extension.onMethodSelection(call, directMethodCallCandidate);
+ checkInterfaceStaticCall(call, target);
+ checkOrMarkPrivateAccess(call, target);
+ checkSuperCallFromClosure(call, target);
+ extension.onMethodSelection(call, target);
}
- private void checkSuperCallFromClosure(final Expression call, final MethodNode directCallTarget) {
- if (call instanceof MethodCallExpression && typeCheckingContext.getEnclosingClosure() != null) {
- Expression objectExpression = ((MethodCallExpression) call).getObjectExpression();
- if (isSuperExpression(objectExpression)) {
- ClassNode current = typeCheckingContext.getEnclosingClassNode();
- current.getNodeMetaData(SUPER_MOP_METHOD_REQUIRED, x -> new LinkedList<>()).add(directCallTarget);
- call.putNodeMetaData(SUPER_MOP_METHOD_REQUIRED, current);
+ private void checkInterfaceStaticCall(final Expression call, final MethodNode target) {
+ if (target instanceof ExtensionMethodNode) return;
+ ClassNode declaringClass = target.getDeclaringClass();
+ if (declaringClass.isInterface() && target.isStatic()) {
+ Expression objectExpression = getObjectExpression(call);
+ if (objectExpression != null) { ClassNode type = getType(objectExpression);
+ if (!isClassClassNodeWrappingConcreteType(type) || !type.getGenericsTypes()[0].getType().equals(declaringClass)) {
+ addStaticTypeError("static method of interface " + prettyPrintTypeName(declaringClass) + " can only be accessed with class qualifier", call);
+ }
}
}
}
+ private void checkSuperCallFromClosure(final Expression call, final MethodNode target) {
+ if (isSuperExpression(getObjectExpression(call)) && typeCheckingContext.getEnclosingClosure() != null) {
+ ClassNode current = typeCheckingContext.getEnclosingClassNode();
+ current.getNodeMetaData(SUPER_MOP_METHOD_REQUIRED, x -> new LinkedList<>()).add(target);
+ call.putNodeMetaData(SUPER_MOP_METHOD_REQUIRED, current);
+ }
+ }
+
+ private static Expression getObjectExpression(final Expression expression) {
+ if (expression instanceof MethodCallExpression) {
+ return ((MethodCallExpression) expression).getObjectExpression();
+ }
+ if (expression instanceof PropertyExpression) {
+ return ((PropertyExpression) expression).getObjectExpression();
+ }
+ /*if (expression instanceof MethodPointerExpression) {
+ return ((MethodPointerExpression) expression).getExpression();
+ }*/
+ return null;
+ }
+
protected void typeCheckClosureCall(final Expression arguments, final ClassNode[] argumentTypes, final Parameter[] parameters) {
if (allParametersAndArgumentsMatchWithDefaultParams(parameters, argumentTypes) < 0 && lastArgMatchesVarg(parameters, argumentTypes) < 0) {
addStaticTypeError("Cannot call closure that accepts " + formatArgumentList(extractTypesFromParameters(parameters)) + " with " + formatArgumentList(argumentTypes), arguments);
diff --git a/src/test/groovy/bugs/Groovy8579.groovy b/src/test/groovy/bugs/Groovy8579.groovy
index e4e9f1f565..845174a161 100644
--- a/src/test/groovy/bugs/Groovy8579.groovy
+++ b/src/test/groovy/bugs/Groovy8579.groovy
@@ -23,6 +23,7 @@ import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
import org.junit.Test
import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
final class Groovy8579 {
@@ -99,4 +100,43 @@ final class Groovy8579 {
}
}
}
+
+ @Test // GROOVY-10592
+ void testCallToStaticInterfaceMethod4() {
+ ['CompileStatic', 'TypeChecked'].each { mode ->
+ def sourceDir = File.createTempDir()
+ def config = new CompilerConfiguration(
+ targetDirectory: File.createTempDir(),
+ jointCompilationOptions: [memStub: true]
+ )
+ try {
+ def a = new File(sourceDir, 'Face.java')
+ a.write '''
+ interface Face {
+ static String getValue() {
+ return "value";
+ }
+ }
+ '''
+ def b = new File(sourceDir, 'Main.groovy')
+ b.write """
+ @groovy.transform.${mode}
+ void test(Face face) {
+ face.value
+ }
+ """
+
+ def loader = new GroovyClassLoader(this.class.classLoader)
+ def cu = new JavaAwareCompilationUnit(config, loader)
+ cu.addSources(a, b)
+ def err = shouldFail {
+ cu.compile()
+ }
+ assert err =~ /static method of interface Face can only be accessed /
+ } finally {
+ sourceDir.deleteDir()
+ config.targetDirectory.deleteDir()
+ }
+ }
+ }
}