[daniellansun]

https://issues.apache.org/jira/browse/GROOVY-12726

• Replace or remove CONVERSION-phase stubber placeholder methods in @tostring, @EqualsAndHashCode, @IndexedProperty, @bindable, and @Vetoable when custom helper methods (_toString, _hashCode, _equals) or @Immutable/@ImmutableBase cause the CANONICALIZATION transform to take an alternate branch or skip generating mutator methods.
• Extend BindableASTStubber and VetoableASTStubber to also emit listener registration stubs when @Bindable/@Vetoable is applied at the field level.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.26214% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.5400%. Comparing base (856b28a) to head (71a0f0a).
⚠️ Report is 63 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/groovy/beans/BindableASTTransformation.java	87.5000%	0 Missing and 3 partials ⚠️
...n/java/groovy/beans/VetoableASTTransformation.java	90.0000%	0 Missing and 3 partials ⚠️
...us/groovy/transform/IndexedPropertyASTStubber.java	0.0000%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #3262        +/-   ##
==================================================
+ Coverage     74.4429%   74.5400%   +0.0971%     
- Complexity      44774      45018       +244     
==================================================
  Files            1702       1703         +1     
  Lines          151891     152380       +489     
  Branches        29091      29189        +98     
==================================================
+ Hits           113072     113584       +512     
+ Misses          28413      28400        -13     
+ Partials        10406      10396        -10     

Files with missing lines	Coverage Δ
src/main/java/groovy/beans/BindableASTStubber.java	100.0000% <100.0000%> (+25.0000%)	⬆️
src/main/java/groovy/beans/VetoableASTStubber.java	100.0000% <100.0000%> (+24.3243%)	⬆️
.../transform/EqualsAndHashCodeASTTransformation.java	84.6154% <100.0000%> (+2.6154%)	⬆️
...vy/transform/IndexedPropertyASTTransformation.java	100.0000% <100.0000%> (+16.3934%)	⬆️
...us/groovy/transform/ToStringASTTransformation.java	85.9060% <ø> (+1.2394%)	⬆️
...n/java/groovy/beans/BindableASTTransformation.java	93.2836% <87.5000%> (+0.3702%)	⬆️
...n/java/groovy/beans/VetoableASTTransformation.java	93.2432% <90.0000%> (+0.5425%)	⬆️
...us/groovy/transform/IndexedPropertyASTStubber.java	87.5000% <0.0000%> (+26.9737%)	⬆️

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

[github-actions]

JMH summary — classic (commit f5a3936)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group	Speedup	Calibrated	n
bench	1.117 ×	1.012 ×	134
core	0.951 ×	1.068 ×	108
grails	1.447 ×	1.324 ×	80

No benchmark is ≥1.5× slower than its 90-day baseline.

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: grails-ad, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.

_{Runner calibration (this run vs baseline hardware): bench 1.11× (27 rulers) · core-ag 0.88× (3 rulers) · core-hz 0.90× (3 rulers) · grails-ad 0.86× (3 rulers) · grails-ez 1.33× (3 rulers)}

_{Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data}

[github-actions]

JMH summary — indy (commit f5a3936)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group	Speedup	Calibrated	n
bench	0.895 ×	0.989 ×	134
core	2.288 ×	2.080 ×	108
grails	0.950 ×	1.187 ×	80

No benchmark is ≥1.5× slower than its 90-day baseline.

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-ag, grails-ad, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.

_{Runner calibration (this run vs baseline hardware): bench 0.90× (27 rulers) · core-ag 1.25× (3 rulers) · core-hz 0.88× (3 rulers) · grails-ad 0.74× (3 rulers) · grails-ez 0.85× (3 rulers)}

_{Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data}

[paulk-asert]

Thanks, this is a good catch. I reproduced the cases on master with groovyc -j and here's where I landed.

Confirmed and important (item 1). In joint compilation, a class with @ToString plus a _toString() helper ends up with toString() returning ''; likewise hashCode() returns 0 and equals is always false (new B(v:1) == new B(v:1) is false). Plain compilation is unaffected. Moving the isStub check ahead of the helper check is the right fix, and I traced the remaining combinations (user-written method with/without helper, generated non-stub method) — behaviour is unchanged apart from the placeholder now being completed. Since the stubbers shipped in 6.0.0 and GROOVY_6_0_X has the same code, this should be backported.

Confirmed (item 4). A Java class calling addPropertyChangeListener on a Groovy class that only uses field-level @Bindable fails with cannot find symbol on master. The new classReceivingListenerStubs rules (non-static, non-final, a property) match what the full transform accepts, the duplicate guards cover multiple annotated fields, and every field-level path (including @Bindable + @Vetoable via VetoableASTTransformation.createListenerSetter) reaches the placeholder removal. Item 3 is what makes this safe.

Item 2 (immutable indexed setter). I couldn't reproduce this end to end: with the exact @MyImmutable(defaults = false, propertyHandler = ImmutablePropertyHandler) setup from IndexedPropertyJointCompilationStubTest, master has no setItems(int, String) in either the stub or the class file. The removal is harmless and reasonable as a safeguard, but the JIRA description should say it was only shown with the hand-built AST in IndexedPropertyStubHandoffTest.

Records. While checking the @RecordType/@RecordBase change I found that the compiled record does get setItems(int, V) from @IndexedProperty, in plain compilation as well as joint — so as it stands the stub and the class file disagree. That's a pre-existing bug, now GROOVY-12730 (fix for 7.0, since it removes a public method). With 12730 in place the stubber change here is consistent, so I'd keep it. Could you add a class-file assertion for RecordBox/BaseBox alongside the stub assertion (as the test already does for ImmutableBox) once 12730 lands? For the 6.0.x backport, the record part can stay: the stub would offer one method fewer than the class file has, and Java code can't call a method it can't see, so nothing breaks at runtime.

A few smaller things:

1. The Bindable/Vetoable stubber Javadoc now says "Unrelated stubber placeholders on the same class are left in place", but addListenerToClass (class-level path) still removes every stub-tagged method. That's harmless today — completed placeholders are untagged via clearStub, and the other transforms regenerate anything missing — but either narrow that removal to the listener placeholders too, or adjust the sentence.
2. The placeholder removal now lives inside protected needsPropertyChangeSupport / needsVetoableChangeSupport. A subclass overriding either would silently skip it, and a "needs…" query changing the class is surprising. Calling the removal from the callers would keep those methods side-effect free.
3. The literal-to-constant changes in Bindable/Vetoable (~40 lines) and the IndexedPropertyASTTransformation restructuring make the fix harder to review and to backport. If they're there for Sonar, could they go in a separate commit?
4. Please squash to a single commit before merge. On the JIRA, Affects Version 6.0.0, Fix Versions 6.0.x and 7.0.0, and a component would help.

With those addressed I'm +1 to merge to master and backport to 6.0.x.

[blackdrag]

I have nothing to add to what Paul said

[daniellansun]

Thanks Paul — the smaller items are now on the tip:

1. Class-level Bindable/Vetoable stubber removal is narrowed to the listener placeholders, in line with the field-level path and the Javadoc. Class-level @Vetoable no longer drops @Bindable's property-change placeholders.
2. Placeholder removal is no longer inside needsPropertyChangeSupport / needsVetoableChangeSupport. Those methods stay side-effect free, and the callers invoke removal explicitly.
3. The literal-to-constant Bindable/Vetoable changes and the IndexedPropertyASTTransformation restructuring are in a separate commit from the functional stubber fix.
4. I'll squash to a single commit before merge, and I'll update the JIRA with Affects Version 6.0.0, Fix Versions 6.0.x and 7.0.0, a component, and a note that the immutable indexed-setter case was only shown with the hand-built AST in IndexedPropertyStubHandoffTest.

Once GROOVY-12730 lands I'll add the RecordBox/BaseBox class-file assertion alongside the stub check. Agreed on the 6.0.x backport, including the record stub part as you described. I'll also rebase onto current master.

Jochen, thanks — agreed with Paul on the above.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
94.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 71a0f0a
▶️ Tests: 132044 executed
⚪️ Checks: 26/26 completed

---

Learn more about TestLens at testlens.app/docs.