You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2022/07/16 02:09:33 UTC
[groovy] branch master updated: GROOVY-10679: Annotations not being correctly placed in native records
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new aa25312449 GROOVY-10679: Annotations not being correctly placed in native records
aa25312449 is described below
commit aa25312449f64cd75cd37786ba9028817a108cae
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Jul 14 19:20:21 2022 +1000
GROOVY-10679: Annotations not being correctly placed in native records
---
.../codehaus/groovy/classgen/ExtendedVerifier.java | 25 +++++++++++++++-------
.../org/codehaus/groovy/classgen/Verifier.java | 17 ++++++---------
.../transform/RecordTypeASTTransformation.java | 18 ++++++++++++++--
.../TupleConstructorASTTransformation.java | 15 ++++++++++++-
.../org/codehaus/groovy/classgen/RecordTest.groovy | 19 ++++++++++++++++
5 files changed, 72 insertions(+), 22 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
index ddf44415c2..1dcefc1ede 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java
@@ -47,6 +47,7 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -273,7 +274,10 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
}
this.currentClass.setAnnotated(true);
Map<String, List<AnnotationNode>> nonSourceAnnotations = new LinkedHashMap<>();
- for (AnnotationNode unvisited : annotations) {
+ boolean skippable = node.getNodeMetaData("_SKIPPABLE_ANNOTATIONS") != null;
+ Iterator<AnnotationNode> iterator = annotations.iterator();
+ while (iterator.hasNext()) {
+ AnnotationNode unvisited = iterator.next();
AnnotationNode visited;
{
ErrorCollector errorCollector = new ErrorCollector(source.getConfiguration());
@@ -283,7 +287,10 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
}
String name = visited.getClassNode().getName();
- boolean skip = currentClass.isRecord() && skippableRecordAnnotation(node, visited);
+ if (skippable && shouldSkip(node, visited)) {
+ iterator.remove();
+ continue;
+ }
if (!visited.hasSourceRetention()) {
List<AnnotationNode> seen = nonSourceAnnotations.get(name);
if (seen == null) {
@@ -292,15 +299,13 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
addError("Cannot specify duplicate annotation on the same member : " + name, visited);
}
seen.add(visited);
- if (!skip) {
- nonSourceAnnotations.put(name, seen);
- }
+ nonSourceAnnotations.put(name, seen);
}
// Check if the annotation target is correct, unless it's the target annotating an annotation definition
// defining on which target elements the annotation applies
boolean isTargetAnnotation = name.equals("java.lang.annotation.Target");
- if (!isTargetAnnotation && !skip && !visited.isTargetAllowed(target) && !isTypeUseScenario(visited, target)) {
+ if (!isTargetAnnotation && !visited.isTargetAllowed(target) && !isTypeUseScenario(visited, target)) {
addError("Annotation @" + name + " is not allowed on element " + AnnotationNode.targetToName(target), visited);
}
visitDeprecation(node, visited);
@@ -309,9 +314,13 @@ public class ExtendedVerifier extends ClassCodeVisitorSupport {
processDuplicateAnnotationContainers(node, nonSourceAnnotations);
}
- private boolean skippableRecordAnnotation(AnnotatedNode node, AnnotationNode visited) {
+ private boolean shouldSkip(AnnotatedNode node, AnnotationNode visited) {
return (node instanceof ClassNode && !visited.isTargetAllowed(TYPE_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET) && visited.isTargetAllowed(CONSTRUCTOR_TARGET))
- || (node instanceof FieldNode && !visited.isTargetAllowed(FIELD_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET));
+ || (node instanceof ConstructorNode && !visited.isTargetAllowed(CONSTRUCTOR_TARGET) && visited.isTargetAllowed(TYPE_TARGET))
+ || (node instanceof FieldNode && !visited.isTargetAllowed(FIELD_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET))
+ || (node instanceof Parameter && !visited.isTargetAllowed(PARAMETER_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET))
+ || (node instanceof MethodNode && !(node instanceof ConstructorNode) && !visited.isTargetAllowed(METHOD_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET))
+ || (node instanceof RecordComponentNode && !visited.isTargetAllowed(RECORD_COMPONENT_TARGET) && !visited.isTargetAllowed(TYPE_USE_TARGET));
}
private boolean isRepeatable(final AnnotationNode annoNode) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 6cd9d6528a..404d8fcbb1 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -31,7 +31,6 @@ import org.apache.groovy.ast.tools.ClassNodeUtils;
import org.apache.groovy.util.BeanUtils;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.CodeVisitorSupport;
@@ -100,7 +99,6 @@ import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstan
import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
-import static org.codehaus.groovy.ast.AnnotationNode.METHOD_TARGET;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveDouble;
@@ -823,7 +821,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
setter.setSynthetic(true);
addPropertyMethod(setter);
if (!field.isSynthetic()) {
- copyMethodAnnotations(node, setter);
+ copyAnnotations(node, setter);
}
visitMethod(setter);
}
@@ -833,18 +831,15 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
MethodNode getter = new MethodNode(getterName, getterModifiers, node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
getter.setSynthetic(true);
addPropertyMethod(getter);
- if (!field.isSynthetic()) {
- copyMethodAnnotations(node, getter);
+ if (classNode.getNodeMetaData("_RECORD_HEADER") != null || !field.isSynthetic()) {
+ copyAnnotations(node, getter);
}
visitMethod(getter);
}
- private static void copyMethodAnnotations(final PropertyNode node, final MethodNode accessor) {
- for (AnnotationNode annotationNode : node.getAnnotations()) {
- if (annotationNode.isTargetAllowed(METHOD_TARGET)) {
- accessor.addAnnotation(annotationNode);
- }
- }
+ private static void copyAnnotations(final PropertyNode node, final MethodNode accessor) {
+ accessor.addAnnotations(node.getAnnotations());
+ accessor.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
}
protected void addPropertyMethod(final MethodNode method) {
diff --git a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java
index 387c8c36ec..e60ccaadb5 100644
--- a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java
@@ -93,6 +93,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafeWithGenerics;
import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
@@ -150,6 +151,9 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple
}
private void doProcessRecordType(ClassNode cNode, PropertyHandler handler) {
+ if (cNode.getNodeMetaData("_RECORD_HEADER") != null) {
+ cNode.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
+ }
List<AnnotationNode> annotations = cNode.getAnnotations(RECORD_OPTIONS_TYPE);
AnnotationNode options = annotations.isEmpty() ? null : annotations.get(0);
RecordTypeMode mode = getMode(options, "mode");
@@ -176,7 +180,13 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple
cNode.setRecordComponents(new ArrayList<>());
}
for (PropertyNode pNode : pList) {
- cNode.getRecordComponents().add(new RecordComponentNode(cNode, pNode.getName(), pNode.getOriginType(), pNode.getAnnotations()));
+ ClassNode pType = pNode.getOriginType();
+ ClassNode type = pType.getPlainNodeReference();
+ type.setGenericsPlaceHolder(pType.isGenericsPlaceHolder());
+ type.setGenericsTypes(pType.getGenericsTypes());
+ RecordComponentNode rec = new RecordComponentNode(cNode, pNode.getName(), type, pNode.getAnnotations());
+ rec.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
+ cNode.getRecordComponents().add(rec);
}
} else if (mode == RecordTypeMode.NATIVE) {
addError(message + " when attempting to create a native record", cNode);
@@ -332,9 +342,13 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple
for (PropertyNode pNode : pList) {
String name = pNode.getName();
args.addExpression(ternaryX(callX(mapArg, "containsKey", args(constX(name))), propX(mapArg, name), thisPropX(true, name)));
+ ClassNode pType = pNode.getType();
+ ClassNode type = pType.getPlainNodeReference();
+ type.setGenericsPlaceHolder(pType.isGenericsPlaceHolder());
+ type.setGenericsTypes(pType.getGenericsTypes());
AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE);
namedParam.addMember("value", constX(name));
- namedParam.addMember("type", classX(pNode.getType()));
+ namedParam.addMember("type", classX(type));
namedParam.addMember("required", constX(false, true));
mapParam.addAnnotation(namedParam);
}
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index 6cfd32ba1f..ac2d2505ce 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -248,6 +248,11 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
FieldNode fNode = pNode.getField();
if (shouldSkipUndefinedAware(name, excludes, includes, allNames)) continue;
Parameter nextParam = createParam(fNode, name, defaultsMode, xform, makeImmutable);
+ if (cNode.getNodeMetaData("_RECORD_HEADER") != null) {
+ nextParam.addAnnotations(pNode.getAnnotations());
+ nextParam.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
+ fNode.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
+ }
params.add(nextParam);
}
@@ -272,6 +277,10 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
boolean hasMapCons = AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
ConstructorNode consNode = addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
+ if (cNode.getNodeMetaData("_RECORD_HEADER") != null) {
+ consNode.addAnnotations(cNode.getAnnotations());
+ consNode.putNodeMetaData("_SKIPPABLE_ANNOTATIONS", true);
+ }
if (namedVariant) {
BlockStatement inner = new BlockStatement();
Parameter mapParam = param(ClassHelper.MAP_TYPE.getPlainNodeReference(), NAMED_ARGS);
@@ -310,7 +319,11 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
}
private static Parameter createParam(FieldNode fNode, String name, DefaultsMode defaultsMode, AbstractASTTransformation xform, boolean makeImmutable) {
- Parameter param = new Parameter(fNode.getType(), name);
+ ClassNode fType = fNode.getType();
+ ClassNode type = fType.getPlainNodeReference();
+ type.setGenericsPlaceHolder(fType.isGenericsPlaceHolder());
+ type.setGenericsTypes(fType.getGenericsTypes());
+ Parameter param = new Parameter(type, name);
if (defaultsMode == ON) {
param.setInitialExpression(providedOrDefaultInitialValue(fNode));
} else if (defaultsMode == AUTO && fNode.hasInitialExpression()) {
diff --git a/src/test/org/codehaus/groovy/classgen/RecordTest.groovy b/src/test/org/codehaus/groovy/classgen/RecordTest.groovy
index e93262ab3c..7a72f184cf 100644
--- a/src/test/org/codehaus/groovy/classgen/RecordTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/RecordTest.groovy
@@ -557,4 +557,23 @@ final class RecordTest {
test()
'''
}
+
+ @Test // GROOVY-10679
+ void testAnnotationPropogation() {
+ assertScript '''
+ import java.lang.annotation.*
+
+ @Target([ElementType.FIELD, ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.PARAMETER])
+ @Retention(RetentionPolicy.RUNTIME)
+ public @interface MyAnno { }
+
+ @MyAnno
+ record Car(String make, @MyAnno String model) { }
+
+ assert !Car.getAnnotation(MyAnno)
+ assert Car.getMethod('model').getAnnotation(MyAnno)
+ assert Car.getConstructor(String, String).getAnnotation(MyAnno)
+ assert Car.getConstructor(String, String).getParameterAnnotations()[1][0].annotationType() == MyAnno
+ '''
+ }
}