GROOVY-12115: Reify a receiver's type argument as a Class parameter u…

[paulk-asert]

…nder @CompileStatic (@ClassTag)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.25000% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.8875%. Comparing base (a2ff978) to head (862c01e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...codehaus/groovy/transform/stc/ClassTagSupport.java	84.0491%	4 Missing and 22 partials ⚠️
...codehaus/groovy/control/CompilerConfiguration.java	47.3684%	9 Missing and 1 partial ⚠️
...roovy/transform/stc/StaticTypeCheckingVisitor.java	88.2353%	0 Missing and 2 partials ⚠️
...vy/transform/stc/AbstractExtensionMethodCache.java	88.8889%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2645        +/-   ##
==================================================
+ Coverage     69.8702%   69.8875%   +0.0173%     
- Complexity      35139      35228        +89     
==================================================
  Files            1554       1555         +1     
  Lines          130731     130939       +208     
  Branches        23916      23990        +74     
==================================================
+ Hits            91342      91510       +168     
- Misses          31144      31156        +12     
- Partials         8245       8273        +28     

Files with missing lines	Coverage Δ
.../codehaus/groovy/runtime/DefaultGroovyMethods.java	75.0200% <ø> (+0.0533%)	⬆️
...vy/transform/stc/AbstractExtensionMethodCache.java	85.2941% <88.8889%> (+0.5484%)	⬆️
...roovy/transform/stc/StaticTypeCheckingVisitor.java	87.4618% <88.2353%> (+0.0034%)	⬆️
...codehaus/groovy/control/CompilerConfiguration.java	72.0859% <47.3684%> (-1.5297%)	⬇️
...codehaus/groovy/transform/stc/ClassTagSupport.java	84.0491% <84.0491%> (ø)

... and 3 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

This PR implements GROOVY-12115 by introducing @ClassTag, enabling the static type checker to synthesize missing Class<T> token arguments from a receiver’s generic type arguments under @TypeChecked/@CompileStatic. It also adds a compiler-configuration allowlist to control when this token synthesis may preempt an existing token-less overload (defaulting to Groovy’s withDefault).

Changes:

• Add groovy.transform.stc.@ClassTag and integrate token-synthesis + retry selection into StaticTypeCheckingVisitor.
• Annotate relevant GDK APIs (asChecked, checked withDefault overloads) so tokens can be synthesized under static checking.
• Add CompilerConfiguration.classTagPreemptionTargets (with tests) and introduce new STC tests covering additive injection, preemption, rollback behavior, and config gating.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/main/java/groovy/transform/stc/ClassTag.java	New parameter annotation defining the @ClassTag contract for compiler-supplied Class tokens under static checking.
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java	Implements the token injection/preemption logic, retry selection, rollback behavior, and validates @ClassTag("...") overrides.
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java	Marks asChecked and checked withDefault overload token parameters with @ClassTag and updates documentation accordingly.
src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java	Adds configurable allowlist for preemptive @ClassTag overload selection and preserves immutability of CompilerConfiguration.DEFAULT.
src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java	Tests defaults, copy semantics, null-handling, and immutability for classTagPreemptionTargets.
src/test/groovy/groovy/transform/stc/ClassTagStaticTest.groovy	New STC regression tests validating token synthesis, preemption behavior, rollback, and configuration gating under @CompileStatic.

Comments suppressed due to low confidence (1)

src/main/java/groovy/transform/stc/ClassTag.java:58

• Minor grammar issue in the Javadoc: “a wildcard-only type do not” should be “does not”.

 *   <li><em>The type variable must be statically known.</em> {@code List<String>} works;
 *   {@code def} / raw {@code List} / a wildcard-only type do not &mdash; no token is synthesised.</li>

[daniellansun]

Request changes before merge:

1. Method selection: inject → re-select → rollback

In visitMethodCallExpression, the implementation mutates argumentList in place, re-runs a near-duplicate receiver/findMethod loop, then clears and restores the original arguments on failure. That couples AST surgery to selection success via an implicit rollback contract and duplicates control flow already present in the same method.

Suggestion: Treat @ClassTag matching as a pure computation (no AST mutation). Reuse a single selectOnReceivers(...) for both plain and expanded argument types. Commit the argument rewrite only after tagged selection succeeds. That removes the rollback path and the duplicated loop.

---

2. Preemption policy via method-name config is the wrong layer

CompilerConfiguration.classTagPreemptionTargets gates preemption by bare method name (default "withDefault"). Method names are not unique, so any type’s withDefault shares the policy. User APIs that want preemption need both parameter annotations and a compiler config change—the library cannot express intent alone.

Suggestion: Co-locate preemption with the API (e.g. an attribute on @ClassTag, or a method-level marker). Keep configuration, if needed, as a global kill-switch rather than a name allowlist.

---

3. ClassTag logic should not grow StaticTypeCheckingVisitor

Roughly 250 lines of cohesive feature logic were added to a visitor already past 7k lines. The package already factors helpers into types such as StaticTypeCheckingSupport.

Suggestion: Extract matching/resolution into a dedicated helper (or Support methods) and leave a thin call site in the visitor.

Also: The block was inserted between findMethodsWithGenerated’s Javadoc and the method body, which orphans that documentation. Please restore adjacency.

---

4. Selection model and contract gaps

• Arity-only matching with deferred applicability forces the mutate/rollback design; fold reifiability and remaining-parameter applicability into one pass so a failed match never touches the AST.
• When equal-token candidates disagree on erased types, injection is abandoned silently—consider a diagnostic or a clearer failure policy for API authors.
• Tokens may be resolved against one receiver while selection binds on another (multi-receiver / delegate-owner cases). Bind (receiver, match) together.
• @ClassTag documentation emphasises trailing Class arguments, but any parameter position is supported; preemption is undocumented on the annotation. Align the docs with behaviour.
• On source methods, @ClassTag on a non-Class parameter should error rather than silently disable injection (same class of bug as the override typo check).

[paulk-asert]

Thanks @daniellansun and @blackdrag for the thorough reviews — nearly everything has been actioned, and the preemption design has been reworked substantially in response. Details below; the branch is updated.

Preemption redesign: intent + containment + consent

The classTagPreemptionTargets name allowlist is gone. Daniel was right that a config-level name list was the wrong layer (names aren't unique, and a library couldn't express intent alone), and Jochen's mailing-list concern was that preemption changes the meaning of existing source at all. The replacement splits the decision across three parties, each owning the part that's theirs:

1. Intent (API author) — @ClassTag gains preempt() default false. Only an overload explicitly marked @ClassTag(preempt=true) can ever displace a token-less incumbent; unmarked tags are strictly additive. A library now self-declares — no consumer configuration needed. DGM's checked withDefault overloads declare it; asChecked does not (nothing to preempt).

2. Containment (structural invariant, not configurable) — a tagged overload may only preempt a token-less overload declared by the same owner: for an extension-method incumbent, tagged overloads of the same extension class; for an instance-method incumbent, the receiver's own class hierarchy. This closes what I think is the most important hole in author-declared preemption: without it, any jar on the compile classpath could ship e.g. collect(Iterable, @ClassTag(preempt=true) Class, Closure) and silently capture every statically-compiled collect{} call on the next rebuild. With the same-owner rule, a library can upgrade callers of its own lenient API but structurally cannot capture calls it doesn't own — the residual exposure reduces to "a library can change the behaviour of calls to itself", which it could already do by editing its method bodies.

3. Consent (consumer) — CompilerConfiguration.classTagPreemptionDisabled, a veto set using the same selector forms as groovy.extension.disable: a bare name (withDefault), an exact signature in self-first static form (withDefault(Map,Class,Closure)), or * to disable all preemption. Seeded from -Dgroovy.classtag.preemption.disable, so no config script is needed for the common cases. Default empty = honour declared intent.

For Jochen specifically: * (or simply never writing preempt=true) is now a first-class "no preemption ever" mode, and out of the box the only method that preempts anything is withDefault — whose lenient variant is the GROOVY-11807 bug this exists to route around.

Daniel's review items

• GROOVY-7159: STC claims non-existent Diamond in Anonymous Inner Class wi... #1 inject → re-select → rollback: reworked as suggested. Matching is now a pure computation (ClassTagSupport.matchAdditive/matchPreemption); tagged argument types are computed by splicing token types into the args array, and the AST is rewritten only after the retried selection binds. The rollback path and the duplicated receiver loop are gone.
• Bug Fix for GROOVY-7259 #3 visitor growth + orphaned Javadoc: all ClassTag logic extracted to a new package-private ClassTagSupport (~25-line call site remains in the visitor, net −230 lines); findMethodsWithGenerated's Javadoc is reunited with its method.
• #4a arity-only matching: subsumed by the pure-match refactor.
• #4b silent ambiguity: still fail-soft by design (degrading to the call as written must not become an error), but I agree the additive-case "no matching method" error deserves a hint that injection was attempted and abandoned — follow-up.
• #4c receiver binding: fixed. Additive matching walks receivers in delegate/owner priority order and constrains the retry to the matched receiver; preemptive matching reifies from the receiver the incumbent bound on, so an upgrade can change overload but never binding target.
• #4d docs: annotation Javadoc no longer says "trailing" (tokens fill declared positions, anywhere in the list), and preemption + the veto config are now documented on the annotation itself.
• #4e non-Class parameter: now a compile-time error on source methods, parallel to the override-typo check.

Jochen's test questions

1. token-less overload alongside the tagged one — covered by testUserApiWithoutDeclaredIntentStaysLenient / testUserApiDeclaresPreemptionIntent (and now also testUserApiIntentVetoedByConsumer).
2. switched Class<V>/Class<K> positions — testTokensReorderedByTypeVariableNotPosition covers the swap; new testTokensInterleavedWithSuppliedArguments additionally covers a token declared after a supplied argument.
3. method-declared type parameters — this was a genuinely good catch: chasing it surfaced that a method-declared type variable shadowing a class-level one (per Java scoping) could have reified the wrong class. Now: on instance methods compiled from source, tagging a method-declared variable is a compile-time error; a matching-side guard makes precompiled libraries degrade to no-injection rather than mis-inject; static self-style methods (the extension-authoring pattern) are exempt since there the method-level variable is the mechanism. Tests: testMethodTypeParameterIsRejectedAtCompileTime, testShadowedTypeParameterIsRejectedAtCompileTime, testStaticSelfStyleMethodTypeParameterAccepted.

Status / open items

ClassTagStaticTest 32/32, CompilerConfigurationTest 15/15, full groovy.transform.stc.* (2012 tests) and classgen.asm.sc.* (2295 tests) sweeps green, Groovy11807 regression green.

Two known follow-ups: (a) the preemption check now runs without the old O(1) name pre-gate — it's bounded (cached DGM lookup filtered by owner / receiver-hierarchy scan), but I'd like a compile-time benchmark on a large @CompileStatic codebase before merge, with an ExtensionMethodCache-level "names with preempt intent" cache as the optimization if needed; (b) the cross-module containment ("an extension module cannot capture DGM's methods") is implemented but wants an m12n-style fixture-jar integration test.

Comments on the revised model welcome — particularly whether preempt=true + same-owner containment + selector veto addresses the "why is one method better" and "wrong layer" concerns, respectively.

[paulk-asert]

Follow-up (a) in more detail: preemption-check cost — benchmark plan + proposed gate

Expanding on the compile-time concern noted above, since it's the one open item that could block merge.

The cost, precisely

The earlier allowlist design had an accidental virtue: classTagPreemptionTargets.contains(name) was an O(1) pre-gate that was false for virtually every call, so resolved calls paid nothing. In the revised design, isPreemptionPossible() is still O(1) but almost always true, after which matchPreemption runs for every resolved method call under @TypeChecked/@CompileStatic:

• an incumbent-kind scan over the matched methods (cheap, usually one element);
• for extension-method incumbents — the dominant case (each, collect, …) — a findDGMMethodsForClassNode lookup, which is cache-backed but still walks the receiver hierarchy and filters by name per call;
• intent/veto checks that call getAnnotations(...) per parameter per candidate.

Almost always the outcome is "no preempt-intent overload exists here" — we pay enumeration cost to discover a negative that is knowable in advance. That's the target.

Proposed gate: a per-classloader "preemptive names" set in ExtensionMethodCache

ExtensionMethodCache already solves the exact lifecycle problem we need: it caches the receiver→methods map per classloader and invalidates when extension modules change. The plan piggybacks on the same scan:

• Change the cached value to a small holder — {byReceiverType map; preemptiveNames set} — so the name set shares the map's lifecycle and can never go stale independently (no separate invalidation logic to get wrong).
• While AbstractExtensionMethodCache accumulates each MethodNode, additionally record the method name if any parameter carries @ClassTag(preempt=true). One-time cost, amortized across the compilation.
• Fast path in matchPreemption: when all incumbents are extension methods and !preemptiveNames.contains(name), return before any candidate enumeration. Out of the box that set contains exactly {"withDefault"}, so ~every DGM call short-circuits on one set lookup — restoring the allowlist's O(1) profile while keeping author self-declaration.

Two constraints worth stating: the gate must be conservative (false positives fall through to the full match, which fails softly; false negatives would be a correctness bug — hence deriving the set from the same scan that feeds method lookup, never a hand-maintained list), and it only covers the extension-incumbent path. Instance-method incumbents still scan the receiver's same-name overloads — bounded, and the rarer path. If measurement says that matters, the cheap fix is reordering matchOverload to compare arity (integer math) before scanning tag annotations (list allocation), so untagged overloads bail for free; I'd hold that until the data asks for it.

Benchmark plan

Compile-time benchmarks are noisy, so: JMH, fresh GroovyClassLoader/CompilationUnit per invocation, compiling to INSTRUCTION_SELECTION (captures all STC work, skips classgen noise), average-time mode with proper warmup rather than single-shot.

Corpus: a generated synthetic source set (reproducible, tunable) — ~200 @CompileStatic classes × 50 calls, in three mixes as benchmark params:

• dgmHeavy — DGM calls on typed receivers (the path the gate targets);
• instanceHeavy — same-class/hierarchy instance calls with overloads (the path it doesn't);
• withDefault — calls that actually preempt, confirming the happy path doesn't regress.

Variants: (1) master baseline; (2) this branch as-is; (3) branch with classTagPreemptionDisabled = ['*'] — the floor the gate could reach; (4) branch + name-set gate.

Acceptance criteria: if (2) is within ~1–2% of (1) on dgmHeavy, ship as-is and the gate becomes optional hygiene; otherwise land the gate and expect (4) ≈ (3) ≈ (1). A coarse cross-check against the existing :performance corpus is easy to add, but the variant comparison wants the JMH harness.

Scope note: I don't propose extending the gate to instance methods via per-ClassNode cached flags — the invalidation story during compilation (classes still being built, AST mutation) is genuinely tricky, and the bounded per-call scan is very likely in the noise. The benchmark exists to prove or refute that cheaply before adding machinery.

I'll post numbers here once the harness runs; if anyone has a preferred real-world @CompileStatic-heavy corpus to include alongside the synthetic one, suggestions welcome.

[paulk-asert]

Follow-up (a): done — preemption-check cost measured, gate landed

Resolving the compile-time question I flagged above. Short version: the concern was real, the gate fixes it, and it's now measured rather than asserted.

Benchmark

Added ClassTagResolutionBench (JMH, in :performance) — a generated @CompileStatic corpus compiled to INSTRUCTION_SELECTION, fresh CompilationUnit per invocation. Three call mixes and a controlled active vs vetoAll comparison, where vetoAll sets classTagPreemptionDisabled=['*'] so the preemption path is never entered — the pre-feature floor for a corpus whose calls all resolve. active − vetoAll is therefore the cost of active preemption matching. The corpus and the numbers below live in the benchmark's Javadoc so they travel with the code.

The regression, confirmed

Without the gate, preemption matching runs on every resolved call once a token-less overload has matched:

mix            active   vetoAll(floor)   delta
dgmHeavy       121.8    109.7            +11.1%   <- ordinary each/collect/findAll code
instanceHeavy   52.8     51.0            + 3.6%
withDefault    263.1    228.7            +15.0%   <- preemption actually firing every call

(avgt ms/op, 60 classes × 80 calls, 2 forks × 5+5 iterations.)

The dgmHeavy +11% is the pathology: each/collect/findAll on typed receivers each pay a full DGM candidate scan + intent check, only to find nothing preemptive (no DGM method except withDefault declares intent). Pure overhead — well past the ~1–2% I'd have shipped as-is on.

The gate

ExtensionMethodCache now exposes getPreemptiveNames(loader) — a per-classloader Set<String> of extension-method names that declare @ClassTag(preempt=true), derived lazily from the already-cached method map and sharing its lifecycle (so it can never go stale independently). Out of the box it holds exactly {"withDefault"}. matchPreemption consults it before the extension candidate scan: if the call's name isn't in the set, one HashSet.contains replaces the whole scan+filter. Conservative by construction — a false positive falls through to the full match and fails softly; a false negative can't occur because any intent-declaring overload's name is in the set by derivation. The instance-incumbent path is untouched (it must still see subclass-preempts-superclass), and additive matching is untouched (it only runs on otherwise-unresolved calls).

Post-gate

mix            active(gated)   vetoAll(floor)   pre-gate
dgmHeavy       108.1           110.3            121.8    <- regression eliminated (at the floor)
instanceHeavy   54.6            51.4             52.8
withDefault    269.7           227.0            263.1    <- unchanged; still does the real work

dgmHeavy gated active is back at the vetoAll floor, within noise — the +11% is gone. withDefault is deliberately unchanged: it's in the preemptive-names set, so it still does the full token synthesis + rewrite the feature exists to do (this number is a worst case — every call preempts, which no real code does).

Diff and verification

Three files: the benchmark (new), AbstractExtensionMethodCache (+21, the name-set cache), ClassTagSupport (+7, the guard). ClassTagStaticTest 32/32, CompilerConfigurationTest 15/15, Groovy11807 5/5, and the full groovy.transform.stc.* (2012 tests) + classgen.asm.sc.* (2295 tests) sweeps green — the shared-cache change is safe.

One documented residual

instanceHeavy still shows ~+6% (gated 54.6 vs floor 51.4) on a corpus where every call is to a same-class overloaded method. The gate deliberately doesn't cover the instance path — a per-ClassNode equivalent runs into invalidation-during-compilation complexity, and real instance calls are overwhelmingly non-overloaded (the scan returns one method, trivial). I've left it as a documented residual rather than adding speculative ClassNode-metadata caching; happy to revisit if anyone has a real-world instanceHeavy profile that justifies it.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 862c01e
▶️ Tests: 107160 executed
⚪️ Checks: 31/31 completed

---

Learn more about TestLens at testlens.app.