Copilot commented on code in PR #2407:
URL: https://github.com/apache/groovy/pull/2407#discussion_r3003915697
##########
src/main/java/org/codehaus/groovy/ast/AnnotationNode.java:
##########
@@ -159,9 +164,39 @@ public boolean isTargetAllowed(final int target) {
return 0x4FF; // GROOVY-11838: JLS 9.6.4.1
});
+ // check @ExtendedTarget for Groovy-specific element types
+ allowedTargets |=
classNode.redirect().getNodeMetaData(ExtendedTarget.class, (k) -> {
+ for (AnnotationNode an : classNode.getAnnotations()) {
+ if
("groovy.lang.annotation.ExtendedTarget".equals(an.getClassNode().getName())) {
+ Expression member = an.getMember("value");
+ int bits = 0;
+ if (member instanceof ListExpression list) {
+ for (Expression e : list.getExpressions()) {
+ bits |= extendedElementTypeBits(e);
+ }
+ } else {
+ bits |= extendedElementTypeBits(member);
+ }
+ return bits;
+ }
+ }
+ return 0;
+ });
+
return (target & allowedTargets) == target;
}
+ private static int extendedElementTypeBits(final Expression e) {
+ if (e instanceof PropertyExpression item) {
+ String name = item.getPropertyAsString();
+ return switch (ExtendedElementType.valueOf(name)) {
+ case IMPORT -> IMPORT_TARGET;
+ case LOOP -> STATEMENT_TARGET;
+ };
Review Comment:
`extendedElementTypeBits` calls `ExtendedElementType.valueOf(name)` without
guarding against `name == null` or unknown values. If an invalid enum constant
is used in `@ExtendedTarget(...)`, the compiler may throw
`IllegalArgumentException`/`NPE` here (potentially masking the existing
validation error) instead of reporting a clean compilation error. Consider
handling null/invalid names (e.g., check for null and catch
`IllegalArgumentException`) and returning 0 or producing a targeted error
message.
```suggestion
if (name == null) {
return 0;
}
try {
return switch (ExtendedElementType.valueOf(name)) {
case IMPORT -> IMPORT_TARGET;
case LOOP -> STATEMENT_TARGET;
};
} catch (IllegalArgumentException ignore) {
// unknown ExtendedElementType value; no extended element
type bits
return 0;
}
```
##########
src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java:
##########
@@ -221,6 +232,32 @@ public void visitMethod(final MethodNode node) {
}
}
+ @Override
+ protected void visitStatementAnnotations(final Statement statement) {
+ for (AnnotationNode annotation : statement.getStatementAnnotations()) {
+ ErrorCollector errorCollector = new
ErrorCollector(source.getConfiguration());
+ AnnotationVisitor visitor = new AnnotationVisitor(source,
errorCollector);
+ AnnotationNode visited = visitor.visit(annotation);
+ source.getErrorCollector().addCollectorContents(errorCollector);
+
+ if (!visited.isTargetAllowed(STATEMENT_TARGET)) {
+ addError("Annotation @" + visited.getClassNode().getName() + "
is not allowed on element "
+ + AnnotationNode.targetToName(STATEMENT_TARGET),
visited);
+ }
+ }
+ }
Review Comment:
This PR adds new validation for statement annotations via
`visitStatementAnnotations`, but the test additions only cover import-level
annotations. Consider adding a focused test that (1) rejects a
non-`@ExtendedTarget(LOOP)` annotation on a loop and (2) accepts an annotation
that declares `@ExtendedTarget(LOOP)`, to ensure this new validation path is
exercised and doesn’t regress.
##########
src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java:
##########
@@ -43,23 +45,27 @@
import org.objectweb.asm.Opcodes;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
Review Comment:
Unused import: `java.util.Collections` is added but not referenced in this
file. Please remove it to keep the import list clean and avoid warnings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
