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 2021/12/22 15:20:33 UTC
[groovy] branch master updated: GROOVY-7890: STC: closure within static context
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 c2a6c4b GROOVY-7890: STC: closure within static context
c2a6c4b is described below
commit c2a6c4b2fc0ff2c43c61425900725c78165d2672
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Dec 22 09:19:17 2021 -0600
GROOVY-7890: STC: closure within static context
---
.../transform/stc/StaticTypeCheckingVisitor.java | 65 ++++++++++------------
.../stc/FieldsAndPropertiesSTCTest.groovy | 28 ++++++++++
2 files changed, 57 insertions(+), 36 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 c0e5ea5..0d7fcf8 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1540,15 +1540,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
Collections.addAll(queue, current.getInterfaces());
}
- // in case of a lookup on Class we look for instance methods on Class
- // as well, since in case of a static property access we have the class
- // itself in the list of receivers already;
- boolean staticOnly;
- if (isClassClassNodeWrappingConcreteType(current)) {
- staticOnly = false;
- } else {
- staticOnly = staticOnlyAccess;
- }
+ boolean staticOnly = (receiver.getData() == null ? staticOnlyAccess : false);
+ // in case of a lookup on java.lang.Class, look for instance methods on Class
+ // as well; in case of static property access Class<Type> and Type are listed
+ if (isClassClassNodeWrappingConcreteType(current)) staticOnly = false;
field = allowStaticAccessToMember(field, staticOnly);
@@ -2375,9 +2370,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
@Override
public void visitClosureExpression(final ClosureExpression expression) {
- boolean oldStaticContext = typeCheckingContext.isInStaticContext;
- typeCheckingContext.isInStaticContext = false;
-
// collect every variable expression used in the closure body
Map<VariableExpression, ClassNode> varTypes = new HashMap<>();
expression.getCode().visit(new VariableExpressionTypeMemoizer(varTypes, true));
@@ -2425,7 +2417,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
// restore original metadata
restoreVariableExpressionMetadata(variableMetadata);
- typeCheckingContext.isInStaticContext = oldStaticContext;
for (Parameter parameter : getParametersSafe(expression)) {
typeCheckingContext.controlStructureVariables.remove(parameter);
// GROOVY-10072: visit param default argument expression if present
@@ -2571,25 +2562,21 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
// we must not visit a method which used dynamic dispatch.
// We do not check for an annotation because some other AST transformations
// may use this visitor without the annotation being explicitly set
- if (!typeCheckingContext.methodsToBeVisited.isEmpty() && !typeCheckingContext.methodsToBeVisited.contains(node))
- return;
-
- // alreadyVisitedMethods prevents from visiting the same method multiple times
- // and prevents from infinite loops
- if (typeCheckingContext.alreadyVisitedMethods.contains(node)) return;
- typeCheckingContext.alreadyVisitedMethods.add(node);
-
- typeCheckingContext.pushErrorCollector(collector);
- boolean osc = typeCheckingContext.isInStaticContext;
- try {
- typeCheckingContext.isInStaticContext = node.isStatic();
+ if ((typeCheckingContext.methodsToBeVisited.isEmpty() || typeCheckingContext.methodsToBeVisited.contains(node))
+ && typeCheckingContext.alreadyVisitedMethods.add(node)) { // prevent re-visiting method (infinite loop)
+ typeCheckingContext.pushErrorCollector(collector);
+ boolean osc = typeCheckingContext.isInStaticContext;
+ try {
+ // GROOVY-7890: non-static trait method is static in helper type
+ typeCheckingContext.isInStaticContext = isNonStaticHelperMethod(node) ? false : node.isStatic();
- super.visitMethod(node);
- } finally {
- typeCheckingContext.isInStaticContext = osc;
+ super.visitMethod(node);
+ } finally {
+ typeCheckingContext.isInStaticContext = osc;
+ }
+ typeCheckingContext.popErrorCollector();
+ node.putNodeMetaData(ERROR_COLLECTOR, collector);
}
- typeCheckingContext.popErrorCollector();
- node.putNodeMetaData(ERROR_COLLECTOR, collector);
}
@Override
@@ -3415,12 +3402,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode receiverType = currentReceiver.getType();
mn = findMethod(receiverType, name, args);
- // if the receiver is "this" or "implicit this", then we must make sure that the compatible
- // methods are only static if we are in a static context
- // if we are not in a static context but the current receiver is a static class, we must
- // ensure that all methods are either static or declared by the current receiver or a superclass
- if (!mn.isEmpty()
- && (isThisObjectExpression || call.isImplicitThis())
+ // if receiver is "this" in a static context then only static methods are compatible
+ // if not in a static context but the current receiver is a static class ensure that
+ // all methods are either static or declared by the current receiver or a superclass
+ if (!mn.isEmpty() && currentReceiver.getData() == null && (isThisObjectExpression || call.isImplicitThis())
&& (typeCheckingContext.isInStaticContext || (receiverType.getModifiers() & Opcodes.ACC_STATIC) != 0)) {
// we create separate method lists just to be able to print out
// a nice error message to the user
@@ -5824,6 +5809,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return false;
}
+ private static boolean isNonStaticHelperMethod(final MethodNode method) {
+ Parameter[] parameters = method.getParameters(); // check first param is "$self"
+ if (parameters.length > 0 && parameters[0].getName().equals(Traits.THIS_OBJECT)) {
+ return !method.getName().contains("$init$") && Traits.isTrait(method.getDeclaringClass().getOuterClass());
+ }
+ return false;
+ }
+
private static BinaryExpression assignX(final Expression lhs, final Expression rhs, final ASTNode pos) {
BinaryExpression exp = (BinaryExpression) GeneralUtils.assignX(lhs, rhs);
exp.setSourcePosition(pos);
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 3a8b66b..c9d56cf 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -875,6 +875,34 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-7890
+ void testNonStaticPropertyAndStaticMethodClosure() {
+ shouldFailWithMessages '''
+ class C {
+ List<String> replace
+ static String m(String s) {
+ s.collectReplacements {
+ (it in replace) ? 'o' : null
+ }
+ }
+ }
+ ''',
+ 'The variable [replace] is undeclared'
+
+ assertScript '''
+ class C {
+ List<String> replace
+ String m(String s) {
+ s.collectReplacements {
+ (it in replace) ? 'o' : null
+ }
+ }
+ }
+ String result = new C(replace:['a','b','c']).m('foobar')
+ assert result == 'foooor'
+ '''
+ }
+
// GROOVY-5872
void testAssignNullToFieldWithGenericsShouldNotThrowError() {
assertScript '''