Copilot commented on code in PR #2618: URL: https://github.com/apache/groovy/pull/2618#discussion_r3456806143
########## src/test/groovy/org/codehaus/groovy/transform/traitx/TraitStaticDispatchMatrix.groovy: ########## @@ -0,0 +1,543 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Trait-static dispatch behaviour matrix — the executable companion to +// GEP-22 § Static members. Three plain-class baseline rows (A/B/C), the +// 12 design rows from GEP-22-progression-analysis.html (with row 11 +// expanded to rows 11/11c/11esc covering the trait-shadows-superclass +// quadrant + the T.super.m() escape; row 9 paired with row 9acc covering +// the static-field non-goal and its accessor workaround), and four +// @Anchored-marker rows characterising the GROOVY-12093 proposal. +// +// Runs against Groovy 4 and 6 either: +// * via gradle (`./gradlew :test --tests '*.TraitStaticDispatchMatrix'`) +// * standalone, against any SDKMAN-installed Groovy: +// `groovy src/test/groovy/.../traitx/TraitStaticDispatchMatrix.groovy` +// +// Two kinds of test: +// * Matrix tests assert OBSERVED behaviour. They pass on every +// currently-shipping Groovy that delivers the spec behaviour and turn +// RED on releases that deviate (e.g. rows 1/10/12 on Groovy 5.0.0– +// 5.0.6 / 6.0.0-alpha-1, which carry the GROOVY-8854 regression +// corrected by GROOVY-11985 in 5.0.7 / 6.0.0-alpha-2). That red is +// the intended bug-detector signal, not a defect in the test. +// * @NotYetImplemented tests assert GROOVY-12093 desired-end-state +// behaviour unmet on every currently-shipping Groovy (the four +// @Anchored variants 1a/7a/8a/13). Each references +// `groovy.transform.Anchored` by import; on any Groovy without the +// annotation the import fails and the test body throws, which +// @NotYetImplemented swallows. On a Groovy built with the spike +// (branch `spike/anchored-annotation`), the assertions hold and the +// NYI expectation flips red — the cue to drop @NotYetImplemented. Review Comment: The header comment still describes the `@Anchored` rows as `@NotYetImplemented` tests that only work on a spike branch. In this PR, `@Anchored` is added and the rows are real assertions, so this description is misleading for future maintainers. ########## src/test/groovy/org/codehaus/groovy/transform/traitx/Groovy12093.groovy: ########## @@ -0,0 +1,249 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.transform.traitx + +import org.junit.Test + +import static groovy.test.GroovyAssert.assertScript +import static groovy.test.GroovyAssert.shouldFail + +/** + * Exercises the {@code @Anchored} marker on trait static methods (GROOVY-12093). + * + * <p>The marker selects declarer-bound dispatch (Java/interface-static + * flavour) in place of the default override-flavour dispatch, and (by + * default) also promotes the annotated method onto the generated trait + * interface. + * + * <pre> + * plain static m() -> override flavour (dispatch via + * implementer; Grails Validateable + * canary; Groovy 4.0.32 + post-#2529) + * {@literal @}Anchored static m() -> declarer-bound dispatch + on the + * generated trait interface + * {@literal @}Anchored(inInterface=false) + * static m() -> declarer-bound dispatch only, not + * published on the interface + * </pre> + * + * Dispatch is implemented in {@code TraitReceiverTransformer}; interface + * promotion is implemented in {@code TraitASTTransformation}. + */ +final class Groovy12093 { + + // Basic-case coverage lives in TraitStaticDispatchMatrix: + // * plain-static override visible to trait body → matrix row 1 + // * @Anchored makes dispatch trait-anchored → matrix row 1a + // * @Anchored from-trait T.m() → matrix row 7a + // * @Anchored external Trait.m() → matrix row 8a + // * @Anchored(inInterface=false) opt-out → matrix row 13 + // This class covers the @Anchored-specific tests that the matrix + // doesn't carry: per-callee keying, @CompileStatic variants, the + // inInterface=false STC compile-error case, and validation that + // @Anchored is rejected on non-public-static targets. + + /** + * The marker is keyed on the <em>callee</em>, not the caller — same + * discipline as the existing private-static escape. + */ + @Test + void anchored_isKeyedOnCallee() { + assertScript ''' + import groovy.transform.Anchored + + trait V { + static boolean overridable() { false } + @Anchored + static boolean anchored() { false } + // Same caller, different callees: result depends on each + // callee's own marker, not the caller's. + static List<Boolean> bothSeen() { [this.overridable(), this.anchored()] } + } + class Over implements V { + static boolean overridable() { true } + static boolean anchored() { true } + } + assert Over.bothSeen() == [true, false] + ''' + } + + /** Plain {@code static} inside {@code @CompileStatic} still routes via the implementer. */ + @Test + void plainStatic_compileStatic_overrideVisible() { + assertScript ''' + @groovy.transform.CompileStatic + trait V { + static String name() { 'trait' } + static String seen() { this.name() } + } + @groovy.transform.CompileStatic + class Impl implements V { static String name() { 'impl' } } + + assert Impl.seen() == 'impl' + ''' + } + + /** {@code @Anchored} inside {@code @CompileStatic} stays trait-anchored. */ + @Test + void anchored_compileStatic_traitAnchored() { + assertScript ''' + import groovy.transform.Anchored + + @groovy.transform.CompileStatic + trait V { + @Anchored + static String name() { 'trait' } + static String seen() { this.name() } + } + @groovy.transform.CompileStatic + class Impl implements V { static String name() { 'impl' } } + + assert Impl.seen() == 'trait' + ''' + } + + /** + * STC sanity check: external {@code V.name()} from a {@code @CompileStatic} + * caller type-checks and runs — the interface static is visible to the + * static type checker, not just to dynamic dispatch. + */ + @Test + void anchored_compileStatic_externalTraitDotM_typeChecks() { + assertScript ''' + import groovy.transform.Anchored + import groovy.transform.CompileStatic + + trait V { + @Anchored + static String name() { 'trait' } + } + class Impl implements V { } + + @CompileStatic + static String callExternally() { + V.name() // must type-check against the interface static + } + + assert callExternally() == 'trait' + ''' + } + + /** + * STC sanity check: from-trait {@code V.m()} from inside a + * {@code @CompileStatic} trait body also type-checks against the + * interface static. + */ + @Test + void anchored_compileStatic_fromTraitT_DotM_typeChecks() { + assertScript ''' + import groovy.transform.Anchored + + @groovy.transform.CompileStatic + trait V { + @Anchored + static String name() { 'trait' } + static String forced() { V.name() } // trait-qualified, under @CS + } + @groovy.transform.CompileStatic + class Over implements V { static String name() { 'over' } } + + assert Over.forced() == 'trait' + ''' + } + + /** + * STC sanity check: with the {@code inInterface=false} opt-out, external + * {@code V.name()} from a {@code @CompileStatic} caller should fail to + * compile — no interface static to bind against. + */ + @Test + void anchored_inInterfaceFalse_compileStatic_externalCall_isCompileError() { + def err = null + try { + new GroovyShell().evaluate ''' + import groovy.transform.Anchored + import groovy.transform.CompileStatic + + trait V { + @Anchored(inInterface=false) + static String name() { 'trait' } + } + class Impl implements V { } + + @CompileStatic + static String callExternally() { + V.name() // not on the interface — should be a compile error under @CS + } + callExternally() + ''' + } catch (Throwable t) { + err = t + } + assert err != null : 'expected @CompileStatic external V.name() to fail when inInterface=false' + } Review Comment: This is intended to assert a compile-time failure, but catching any Throwable can let the test pass for unrelated runtime errors. Using shouldFail(MultipleCompilationErrorsException) makes the expectation precise and self-documenting. ########## src/test/groovy/org/codehaus/groovy/transform/traitx/TraitStaticDispatchMatrix.groovy: ########## @@ -0,0 +1,543 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Trait-static dispatch behaviour matrix — the executable companion to +// GEP-22 § Static members. Three plain-class baseline rows (A/B/C), the +// 12 design rows from GEP-22-progression-analysis.html (with row 11 +// expanded to rows 11/11c/11esc covering the trait-shadows-superclass +// quadrant + the T.super.m() escape; row 9 paired with row 9acc covering +// the static-field non-goal and its accessor workaround), and four +// @Anchored-marker rows characterising the GROOVY-12093 proposal. +// +// Runs against Groovy 4 and 6 either: +// * via gradle (`./gradlew :test --tests '*.TraitStaticDispatchMatrix'`) +// * standalone, against any SDKMAN-installed Groovy: +// `groovy src/test/groovy/.../traitx/TraitStaticDispatchMatrix.groovy` +// +// Two kinds of test: +// * Matrix tests assert OBSERVED behaviour. They pass on every +// currently-shipping Groovy that delivers the spec behaviour and turn +// RED on releases that deviate (e.g. rows 1/10/12 on Groovy 5.0.0– +// 5.0.6 / 6.0.0-alpha-1, which carry the GROOVY-8854 regression +// corrected by GROOVY-11985 in 5.0.7 / 6.0.0-alpha-2). That red is +// the intended bug-detector signal, not a defect in the test. +// * @NotYetImplemented tests assert GROOVY-12093 desired-end-state +// behaviour unmet on every currently-shipping Groovy (the four +// @Anchored variants 1a/7a/8a/13). Each references +// `groovy.transform.Anchored` by import; on any Groovy without the +// annotation the import fails and the test body throws, which +// @NotYetImplemented swallows. On a Groovy built with the spike +// (branch `spike/anchored-annotation`), the assertions hold and the +// NYI expectation flips red — the cue to drop @NotYetImplemented. +// +// Verdicts encoded here are PROPOSED PENDING TEAM REVIEW where the +// underlying design point is not yet ratified (the four @Anchored rows +// in particular). + +package org.codehaus.groovy.transform.traitx + +import groovy.test.GroovyAssert +import groovy.test.NotYetImplemented +import org.junit.Test +import org.junit.runner.JUnitCore + +class TraitStaticDispatchMatrix { + + /** Major version of the running Groovy: 4, 5, 6, ... */ + static final int MAJOR = GroovySystem.version.tokenize('.')[0] as int + + /** Compile+run a snippet with the *running* Groovy compiler. */ + private static Object ev(String src) { new GroovyShell().evaluate(src) } + + // =================== PLAIN-CLASS BASELINE (NO TRAITS) =================== + // Establishes what plain Groovy does for the same dispatch shapes the + // trait rows below test. Read these first — the trait rows characterise + // how trait machinery DEPARTS from this baseline, not how it matches it. + // + // VERIFIED on 4.0.32: plain Groovy static dispatch is declarer-bound in + // every context (method body, closure body, dynamic, @CompileStatic); + // only INSTANCE methods are polymorphic. Trait row 1's "override visible + // to trait method body" is therefore a deliberate departure from this + // baseline (the per-implementer $static$self mechanism); the row 2 + // closure carve-out keeps trait closure-context dispatch consistent + // with this baseline. The simplified-model RULE wording "dispatch like + // a plain class member" was an aspirational fiction — the empirical + // truth is more nuanced and the baseline below records it. + + // ---- Baseline A — plain class static, this.foo() in METHOD BODY ---- + // The control for trait row 1. Plain Groovy: declarer-bound ('base'). + // Trait row 1 returns 'true' (override visible) — that's the trait + // distinctive, NOT a plain-class semantic. + @Test + void baseline_A_plainStatic_methodBody_isDeclarerBound() { + def r = ev ''' + class Base { + static String foo() { 'base' } + static String seen() { this.foo() } + } + class Sub extends Base { static String foo() { 'sub' } } + Sub.seen() + ''' + assert r == 'base' : "baseline A: plain-class static must be declarer-bound, got ${r}" + } + + // ---- Baseline B — plain class static, this.foo() in CLOSURE BODY ---- + // The control for trait row 2 (the closure carve-out). Plain Groovy: + // declarer-bound ('base') — SAME as baseline A. There is no plain-class + // carve-out; static dispatch is uniformly declarer-bound. The trait + // carve-out aligns trait closure-context with this baseline; removing + // it (Move 3) would make traits MORE polymorphic than plain Groovy in + // closure context, not less. + @Test + void baseline_B_plainStatic_closureBody_isDeclarerBound() { + def r = ev ''' + class Base { + static String foo() { 'base' } + static String seen() { [1].collect { this.foo() }[0] } + } + class Sub extends Base { static String foo() { 'sub' } } + Sub.seen() + ''' + assert r == 'base' : "baseline B: plain-class static stays declarer-bound in closures too, got ${r}" + } + + // ---- Baseline C — plain class INSTANCE method, this.foo() in CLOSURE BODY ---- + // Confirms ordinary instance dispatch IS polymorphic in closure context + // (row 6 in the trait matrix is the instance-method analogue). Only + // static dispatch falls back to declarer-bound. Together baselines A/B/C + // show: trait row 1's override-via-implementer in method body is a + // deliberate Groovy-trait distinctive — neither plain statics nor plain + // instance methods give that shape in closure context unaided. + @Test + void baseline_C_plainInstance_closureBody_isPolymorphic() { + def r = ev ''' + class Base { + String foo() { 'base' } + String seen() { [1].collect { this.foo() }[0] } + } + class Sub extends Base { String foo() { 'sub' } } + new Sub().seen() + ''' + assert r == 'sub' : "baseline C: plain-class instance dispatch must be polymorphic everywhere, got ${r}" + } + + // ============================ TRAIT MATRIX ============================ + + // ---- Row 1 — public static, this./unqualified, impl overrides ---- + // Scenario: overridable static defaults (Grails Validateable.defaultNullable). + // SPEC-NORMATIVE end-state (GEP-22 § Static members, item 4): override + // visible to trait code. Holds on 4.x (always did) and on 5.0.7+ / + // 6.0.0-alpha-2+ (where GROOVY-11985 corrects the GROOVY-8854 regression). + // Red on 5.0.0–5.0.6 and 6.0.0-alpha-1 = bug detector for those releases. + // Contrast baseline A (plain class same shape — declarer-bound, 'base'): + // this row's polymorphic answer is the trait-machinery distinctive. + @Test + void row01_publicStatic_overrideSeenByTrait() { + def r = ev ''' + trait V { + static boolean defaultNullable() { false } + static boolean seenThis() { this.defaultNullable() } + static boolean seenUnqualified() { defaultNullable() } + } + class Over implements V { static boolean defaultNullable() { true } } + class Def implements V { } + [ overThis: Over.seenThis(), overUnq: Over.seenUnqualified(), + defThis: Def.seenThis(), direct: Over.defaultNullable() ] + ''' + assert r.direct == true // direct call: override always wins + assert r.defThis == false // row 1': no override -> trait default + assert r.overThis == true : "row1 this. : override must be visible to trait body, got ${r.overThis} — on 5.0.0–5.0.6 / 6.0.0-alpha-1 this is the GROOVY-8854 regression (fixed in 5.0.7 / 6.0.0-alpha-2 by GROOVY-11985)" + assert r.overUnq == true : "row1 unqual: override must be visible to trait body, got ${r.overUnq}" + } + + // ---- Row 1a — @Anchored static, trait body sees trait's own copy ---- + // The dispatch half of the @Anchored marker. With the annotation, + // `this.m()`/`m()` in the trait body should dispatch to the trait's own + // copy regardless of any implementer override (the JVM/interface-static + // model — Eric's "static implies final" use case). Unmet on every + // shipping Groovy (annotation does not exist there); met on the spike. + @Test + void row01a_anchored_dispatchIsTraitAnchored() { + def r = ev ''' + import groovy.transform.Anchored + trait V { + @Anchored + static boolean defaultNullable() { false } + static boolean seen() { this.defaultNullable() } + } + class Over implements V { static boolean defaultNullable() { true } } + Over.seen() + ''' + assert r == false : "row1a @Anchored: trait body should always see the trait's own copy (got ${r})" + } + + // ---- Row 2 — same as #1 but the call is inside a closure ---- + // The closure carve-out. Helper-bound on every version (long-standing, + // not a 5/6 regression). Matches baseline B (plain class same shape — + // declarer-bound, 'base'): the carve-out aligns trait closure-context + // dispatch with plain-Groovy semantics. Removing it would extend the + // trait-machinery distinctive into closures, further from plain Groovy + // not closer — the simplified-model "Move 3" proposal was retired + // after the baseline analysis surfaced this. + @Test + void row02_publicStatic_insideClosure() { + def r = ev ''' + trait V { + static boolean defaultNullable() { false } + static boolean seenInClosure() { [1].collect { this.defaultNullable() }[0] } + } + class Over implements V { static boolean defaultNullable() { true } } + Over.seenInClosure() + ''' + assert r == false : "row2 closure: expected false on every version (Groovy ${MAJOR}), got ${r} — carve-out aligns with baseline B" + } + + // ---- Row 3 — private trait static is NOT overridable by the impl ---- + @Test + void row03_privateStatic_notOverridable() { + def r = ev ''' + trait T { + static String m1() { bar() } + private static String bar() { 't' } + } + class C implements T { static String bar() { 'c' } } + C.m1() + ''' + assert r == 't' : "row3: private trait static must stay trait-internal, got ${r}" + } + + // ---- Row 4 (dynamic) — name only on the implementer, not in T ---- + // Devil's-advocate outcome: dynamic Groovy permits this exactly like a + // missing method on any plain class. Works on 4 AND 6. + @Test + void row04_dyn_nameNotInTrait_resolvesOnImplementer() { + def r = ev ''' + trait T { + private static String entry() { m2() } // m2 not declared in T + static String run() { entry() } + } + class A implements T { static String m2() { 'A.m2' } } + A.run() + ''' + assert r == 'A.m2' : "row4 dynamic: expected runtime resolution to A.m2, got ${r}" + } + + // ---- Row 4 (@CompileStatic) — same, but must fail to compile ---- + // Ordinary @CompileStatic resolvability rule, NOT a trait-specific one. + @Test + void row04cs_compileStatic_nameNotInTrait_isCompileError() { + GroovyAssert.shouldFail { + ev ''' + @groovy.transform.CompileStatic + trait T { static String run() { m2() } } // m2 unresolved at compile time + @groovy.transform.CompileStatic + class A implements T { static String m2() { 'A' } } + A.run() + ''' + } + } + + // ---- Row 6 — instance method baseline (override always wins) ---- + // Matches baseline C (plain class instance — polymorphic, 'sub'): + // ordinary instance dispatch is polymorphic in trait code too. + @Test + void row06_instanceMethod_overrideWins() { + def r = ev ''' + trait T { String which() { 'trait' }; String greet() { which() } } + class C implements T { String which() { 'class' } } + class D implements T { } + [ c: new C().greet(), d: new D().greet() ] + ''' + assert r.c == 'class' && r.d == 'trait' + } + + // ---- Row 7 — T.m() trait-qualified inside trait body ---- + // Throws MissingMethodException on every version (the trait interface + // carries no statics — GEP-22 § Static members, item 9). The desired + // "force trait default" escape this could represent is provided by + // @Anchored via interface promotion — see row 7a. + @Test + void row07_traitQualified_observed() { + def outcome + try { + outcome = ev ''' + trait T { + static String who() { 'T' } + static String viaTrait() { T.who() } + } + class C implements T { static String who() { 'C' } } + C.viaTrait() + ''' + } catch (Throwable t) { + outcome = "THREW:${t.class.simpleName}" + } + println "row07 observed on Groovy ${MAJOR}: ${outcome} (T.m() from trait body throws on every version; @Anchored fixes it — see row 7a)" + assert outcome == 'T' || outcome.toString().startsWith('THREW') : "row7 unexpected: ${outcome}" + } Review Comment: This test currently prints the outcome and then asserts a condition that allows both a throw and a successful return value. If the intention is to lock in the documented behavior (“throws MissingMethodException”), the assertion should require that exception; otherwise the matrix won’t detect changes to this row. ########## src/test/groovy/org/codehaus/groovy/transform/traitx/TraitStaticDispatchMatrix.groovy: ########## @@ -0,0 +1,543 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Trait-static dispatch behaviour matrix — the executable companion to +// GEP-22 § Static members. Three plain-class baseline rows (A/B/C), the +// 12 design rows from GEP-22-progression-analysis.html (with row 11 +// expanded to rows 11/11c/11esc covering the trait-shadows-superclass +// quadrant + the T.super.m() escape; row 9 paired with row 9acc covering +// the static-field non-goal and its accessor workaround), and four +// @Anchored-marker rows characterising the GROOVY-12093 proposal. +// +// Runs against Groovy 4 and 6 either: +// * via gradle (`./gradlew :test --tests '*.TraitStaticDispatchMatrix'`) +// * standalone, against any SDKMAN-installed Groovy: +// `groovy src/test/groovy/.../traitx/TraitStaticDispatchMatrix.groovy` +// +// Two kinds of test: +// * Matrix tests assert OBSERVED behaviour. They pass on every +// currently-shipping Groovy that delivers the spec behaviour and turn +// RED on releases that deviate (e.g. rows 1/10/12 on Groovy 5.0.0– +// 5.0.6 / 6.0.0-alpha-1, which carry the GROOVY-8854 regression +// corrected by GROOVY-11985 in 5.0.7 / 6.0.0-alpha-2). That red is +// the intended bug-detector signal, not a defect in the test. +// * @NotYetImplemented tests assert GROOVY-12093 desired-end-state +// behaviour unmet on every currently-shipping Groovy (the four +// @Anchored variants 1a/7a/8a/13). Each references +// `groovy.transform.Anchored` by import; on any Groovy without the +// annotation the import fails and the test body throws, which +// @NotYetImplemented swallows. On a Groovy built with the spike +// (branch `spike/anchored-annotation`), the assertions hold and the +// NYI expectation flips red — the cue to drop @NotYetImplemented. +// +// Verdicts encoded here are PROPOSED PENDING TEAM REVIEW where the +// underlying design point is not yet ratified (the four @Anchored rows +// in particular). + +package org.codehaus.groovy.transform.traitx + +import groovy.test.GroovyAssert +import groovy.test.NotYetImplemented Review Comment: Unused import: NotYetImplemented is imported but not used anywhere in this test class. ########## src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java: ########## @@ -623,6 +650,98 @@ private void processField(final FieldNode field, final MethodNode initializer, f fieldHelper.addField(dummyField); } + /** + * Reports a compile error for any {@code @Anchored} annotation that is + * applied to something other than a public static non-abstract trait + * method. Without this check the misapplied annotation would be silently + * ignored, leaving the user with no signal that the marker had no effect. + */ + private void validateAnchoredAnnotations(final ClassNode traitClass) { + for (MethodNode methodNode : traitClass.getMethods()) { + List<AnnotationNode> annotations = methodNode.getAnnotations(ANCHORED_TYPE); + if (annotations.isEmpty()) continue; + String issue; + if (!methodNode.isStatic()) { + issue = "is not static"; + } else if (methodNode.isPrivate()) { + issue = "is private"; + } else if (methodNode.isAbstract()) { + issue = "is abstract"; + } else { + continue; // valid + } + AnnotationNode anchored = annotations.get(0); + sourceUnit.addError(new SyntaxException( + "@Anchored can only be applied to public static trait methods; " + + traitClass.getName() + "#" + methodNode.getName() + " " + issue, + anchored.getLineNumber(), anchored.getColumnNumber())); Review Comment: The validation error message uses only method name, which can be ambiguous for overloaded methods. Using methodNode.getTypeDescriptor() (as other trait diagnostics do) provides the full signature. ########## src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java: ########## @@ -623,6 +650,98 @@ private void processField(final FieldNode field, final MethodNode initializer, f fieldHelper.addField(dummyField); } + /** + * Reports a compile error for any {@code @Anchored} annotation that is + * applied to something other than a public static non-abstract trait + * method. Without this check the misapplied annotation would be silently + * ignored, leaving the user with no signal that the marker had no effect. + */ + private void validateAnchoredAnnotations(final ClassNode traitClass) { + for (MethodNode methodNode : traitClass.getMethods()) { + List<AnnotationNode> annotations = methodNode.getAnnotations(ANCHORED_TYPE); + if (annotations.isEmpty()) continue; + String issue; + if (!methodNode.isStatic()) { + issue = "is not static"; + } else if (methodNode.isPrivate()) { + issue = "is private"; + } else if (methodNode.isAbstract()) { + issue = "is abstract"; + } else { + continue; // valid + } + AnnotationNode anchored = annotations.get(0); + sourceUnit.addError(new SyntaxException( + "@Anchored can only be applied to public static trait methods; " + + traitClass.getName() + "#" + methodNode.getName() + " " + issue, + anchored.getLineNumber(), anchored.getColumnNumber())); + } + } + + /** + * Returns the public {@code static} trait methods whose {@code @Anchored} + * marker requests interface promotion (i.e. {@code inInterface=true}, the + * default). The returned list snapshots the trait's method set so the + * caller can iterate the methods without being affected by later + * mutations to {@code traitClass.getMethods()}. + */ + private static List<MethodNode> collectAnchoredOnInterface(final ClassNode traitClass) { + List<MethodNode> result = new ArrayList<>(); + for (MethodNode methodNode : traitClass.getMethods()) { + if (methodNode.isStatic() && !methodNode.isPrivate() && !methodNode.isAbstract() + && isAnchoredOnInterface(methodNode)) { + result.add(methodNode); + } + } + return result; + } + + /** + * Returns {@code true} if the method is annotated with {@code @Anchored} + * and the {@code inInterface} attribute is true (the default). + */ + private static boolean isAnchoredOnInterface(final MethodNode methodNode) { + List<AnnotationNode> anns = methodNode.getAnnotations(ANCHORED_TYPE); + if (anns.isEmpty()) return false; + Expression member = anns.get(0).getMember("inInterface"); + if (member instanceof ConstantExpression + && Boolean.FALSE.equals(((ConstantExpression) member).getValue())) { + return false; + } + return true; + } + + /** + * Builds a public-static method on the trait interface that delegates to + * the corresponding helper method. + * + * <p>Emits {@code public static R m(args) { return T$Trait$Helper.m(T.class, args); }}, + * preserving generics, exceptions and parameter list of the original + * trait static. The trait class itself is passed as the synthetic + * {@code $self} receiver expected by the helper, consistent with the + * declarer-bound dispatch model that {@code @Anchored} selects. + */ + private static MethodNode createAnchoredInterfaceForwarder(final ClassNode traitClass, final ClassNode helper, final MethodNode original) { + Parameter[] params = original.getParameters(); + Expression[] callArgs = new Expression[params.length + 1]; + callArgs[0] = classX(traitClass); + for (int i = 0; i < params.length; i++) { + callArgs[i + 1] = varX(params[i]); + } + MethodCallExpression call = callX(classX(helper), original.getName(), args(callArgs)); + Statement body = VOID_TYPE.equals(original.getReturnType()) ? stmt(call) : returnS(call); + MethodNode forwarder = new MethodNode( + original.getName(), + ACC_PUBLIC | ACC_STATIC, + original.getReturnType(), + params, + original.getExceptions(), + body); + forwarder.setGenericsTypes(original.getGenericsTypes()); + forwarder.setSynthetic(true); + return forwarder; Review Comment: The generated interface forwarder is a new public API surface for `@Anchored` methods. It currently drops method annotations/source position from the original trait method, which can lose important metadata (e.g. @Deprecated, @Incubating) and makes diagnostics point at synthetic nodes. -- 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]
