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/08/29 21:04:59 UTC
[groovy] branch GROOVY_2_5_X 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 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 569e759111 GROOVY-7890: STC: closure within static context
569e759111 is described below
commit 569e75911161113afb1c091a7681bf1ee1081fad
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 | 88 +++++++++-------------
.../stc/FieldsAndPropertiesSTCTest.groovy | 28 +++++++
2 files changed, 65 insertions(+), 51 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 81572b6965..91149d293c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1528,15 +1528,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
- // 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);
// skip property/accessor checks for "x.@field"
@@ -2450,9 +2446,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));
@@ -2502,7 +2495,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
// restore original metadata
restoreVariableExpressionMetadata(typesBeforeVisit);
- typeCheckingContext.isInStaticContext = oldStaticContext;
for (Parameter parameter : getParametersSafe(expression)) {
typeCheckingContext.controlStructureVariables.remove(parameter);
// GROOVY-10072: visit param default argument expression if present
@@ -2661,26 +2653,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);
-
- final 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);
}
protected void addTypeCheckingInfoAnnotation(final MethodNode node) {
@@ -3430,10 +3417,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
checkForbiddenSpreadArgument(argumentList);
- // for arguments, we need to visit closures *after* the method has been chosen
-
-
ClassNode receiver = getType(objectExpression);
+ // visit closures *after* the method has been chosen
visitMethodCallArguments(receiver, argumentList, false, null);
ClassNode[] args = getArgumentTypes(argumentList);
@@ -3488,18 +3473,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
storeType(call, type);
}
}
- int nbOfArgs;
+
+ int nArgs = 0;
if (callArguments instanceof ArgumentListExpression) {
- ArgumentListExpression list = (ArgumentListExpression) callArguments;
- nbOfArgs = list.getExpressions().size();
- } else {
- // todo : other cases
- nbOfArgs = 0;
+ nArgs = ((ArgumentListExpression) callArguments).getExpressions().size();
}
- storeTargetMethod(call,
- nbOfArgs == 0 ? CLOSURE_CALL_NO_ARG :
- nbOfArgs == 1 ? CLOSURE_CALL_ONE_ARG :
- CLOSURE_CALL_VARGS);
+ storeTargetMethod(call, nArgs == 0 ? CLOSURE_CALL_NO_ARG : nArgs == 1 ? CLOSURE_CALL_ONE_ARG : CLOSURE_CALL_VARGS);
} else {
// method call receivers are :
// - possible "with" receivers
@@ -3515,13 +3494,12 @@ 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()
- && (typeCheckingContext.isInStaticContext || Modifier.isStatic(receiverType.getModifiers()))
- && (call.isImplicitThis() || isThisExpression(objectExpression))) {
+ // 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
+ && (isThisExpression(objectExpression) || call.isImplicitThis())
+ && (typeCheckingContext.isInStaticContext || Modifier.isStatic(receiverType.getModifiers()))) {
// we create separate method lists just to be able to print out
// a nice error message to the user
// a method is accessible if it is static, or if we are not in a static context and it is
@@ -5717,6 +5695,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;
+ }
+
//--------------------------------------------------------------------------
protected class VariableExpressionTypeMemoizer extends ClassCodeVisitorSupport {
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index fa6a866e5e..8fca6e79fb 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -837,6 +837,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 '''