GROOVY-11884: Close gaps in Groovy annotation validation by paulk-asert · Pull Request #2407 · apache/groovy

paulk-asert · 2026-03-27T23:38:20Z

No description provided.

codecov-commenter · 2026-03-28T00:03:05Z

Codecov Report

❌ Patch coverage is 87.75510% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.4086%. Comparing base (43231bb) to head (8244ea1).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/org/codehaus/groovy/ast/AnnotationNode.java	78.2609%	3 Missing and 2 partials ⚠️
...org/codehaus/groovy/classgen/ExtendedVerifier.java	95.6522%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2407        +/-   ##
==================================================
- Coverage     66.4099%   66.4086%   -0.0013%     
- Complexity      29890      29906        +16     
==================================================
  Files            1392       1394         +2     
  Lines          116823     116893        +70     
  Branches        20675      20682         +7     
==================================================
+ Hits            77582      77627        +45     
- Misses          32873      32893        +20     
- Partials         6368       6373         +5

Files with missing lines	Coverage Δ
...va/groovy/lang/annotation/ExtendedElementType.java	`100.0000% <100.0000%> (ø)`
...org/codehaus/groovy/classgen/ExtendedVerifier.java	`95.0000% <95.6522%> (+0.0633%)`	⬆️
...n/java/org/codehaus/groovy/ast/AnnotationNode.java	`71.6535% <78.2609%> (+2.4228%)`	⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Adds Groovy-specific annotation target validation to close gaps around annotations applied to imports and loop statements, aligning compiler checks with Groovy language constructs that don’t map to java.lang.annotation.ElementType.

Changes:

Introduces @ExtendedTarget / ExtendedElementType to declare Groovy-only targets (currently IMPORT and LOOP).
Extends compiler verification to validate annotations on import statements and statement-level (loop) annotations.
Updates core/test annotations and tests/resources to reflect the new validation rules.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
subprojects/groovy-contracts/src/main/java/groovy/contracts/Invariant.java	Allows `@Invariant` on loops via `@ExtendedTarget(LOOP)`.
src/test/groovy/groovy/annotations/MyIntegerAnno.groovy	Marks test annotation as allowed on imports via `@ExtendedTarget(IMPORT)`.
src/test/groovy/gls/annotations/AnnotationTest.groovy	Adds tests for import annotation validation and `@ExtendedTarget(IMPORT)`.
src/test-resources/core/AnnotatedLoop_02x.groovy	Updates loop annotation expectations by removing an annotation no longer allowed on loops.
src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java	Adds validation for import annotations and statement (loop) annotations.
src/main/java/org/codehaus/groovy/ast/AnnotationNode.java	Adds `IMPORT_TARGET` and `@ExtendedTarget` handling in `isTargetAllowed`.
src/main/java/groovy/transform/Parallel.java	Allows `@Parallel` on loops via `@ExtendedTarget(LOOP)`.
src/main/java/groovy/transform/BaseScript.java	Allows `@BaseScript` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/transform/ASTTest.java	Allows `@ASTTest` on loops via `@ExtendedTarget(LOOP)`.
src/main/java/groovy/lang/annotation/ExtendedTarget.java	New meta-annotation for Groovy-only targets.
src/main/java/groovy/lang/annotation/ExtendedElementType.java	New enum defining Groovy-only element types (`IMPORT`, `LOOP`).
src/main/java/groovy/lang/Newify.java	Allows `@Newify` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/lang/Grapes.java	Allows `@Grapes` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/lang/GrabResolver.java	Allows `@GrabResolver` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/lang/GrabExclude.java	Allows `@GrabExclude` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/lang/GrabConfig.java	Allows `@GrabConfig` on imports via `@ExtendedTarget(IMPORT)`.
src/main/java/groovy/lang/Grab.java	Allows `@Grab` on imports via `@ExtendedTarget(IMPORT)`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T01:46:36Z

src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java

+    @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);
+            }
+        }
+    }


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.

Copilot · 2026-03-28T01:46:37Z

src/main/java/org/codehaus/groovy/ast/AnnotationNode.java

+            return switch (ExtendedElementType.valueOf(name)) {
+                case IMPORT -> IMPORT_TARGET;
+                case LOOP   -> STATEMENT_TARGET;
+            };


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.

Suggested change

return switch (ExtendedElementType.valueOf(name)) {

case IMPORT -> IMPORT_TARGET;

case LOOP -> STATEMENT_TARGET;

};

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;

}

Copilot · 2026-03-28T01:46:37Z

src/main/java/org/codehaus/groovy/classgen/ExtendedVerifier.java

 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;


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.

daniellansun requested a review from Copilot March 28, 2026 01:40

Copilot started reviewing on behalf of daniellansun March 28, 2026 01:41 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

GROOVY-11884: Close gaps in Groovy annotation validation

8244ea1

paulk-asert force-pushed the groovy11884 branch from 88afe5b to 8244ea1 Compare March 28, 2026 11:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GROOVY-11884: Close gaps in Groovy annotation validation#2407

GROOVY-11884: Close gaps in Groovy annotation validation#2407
paulk-asert wants to merge 1 commit intoapache:masterfrom
paulk-asert:groovy11884

paulk-asert commented Mar 27, 2026

Uh oh!

codecov-commenter commented Mar 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

paulk-asert commented Mar 27, 2026

Uh oh!

codecov-commenter commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Mar 28, 2026 •

edited

Loading