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:42 UTC
[groovy] branch GROOVY-10695 created (now c790e29736)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-10695
in repository https://gitbox.apache.org/repos/asf/groovy.git
at c790e29736 GROOVY-10695: `Type.name` within `Type` gets explicit-`this` treatment
This branch includes the following new commits:
new c790e29736 GROOVY-10695: `Type.name` within `Type` gets explicit-`this` treatment
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-10695: `Type.name` within `Type` gets explicit-`this` treatment
Posted by em...@apache.org.
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()