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 2020/05/23 03:16:56 UTC
[groovy] 01/01: GROOVY-9569: check outers and uppers for static
fields
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9569
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 0784d393cfffdb617372c68ca2c08198d269a136
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri May 22 19:32:39 2020 -0500
GROOVY-9569: check outers and uppers for static fields
- use ArrtibuteExpression for outer field
- use PropertyExpression for upper field
---
.../groovy/classgen/AsmClassGenerator.java | 68 ++++++---
src/test/gls/innerClass/InnerClassTest.groovy | 155 ++++++++++++++++++++-
src/test/groovy/bugs/Groovy5259.groovy | 131 -----------------
3 files changed, 203 insertions(+), 151 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index d62c732..6e0aa6e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -124,7 +124,13 @@ import java.util.Objects;
import java.util.Optional;
import static org.apache.groovy.util.BeanUtils.capitalize;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
+import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PROPERTY_OWNER;
/**
* Generates Java class versions of Groovy classes using ASM.
@@ -1006,6 +1012,40 @@ public class AsmClassGenerator extends ClassGenerator {
return classNode.getSuperClass().getGetterMethod(getterMethodName);
}
+ private boolean checkStaticOuterField(final PropertyExpression expression, final String name) {
+ for (final ClassNode outer : controller.getClassNode().getOuterClasses()) {
+ FieldNode field = outer.getDeclaredField(name);
+ if (field != null) {
+ if (!field.isStatic()) break;
+
+ Expression outerClass = classX(outer);
+ outerClass.setNodeMetaData(PROPERTY_OWNER, outer);
+ outerClass.setSourcePosition(expression.getObjectExpression());
+
+ Expression outerField = attrX(outerClass, expression.getProperty());
+ outerField.setSourcePosition(expression);
+ outerField.visit(this);
+ return true;
+ } else {
+ field = outer.getField(name); // checks supers
+ if (field != null && !field.isPrivate() && (field.isPublic() || field.isProtected()
+ || Objects.equals(field.getDeclaringClass().getPackageName(), outer.getPackageName()))) {
+ if (!field.isStatic()) break;
+
+ Expression upperClass = classX(field.getDeclaringClass());
+ upperClass.setNodeMetaData(PROPERTY_OWNER, field.getDeclaringClass());
+ upperClass.setSourcePosition(expression.getObjectExpression());
+
+ Expression upperField = propX(upperClass, expression.getProperty());
+ upperField.setSourcePosition(expression);
+ upperField.visit(this);
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private boolean isGroovyObject(final Expression objectExpression) {
if (isThisExpression(objectExpression)) return true;
if (objectExpression instanceof ClassExpression) return false;
@@ -1032,35 +1072,25 @@ public class AsmClassGenerator extends ClassGenerator {
if (isSuperExpression(objectExpression)) {
field = classNode.getSuperClass().getDeclaredField(name);
privateSuperField = (field != null && field.isPrivate());
- } else if (expression.isImplicitThis() || !controller.isInGeneratedFunction()) {
+ } else if (controller.isInGeneratedFunction()) {
+ if (expression.isImplicitThis())
+ field = classNode.getDeclaredField(name); // closure params are stored as fields
+ } else {
field = classNode.getDeclaredField(name);
-
- ClassNode outer = classNode.getOuterClass();
- if (field == null && outer != null) {
- do {
- FieldNode outerClassField = outer.getDeclaredField(name);
- if (outerClassField != null && outerClassField.isStatic()) {
- if (outerClassField.isPrivate() && classNode.getOuterClass() != outer) {
- throw new GroovyBugError("Trying to access private field [" + outerClassField.getDeclaringClass() + "#" + outerClassField.getName() + "] from inner class");
- }
- PropertyExpression staticOuterField = new PropertyExpression(new ClassExpression(outer), expression.getProperty());
- staticOuterField.getObjectExpression().setSourcePosition(objectExpression);
- staticOuterField.visit(this);
- return;
- }
- outer = outer.getSuperClass();
- } while (outer != null);
+ if (field == null && classNode.getField(name) == null) {
+ // GROOVY-5259, GROOVY-9501, GROOVY-9569
+ if (checkStaticOuterField(expression, name)) return;
}
}
if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
- visitFieldExpression(new FieldExpression(field));
+ fieldX(field).visit(this);
visited = true;
} else if (isSuperExpression(objectExpression)) {
if (controller.getCompileStack().isLHS()) {
setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
} else {
- visitMethodCallExpression(new MethodCallExpression(objectExpression, "get" + capitalize(name), MethodCallExpression.NO_ARGUMENTS));
+ callX(objectExpression, "get" + capitalize(name)).visit(this); // TODO: "is"
}
visited = true;
}
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index 0044a40..9b5fa33 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -431,7 +431,7 @@ final class InnerClassTest {
'''
}
- @Test // inner class is static instead of final
+ @Test // inner class is static instead of final
void testUsageOfOuterField8() {
assertScript '''
class Main extends Outer {
@@ -467,6 +467,159 @@ final class InnerClassTest {
'''
}
+ @Test // GROOVY-9569
+ void testUsageOfOuterField9() {
+ assertScript '''
+ class Main extends Outer {
+ static main(args) {
+ newInstance().newThread()
+ assert Outer.Inner.error == null
+ }
+ }
+
+ @groovy.transform.CompileStatic
+ abstract class Outer {
+ private static volatile boolean flag
+
+ void newThread() {
+ Thread thread = new Inner()
+ thread.start()
+ thread.join()
+ }
+
+ private static class Inner extends Thread {
+ @Override
+ void run() {
+ try {
+ if (!flag) {
+ // do work
+ }
+ } catch (e) {
+ error = e
+ }
+ }
+ public static error
+ }
+ }
+ '''
+ }
+
+ @Test
+ void testUsageOfOuterField10() {
+ assertScript '''
+ class Outer {
+ static final String OUTER_CONSTANT = 'Constant Value'
+
+ class Inner {
+ String access() {
+ return OUTER_CONSTANT
+ }
+ }
+
+ void testInnerClassAccessOuterConst() {
+ def inner = new Inner()
+ assert inner.access() == OUTER_CONSTANT
+ }
+ }
+
+ def outer = new Outer()
+ outer.testInnerClassAccessOuterConst()
+ '''
+ }
+
+ @Test // GROOVY-5259
+ void testUsageOfOuterField11() {
+ assertScript '''
+ class Base {
+ Base(String string) {
+ }
+ }
+
+ class Outer {
+ static final String OUTER_CONSTANT = 'Constant Value'
+
+ class Inner extends Base {
+ Inner() {
+ super(OUTER_CONSTANT) // "this" is not initialized yet
+ }
+
+ String access() {
+ return OUTER_CONSTANT
+ }
+ }
+
+ void testInnerClassAccessOuterConst() {
+ def inner = new Inner()
+ assert inner.access() == OUTER_CONSTANT
+ }
+ }
+
+ def outer = new Outer()
+ outer.testInnerClassAccessOuterConst()
+ '''
+ }
+
+ @Test
+ void testUsageOfOuterSuperField() {
+ assertScript '''
+ class InnerBase {
+ InnerBase(String string) {
+ }
+ }
+
+ class OuterBase {
+ protected static final String OUTER_CONSTANT = 'Constant Value'
+ }
+
+ class Outer extends OuterBase {
+
+ class Inner extends InnerBase {
+ Inner() {
+ super(OUTER_CONSTANT)
+ }
+
+ String access() {
+ return OUTER_CONSTANT
+ }
+ }
+
+ void testInnerClassAccessOuterConst() {
+ def inner = new Inner()
+ assert inner.access() == OUTER_CONSTANT
+ }
+ }
+
+ def outer = new Outer()
+ outer.testInnerClassAccessOuterConst()
+ '''
+ }
+
+ /*@Test
+ void testUsageOfOuterField_WrongCallToSuper() {
+ shouldFail '''
+ class InnerAccessOuter {
+ protected static final String OUTER_CONSTANT = 'Constant Value'
+
+ class InnerClass {
+ InnerClass() {
+ // there's no Object#<init>(String) method, but it throws a VerifyError when a new instance
+ // is created, meaning a wrong super call is generated
+ super(OUTER_CONSTANT)
+ }
+ String m() {
+ OUTER_CONSTANT
+ }
+ }
+
+ void testInnerClassAccessOuter() {
+ def inner = new InnerClass()
+ inner.m()
+ }
+ }
+ new InnerAccessOuter().testInnerClassAccessOuter()
+ '''
+ }*/
+
@Test
void testUsageOfOuterFieldOverridden() {
assertScript '''
diff --git a/src/test/groovy/bugs/Groovy5259.groovy b/src/test/groovy/bugs/Groovy5259.groovy
deleted file mode 100644
index 1b4bc28..0000000
--- a/src/test/groovy/bugs/Groovy5259.groovy
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * 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 org.junit.Test
-
-import static groovy.test.GroovyAssert.assertScript
-import static groovy.test.GroovyAssert.shouldFail
-
-final class Groovy5259 {
-
- @Test
- void testInnerClassAccessingOuterClassConstant() {
- assertScript '''
- class InnerAccessOuter {
- static final String OUTER_CONSTANT = 'Constant Value'
-
- class InnerClass {
- InnerClass() {
- }
-
- String innerCompiled() {
- OUTER_CONSTANT
- }
- }
-
- void testInnerClassAccessOuter() {
- def inner = new InnerClass()
- assert OUTER_CONSTANT == inner.innerCompiled()
- }
- }
- new InnerAccessOuter().testInnerClassAccessOuter()
- '''
- }
-
- @Test
- void testInnerClassWithWrongCallToSuperAccessingOuterClassConstant() {
- shouldFail '''
- class InnerAccessOuter {
- protected static final String OUTER_CONSTANT = 'Constant Value'
-
- class InnerClass {
- InnerClass() {
- // there's no Object#<init>(String) method, but it throws a VerifyError when a new instance
- // is created, meaning a wrong super call is generated
- super(OUTER_CONSTANT)
- }
- String m() {
- OUTER_CONSTANT
- }
- }
-
- void testInnerClassAccessOuter() {
- def inner = new InnerClass()
- inner.m()
- }
- }
- new InnerAccessOuter().testInnerClassAccessOuter()
- '''
- }
-
- @Test
- void testInnerClassWithSuperClassAccessingOuterClassConstant() {
- assertScript '''
- class Base {
- Base(String str) {}
- }
- class InnerAccessOuter {
- static final String OUTER_CONSTANT = 'Constant Value'
-
- class InnerClass extends Base {
- InnerClass() {
- super(OUTER_CONSTANT)
- }
-
- String innerCompiled() { OUTER_CONSTANT }
- }
-
- void testInnerClassAccessOuter() {
- def inner = new InnerClass() // throws a VerifyError
- assert OUTER_CONSTANT == inner.innerCompiled()
- }
- }
- new InnerAccessOuter().testInnerClassAccessOuter()
- '''
- }
-
- @Test
- void testInnerClassWithSuperClassAccessingSuperOuterClassConstant() {
- assertScript '''
- class Base {
- Base(String str) {}
- }
- class OuterBase {
- protected static final String OUTER_CONSTANT = 'Constant Value'
- }
- class InnerAccessOuter extends OuterBase {
-
- class InnerClass extends Base {
- InnerClass() {
- super(OUTER_CONSTANT)
- }
-
- String innerCompiled() { OUTER_CONSTANT }
- }
-
- void testInnerClassAccessOuter() {
- def inner = new InnerClass() // throws a VerifyError
- assert OUTER_CONSTANT == inner.innerCompiled()
- }
- }
- new InnerAccessOuter().testInnerClassAccessOuter()
- '''
- }
-}