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/08/26 23:58:00 UTC
[groovy] 03/03: GROOVY-9591: no error for dynamic variable in
closure in 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
commit 0273ab8e71138a5df9d064bbf8161e3a48fab28c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jun 13 14:17:41 2020 -0500
GROOVY-9591: no error for dynamic variable in closure in static context
(cherry picked from commit 836887ceec754027083339e56a8fedd3a12c7380)
Conflicts:
src/main/java/org/codehaus/groovy/control/StaticVerifier.java
src/test/groovy/bugs/Groovy8327Bug.groovy
---
.../codehaus/groovy/control/StaticVerifier.java | 94 ++++-----
src/test/groovy/bugs/Groovy8327.groovy | 221 +++++++++++++++++++++
2 files changed, 264 insertions(+), 51 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/StaticVerifier.java b/src/main/java/org/codehaus/groovy/control/StaticVerifier.java
index a0442a4..daeb2e5 100644
--- a/src/main/java/org/codehaus/groovy/control/StaticVerifier.java
+++ b/src/main/java/org/codehaus/groovy/control/StaticVerifier.java
@@ -42,26 +42,21 @@ import java.util.Set;
import static org.apache.groovy.ast.tools.ClassNodeUtils.isInnerClass;
/**
- * Verifier to check non-static access in static contexts
+ * Checks for dynamic variables in static contexts.
*/
public class StaticVerifier extends ClassCodeVisitorSupport {
- private boolean inSpecialConstructorCall;
- private boolean inPropertyExpression; // TODO use it or lose it
- private boolean inClosure;
- private MethodNode currentMethod;
- private SourceUnit source;
-
- public void visitClass(ClassNode node, SourceUnit source) {
- this.source = source;
- super.visitClass(node);
- }
+ private boolean inClosure, inSpecialConstructorCall;
+ private MethodNode methodNode;
+ private SourceUnit sourceUnit;
@Override
- public void visitVariableExpression(VariableExpression ve) {
- Variable v = ve.getAccessedVariable();
- if (v instanceof DynamicVariable) {
- if (!inPropertyExpression || inSpecialConstructorCall) addStaticVariableError(ve);
- }
+ protected SourceUnit getSourceUnit() {
+ return sourceUnit;
+ }
+
+ public void visitClass(ClassNode node, SourceUnit unit) {
+ sourceUnit = unit;
+ visitClass(node);
}
@Override
@@ -82,11 +77,11 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
@Override
public void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
- MethodNode oldCurrentMethod = currentMethod;
- currentMethod = node;
+ MethodNode oldMethodNode = methodNode;
+ methodNode = node;
super.visitConstructorOrMethod(node, isConstructor);
if (isConstructor) {
- final Set<String> exceptions = new HashSet<String>();
+ final Set<String> exceptions = new HashSet<>();
for (final Parameter param : node.getParameters()) {
exceptions.add(param.getName());
if (param.hasInitialExpression()) {
@@ -120,12 +115,12 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
}
}
}
- currentMethod = oldCurrentMethod;
+ methodNode = oldMethodNode;
}
@Override
public void visitMethodCallExpression(MethodCallExpression mce) {
- if (inSpecialConstructorCall && !isInnerClass(currentMethod.getDeclaringClass())) {
+ if (inSpecialConstructorCall && !isInnerClass(methodNode.getDeclaringClass())) {
Expression objectExpression = mce.getObjectExpression();
if (objectExpression instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) objectExpression;
@@ -138,40 +133,38 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
super.visitMethodCallExpression(mce);
}
- @Override
+ @Override // TODO: dead code?
public void visitPropertyExpression(PropertyExpression pe) {
- if (!inSpecialConstructorCall) checkStaticScope(pe);
- }
-
- @Override
- protected SourceUnit getSourceUnit() {
- return source;
- }
-
-
- private void checkStaticScope(PropertyExpression pe) {
- if (inClosure) return;
- for (Expression it = pe; it != null; it = ((PropertyExpression) it).getObjectExpression()) {
- if (it instanceof PropertyExpression) continue;
- if (it instanceof VariableExpression) {
- addStaticVariableError((VariableExpression) it);
+ if (!inClosure && !inSpecialConstructorCall) {
+ for (Expression it = pe; it != null; it = ((PropertyExpression) it).getObjectExpression()) {
+ if (it instanceof PropertyExpression) continue;
+ if (it instanceof VariableExpression) {
+ VariableExpression ve = (VariableExpression) it;
+ if (ve.isThisExpression() || ve.isSuperExpression()) return;
+ if (!inSpecialConstructorCall && (inClosure || !ve.isInStaticContext())) return;
+ if (methodNode != null && methodNode.isStatic()) {
+ FieldNode fieldNode = getDeclaredOrInheritedField(methodNode.getDeclaringClass(), ve.getName());
+ if (fieldNode != null && fieldNode.isStatic()) return;
+ }
+ Variable v = ve.getAccessedVariable();
+ if (v != null && !(v instanceof DynamicVariable) && v.isInStaticContext()) return;
+ addVariableError(ve);
+ }
+ return;
}
- return;
}
}
- private void addStaticVariableError(VariableExpression ve) {
- // closures are always dynamic
- // propertyExpressions will handle the error a bit differently
- if (!inSpecialConstructorCall && (inClosure || !ve.isInStaticContext())) return;
- if (ve.isThisExpression() || ve.isSuperExpression()) return;
- Variable v = ve.getAccessedVariable();
- if (currentMethod != null && currentMethod.isStatic()) {
- FieldNode fieldNode = getDeclaredOrInheritedField(currentMethod.getDeclaringClass(), ve.getName());
- if (fieldNode != null && fieldNode.isStatic()) return;
+ @Override
+ public void visitVariableExpression(VariableExpression ve) {
+ if (ve.getAccessedVariable() instanceof DynamicVariable && (ve.isInStaticContext() || inSpecialConstructorCall) && !inClosure) {
+ // GROOVY-5687: interface constants not visible to implementing sub-class in static context
+ if (methodNode != null && methodNode.isStatic()) {
+ FieldNode fieldNode = getDeclaredOrInheritedField(methodNode.getDeclaringClass(), ve.getName());
+ if (fieldNode != null && fieldNode.isStatic()) return;
+ }
+ addVariableError(ve);
}
- if (v != null && !(v instanceof DynamicVariable) && v.isInStaticContext()) return;
- addVariableError(ve);
}
private void addVariableError(VariableExpression ve) {
@@ -188,7 +181,7 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
while (node != null) {
FieldNode fn = node.getDeclaredField(fieldName);
if (fn != null) return fn;
- List<ClassNode> interfacesToCheck = new ArrayList<ClassNode>(Arrays.asList(node.getInterfaces()));
+ List<ClassNode> interfacesToCheck = new ArrayList<>(Arrays.asList(node.getInterfaces()));
while (!interfacesToCheck.isEmpty()) {
ClassNode nextInterface = interfacesToCheck.remove(0);
fn = nextInterface.getDeclaredField(fieldName);
@@ -199,5 +192,4 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
}
return null;
}
-
}
diff --git a/src/test/groovy/bugs/Groovy8327.groovy b/src/test/groovy/bugs/Groovy8327.groovy
new file mode 100644
index 0000000..986e6f7
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8327.groovy
@@ -0,0 +1,221 @@
+/*
+ * 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
+
+import groovy.transform.NotYetImplemented
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy8327 {
+
+ @Test
+ void testCallStaticMethodInThisConstructor() {
+ assertScript '''
+ class A {
+ static String f = '123'
+ static String g() { 'abc' }
+ A() {
+ this(g() + getF())
+ }
+ A(a) { assert a == 'abc123' }
+ }
+
+ new A()
+ '''
+ }
+
+ @Test
+ void testCallStaticMethodInSuperConstructor() {
+ assertScript '''
+ class A {
+ A(a) { assert a == 'abc123' }
+ }
+
+ class B extends A {
+ static String f = '123'
+ static String g() { 'abc' }
+ B() {
+ super(g() + getF())
+ }
+ }
+
+ new B()
+ '''
+ }
+
+ @Test
+ void testCallSuperStaticMethodInSuperConstructor() {
+ assertScript '''
+ class A {
+ static String f = '123'
+ static String g() { 'abc' }
+ A(a) { assert a == 'abc123' }
+ }
+
+ class B extends A {
+ B() {
+ super(g() + getF())
+ }
+ }
+
+ new B()
+ '''
+ }
+
+ // GROOVY-8327:
+
+ @Test
+ void testCallStaticMethodInClosureParamOfThisConstructor() {
+ assertScript '''
+ class A {
+ static String f = '123'
+ static String g() { 'abc' }
+ A() {
+ this({ -> g() + getF()})
+ }
+ A(a) { assert a() == 'abc123' }
+ }
+
+ new A()
+ '''
+ }
+
+ @Test
+ void testCallStaticMethodInClosureParamOfSuperConstructor() {
+ assertScript '''
+ class A {
+ A(a) { assert a() == 'abc123' }
+ }
+
+ class B extends A {
+ static String f = '123'
+ static String g() { 'abc' }
+ B() {
+ super({ -> g() + getF() })
+ }
+ }
+
+ new B()
+ '''
+ }
+
+ @Test
+ void testCallSuperStaticMethodInClosureParamOfSuperConstructor() {
+ assertScript '''
+ class A {
+ static String f = '123'
+ static String g() { 'abc' }
+ A(a) { assert a() == 'abc123' }
+ }
+
+ class B extends A {
+ B() {
+ super({ -> g() + getF() })
+ }
+ }
+
+ new B()
+ '''
+ }
+
+ // GROOVY-9591:
+
+ @Test
+ void testTapExpressionAsSpecialConstructorArgument1() {
+ assertScript '''
+ @groovy.transform.ToString
+ class A {
+ A(x) {}
+ def b
+ }
+ class C {
+ C() {
+ this(new A(null).tap { b = 42 })
+ }
+ C(x) {
+ assert x.toString() == 'A(42)'
+ }
+ }
+ new C()
+ '''
+ }
+
+ @NotYetImplemented @Test
+ void testTapExpressionAsSpecialConstructorArgument2() {
+ assertScript '''
+ @groovy.transform.ToString
+ class A {
+ A(x) {}
+ def b
+ void init() { b = 42 }
+ }
+ class C {
+ C() {
+ this(new A(null).tap { init() })
+ }
+ C(x) {
+ assert x.toString() == 'A(42)'
+ }
+ }
+ new C()
+ '''
+ }
+
+ @Test
+ void testWithExpressionAsSpecialConstructorArgument1() {
+ assertScript '''
+ @groovy.transform.ToString
+ class A {
+ A(x) {}
+ def b
+ }
+ class C {
+ C() {
+ this(new A(null).with { b = 42; return it })
+ }
+ C(x) {
+ assert x.toString() == 'A(42)'
+ }
+ }
+ new C()
+ '''
+ }
+
+ @NotYetImplemented @Test
+ void testWithExpressionAsSpecialConstructorArgument2() {
+ assertScript '''
+ @groovy.transform.ToString
+ class A {
+ A(x) {}
+ def b
+ void init() { b = 42 }
+ }
+ class C {
+ C() {
+ this(new A(null).with { init(); return it })
+ }
+ C(x) {
+ assert x.toString() == 'A(42)'
+ }
+ }
+ new C()
+ '''
+ }
+}