GROOVY-12292: Honour method-level type-checking annotations over class-level SKIP - #2829
Conversation
…s-level SKIP A method-level @TypeChecked or @CompileStatic annotation with the default (non-SKIP) mode was silently ignored when the declaring class carried a SKIP-mode annotation: isSkipMode recursed to the declaring class without first considering that the method's own annotation had already answered the question. Nested classes take a different path and already honour the more specific annotation (GROOVY-10238), as does the opt-out direction, making methods the lone anomaly. Stop the walk up to the declaring class when the node itself carries one of the visitor's type-checking annotations with a non-SKIP mode: the most specific annotation wins, and class-level SKIP remains the default for members without their own annotation. STATIC_COMPILE_NODE metadata derives from the same method, so an opted-in method under a @CompileDynamic class is now statically compiled, not just checked. Deliberately unchanged, ratified by tests and documentation: SKIP-mode methods inside checked classes are skipped as before, and method-level @CompileStatic(SKIP) / @CompileDynamic disables static compilation only, not an enclosing class's @TypeChecked checking. Behaviour-changing: previously-inert method annotations now take effect, so a COMPATIBILITY.md entry is included; targets Groovy 6 only.
71d5f98 to
47dffb3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2829 +/- ##
==================================================
- Coverage 70.6100% 70.6076% -0.0023%
Complexity 36472 36472
==================================================
Files 1569 1569
Lines 133831 133834 +3
Branches 24666 24667 +1
==================================================
- Hits 94498 94497 -1
- Misses 30821 30823 +2
- Partials 8512 8514 +2
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: 47dffb3 Learn more about TestLens at testlens.app/docs. |
There was a problem hiding this comment.
Pull request overview
Fixes GROOVY-12292 by ensuring method/constructor-level @TypeChecked / @CompileStatic opt-ins (non-SKIP) take precedence over an enclosing class’s SKIP/@CompileDynamic, aligning method behavior with nested-class precedence rules and ensuring static compilation metadata is set consistently.
Changes:
- Update
StaticTypeCheckingVisitor#isSkipModeto stop inheritingSKIPfrom the declaring class when the node itself has an explicit non-SKIPtype-checking annotation. - Add regression tests covering the previously-broken annotation combinations, including a bytecode-level assertion that an opted-in method under
@CompileDynamicis truly statically compiled. - Document the precedence rules in the language spec and record the behavior change in
COMPATIBILITY.md(targeting Groovy 6).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java | Adjusts isSkipMode resolution so explicit non-SKIP annotations on a node override enclosing SKIP. |
| src/test/groovy/org/codehaus/groovy/classgen/asm/sc/CompileDynamicTest.groovy | Adds regression + bytecode tests for @CompileStatic methods inside @CompileDynamic / SKIP classes. |
| src/test/groovy/groovy/transform/stc/TypeCheckingModeTest.groovy | Adds regression tests for @TypeChecked opt-in inside @TypeChecked(SKIP) and cross-family @CompileStatic(SKIP) behavior under @TypeChecked. |
| src/spec/doc/core-semantics.adoc | Documents “most specific annotation wins” precedence and clarifies cross-family opt-out behavior. |
| COMPATIBILITY.md | Adds Groovy 6 compatibility note describing the behavior change and impact. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
GROOVY-12292: https://issues.apache.org/jira/browse/GROOVY-12292
A method-level
@TypeCheckedor@CompileStaticannotation with the default(non-SKIP) mode was silently ignored when the declaring class carried a
SKIP-mode annotation. Four combinations were affected:
@CompileDynamicclass +@CompileStaticmethod@CompileStatic(SKIP)class +@CompileStaticmethod@TypeChecked(SKIP)class +@CompileStaticmethod@TypeChecked(SKIP)class +@TypeCheckedmethodThe cause is in
StaticTypeCheckingVisitor#isSkipMode, which recursed to thedeclaring class without first considering that the method's own annotation had
already answered the question. Nested classes take a different code path and
already honour the more specific annotation (GROOVY-10238), as does the
opt-out direction (checked class + SKIP method), making methods the lone
anomaly. The behaviour is longstanding (reproduced identically on 4.0.27,
5.0.6 and 6.0.0-alpha-1).
The fix stops the walk up to the declaring class when the node itself carries
one of the visitor's type-checking annotations with a non-SKIP mode: the most
specific annotation wins, and a class-level SKIP remains the default for
members without their own annotation. Because
StaticCompileTransformationderives the
STATIC_COMPILE_NODEmetadata from the same method, bytecodegeneration follows: an opted-in method under a
@CompileDynamicclass is nowstatically compiled, not just checked (asserted in the new bytecode test).
Deliberately unchanged, now ratified by tests and documentation:
before;
@CompileStatic(SKIP)/@CompileDynamicdisables static compilation but does not exempt themethod from an enclosing class's
@TypeCheckedchecking; only@TypeChecked(SKIP)does.Since the change means previously-dynamic (and unchecked) method bodies are
now checked and statically compiled, it is behaviour-changing: a
COMPATIBILITY.md entry is included and the JIRA issue should get the
breakinglabel, targeting 6.0 only. Documentation of the precedence rulesis added to the "Skipping sections" part of core-semantics.adoc.