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/07/26 17:34:43 UTC
[groovy] 01/01: GROOVY-10695: `Type.name` within `Type` gets explicit-`this` treatment
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-10695
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c790e29736fd72b076ddbc9e186e25adae99b2d8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 26 12:33:37 2022 -0500
GROOVY-10695: `Type.name` within `Type` gets explicit-`this` treatment
---
.../groovy/ast/MixinASTTransformation.java | 48 +++++++----------
.../groovy/classgen/AsmClassGenerator.java | 63 +++++++++++-----------
.../stc/FieldsAndPropertiesSTCTest.groovy | 18 +++++++
3 files changed, 68 insertions(+), 61 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/MixinASTTransformation.java b/src/main/java/org/codehaus/groovy/ast/MixinASTTransformation.java
index 61009b8950..db86fe1809 100644
--- a/src/main/java/org/codehaus/groovy/ast/MixinASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/ast/MixinASTTransformation.java
@@ -22,70 +22,60 @@ import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.ast.stmt.BlockStatement;
-import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.transform.AbstractASTTransformation;
import org.codehaus.groovy.transform.GroovyASTTransformation;
-import static org.codehaus.groovy.ast.ClassHelper.make;
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.propX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
-import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
/**
* @deprecated static mixins have been deprecated in favour of traits (trait keyword).
*/
@Deprecated
-@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+@GroovyASTTransformation
public class MixinASTTransformation extends AbstractASTTransformation {
- @SuppressWarnings("deprecation")
- private static final ClassNode MY_TYPE = make(groovy.lang.Mixin.class);
@Override
- public void visit(ASTNode[] nodes, SourceUnit source) {
+ public void visit(final ASTNode[] nodes, final SourceUnit source) {
init(nodes, source);
AnnotationNode node = (AnnotationNode) nodes[0];
- AnnotatedNode parent = (AnnotatedNode) nodes[1];
- if (!MY_TYPE.equals(node.getClassNode()))
+ AnnotatedNode target = (AnnotatedNode) nodes[1];
+ if (!node.getClassNode().getName().equals("groovy.lang.Mixin"))
return;
- final Expression expr = node.getMember("value");
- if (expr == null) {
+ Expression value = node.getMember("value");
+ if (value == null) {
return;
}
Expression useClasses = null;
- if (expr instanceof ClassExpression) {
- useClasses = expr;
- } else if (expr instanceof ListExpression) {
- ListExpression listExpression = (ListExpression) expr;
+ if (value instanceof ClassExpression) {
+ useClasses = value;
+ } else if (value instanceof ListExpression) {
+ ListExpression listExpression = (ListExpression) value;
for (Expression ex : listExpression.getExpressions()) {
if (!(ex instanceof ClassExpression))
return;
}
- useClasses = expr;
+ useClasses = value;
}
-
- if (useClasses == null)
+ if (useClasses == null) {
return;
+ }
- if (parent instanceof ClassNode) {
- ClassNode annotatedClass = (ClassNode) parent;
+ if (target instanceof ClassNode) {
+ ClassNode targetClass = (ClassNode) target;
- final Parameter[] noparams = Parameter.EMPTY_ARRAY;
- MethodNode clinit = annotatedClass.getDeclaredMethod("<clinit>", noparams);
+ MethodNode clinit = targetClass.getDeclaredMethod("<clinit>", Parameter.EMPTY_ARRAY);
if (clinit == null) {
- clinit = annotatedClass.addMethod("<clinit>", ACC_PUBLIC | ACC_STATIC | ACC_SYNTHETIC, ClassHelper.VOID_TYPE, noparams, null, new BlockStatement());
- clinit.setSynthetic(true);
+ clinit = targetClass.addSyntheticMethod("<clinit>", ACC_PUBLIC | ACC_STATIC, ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, null, new BlockStatement());
}
- final BlockStatement code = (BlockStatement) clinit.getCode();
- code.addStatement(
- stmt(callX(propX(classX(annotatedClass), "metaClass"), "mixin", useClasses))
+ ((BlockStatement) clinit.getCode()).addStatement(
+ stmt(callX(callX(targetClass, "getMetaClass"), "mixin", useClasses))
);
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 55ba929b22..c6daf59c9b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -20,7 +20,6 @@ package org.codehaus.groovy.classgen;
import groovy.lang.GroovyRuntimeException;
import groovy.transform.Sealed;
-import org.apache.groovy.ast.tools.ExpressionUtils;
import org.apache.groovy.io.StringBuilderWriter;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ASTNode;
@@ -134,7 +133,8 @@ import java.util.Optional;
import java.util.function.Consumer;
import static org.apache.groovy.ast.tools.ClassNodeUtils.getField;
-import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
import static org.codehaus.groovy.ast.ClassHelper.isClassType;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
@@ -943,7 +943,7 @@ public class AsmClassGenerator extends ClassGenerator {
if (castExpression.isCoerce()) {
operandStack.doAsType(type);
} else {
- if (ExpressionUtils.isNullConstant(subExpression) && !isPrimitiveType(type)) {
+ if (isNullConstant(subExpression) && !isPrimitiveType(type)) {
operandStack.replace(type);
} else {
ClassNode subExprType = controller.getTypeChooser().resolveType(subExpression, controller.getClassNode());
@@ -1159,16 +1159,26 @@ public class AsmClassGenerator extends ClassGenerator {
}
private boolean isGroovyObject(final Expression objectExpression) {
- if (isThisOrSuper(objectExpression)) return true; //GROOVY-8693
if (objectExpression instanceof ClassExpression) return false;
+ if (isThisOrSuper(objectExpression)) return true;//GROOVY-8693
ClassNode objectExpressionType = controller.getTypeChooser().resolveType(objectExpression, controller.getClassNode());
if (isObjectType(objectExpressionType)) objectExpressionType = objectExpression.getType();
- return implementsGroovyObject(objectExpressionType);
+ return objectExpressionType.isDerivedFromGroovyObject();
}
- private static boolean implementsGroovyObject(final ClassNode cn) {
- return cn.isDerivedFromGroovyObject(); // GROOVY-10540: added before classgen
+ private boolean isThisExpression(final Expression expression) {
+ return org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression(expression)
+ // GROOVY-10695: "Type.name" within body of Type should get explicit-this treatment
+ || (expression instanceof ClassExpression && expression.getType().equals(controller.getClassNode()));
+ }
+
+ private boolean isThisOrSuper(final Expression expression) {
+ return isThisExpression(expression) || isSuperExpression(expression);
+ }
+
+ private boolean isStatic(final Expression expression) { // Type, this or super
+ return expression instanceof ClassExpression || controller.isStaticContext();
}
@Override
@@ -1184,7 +1194,7 @@ public class AsmClassGenerator extends ClassGenerator {
FieldNode fieldNode = null;
ClassNode classNode = controller.getClassNode();
- if (ExpressionUtils.isThisExpression(objectExpression)) {
+ if (isThisExpression(objectExpression)) {
if (controller.isInGeneratedFunction()) { // params are stored as fields
if (expression.isImplicitThis()) fieldNode = classNode.getDeclaredField(name);
} else {
@@ -1195,6 +1205,10 @@ public class AsmClassGenerator extends ClassGenerator {
&& fieldNode.getType().equals(ClassHelper.REFERENCE_TYPE)) {
fieldNode = null;
}
+ // GROOVY-10695: "this.name" or "Type.name" where "name" is non-static
+ if (fieldNode != null && !fieldNode.isStatic() && isStatic(objectExpression)) {
+ fieldNode = null;
+ }
// GROOVY-9501, GROOVY-9569, GROOVY-9650, GROOVY-9655, GROOVY-9665, GROOVY-9683, GROOVY-9695
if (fieldNode == null && !isFieldDirectlyAccessible(getField(classNode, name), classNode)) {
if (checkStaticOuterField(expression, name)) return;
@@ -1219,9 +1233,9 @@ public class AsmClassGenerator extends ClassGenerator {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
- adapter = ExpressionUtils.isSuperExpression(objectExpression) ? setPropertyOnSuper : useMetaObjectProtocol ? setGroovyObjectProperty : setProperty;
+ adapter = isSuperExpression(objectExpression) ? setPropertyOnSuper : useMetaObjectProtocol ? setGroovyObjectProperty : setProperty;
} else {
- adapter = ExpressionUtils.isSuperExpression(objectExpression) ? getPropertyOnSuper : useMetaObjectProtocol ? getGroovyObjectProperty : getProperty;
+ adapter = isSuperExpression(objectExpression) ? getPropertyOnSuper : useMetaObjectProtocol ? getGroovyObjectProperty : getProperty;
}
visitAttributeOrProperty(expression, adapter);
}
@@ -1244,8 +1258,8 @@ public class AsmClassGenerator extends ClassGenerator {
String name = expression.getPropertyAsString();
if (name != null) {
ClassNode classNode = controller.getClassNode();
- FieldNode fieldNode = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, ExpressionUtils.isSuperExpression(objectExpression));
- if (fieldNode != null) {
+ FieldNode fieldNode = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
+ if (fieldNode != null && (fieldNode.isStatic() || !isStatic(objectExpression))) {
fieldX(fieldNode).visit(this);
visited = true;
}
@@ -1255,9 +1269,9 @@ public class AsmClassGenerator extends ClassGenerator {
if (!visited) {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
- adapter = ExpressionUtils.isSuperExpression(objectExpression) ? setFieldOnSuper : isGroovyObject(objectExpression) ? setGroovyObjectField : setField;
+ adapter = isSuperExpression(objectExpression) ? setFieldOnSuper : isGroovyObject(objectExpression) ? setGroovyObjectField : setField;
} else {
- adapter = ExpressionUtils.isSuperExpression(objectExpression) ? getFieldOnSuper : isGroovyObject(objectExpression) ? getGroovyObjectField : getField;
+ adapter = isSuperExpression(objectExpression) ? getFieldOnSuper : isGroovyObject(objectExpression) ? getGroovyObjectField : getField;
}
visitAttributeOrProperty(expression, adapter);
}
@@ -2319,21 +2333,6 @@ public class AsmClassGenerator extends ClassGenerator {
if (callback != null) callback.accept(controller);
}
- @Deprecated
- public static boolean isThisExpression(final Expression expression) {
- return ExpressionUtils.isThisExpression(expression);
- }
-
- @Deprecated
- public static boolean isSuperExpression(final Expression expression) {
- return ExpressionUtils.isSuperExpression(expression);
- }
-
- @Deprecated
- public static boolean isNullConstant(final Expression expression) {
- return ExpressionUtils.isNullConstant(expression);
- }
-
private void loadThis(final VariableExpression thisOrSuper) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitVarInsn(ALOAD, 0);
@@ -2352,11 +2351,11 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void loadWrapper(final Expression argument) {
+ public void loadWrapper(final Expression expression) {
MethodVisitor mv = controller.getMethodVisitor();
- ClassNode goalClass = argument.getType();
+ ClassNode goalClass = expression.getType();
visitClassExpression(classX(goalClass));
- if (implementsGroovyObject(goalClass)) {
+ if (goalClass.isDerivedFromGroovyObject()) {
createGroovyObjectWrapperMethod.call(mv);
} else {
createPojoWrapperMethod.call(mv);
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 5dcfcd571a..ed471189ce 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -347,6 +347,24 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-10695
+ void testStaticPropertyOfSelfType() {
+ for (qual in ['', 'this.', 'C.']) {
+ assertScript """
+ class C {
+ private static Object value
+ static Object getValue() {
+ ${qual}value
+ }
+ static void setValue(v) {
+ ${qual}value = v
+ }
+ }
+ C.setValue(null) // StackOverflowError
+ """
+ }
+ }
+
void testDateProperties() {
assertScript '''
Date d = new Date()