GroovyABI annotation to mark backwards compatibility

[blackdrag]

mainly methods as used through the compiler and have special requirements for backwards compatibility as they are used by the compiler

I put this to discussion if we want to have this annotation. I think it would be useful, especially to see the actual "surface" we exhibit through compilation in methods, that are not in groovy.*

This is btw probably not complete. It was a lot of work to find all those cases and their respective version.

@paulk-asert and @daniellansun and @eric-milles would be nice to hear your opinion

[blackdrag]

Looks like I accidentally added the annotation to some groovy.* classes. Please ignore those I am going to remove their annotation before merge (if we merge)

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: ce12a74
▶️ Tests: 108786 executed
⚪️ Checks: 23/23 completed

---

Learn more about TestLens at testlens.app.

[paulk-asert]

I haven't done a proper review yet but here is an early AI read:

Overall I'm in favour — this encodes knowledge that currently lives only in our heads, and it's knowledge no tool can derive after the fact. I did a deep dive comparing it against our existing checkBinaryCompatibility task to see whether they overlap, and I'm convinced they're complementary: japicmp tells us what changed across the whole protected+ surface vs one baseline, but it can't distinguish "internal wiring, safe to change" from "referenced by bytecode an older compiler emitted". The Groovy 6 report makes this vivid: it flags ~850 binary-incompatible items (mostly the callsite extraction and Grape split), and deciding which of those actually break the compiled-code contract required exactly the human knowledge this annotation captures. The since value also covers something a single-baseline compare never can: how far back the promise extends (2.5-compiled bytecode running on 5/6 spans many baselines).

A few suggestions:

1. @Retention(CLASS) instead of SOURCE. This is my main request. With SOURCE retention the annotation is documentation only — invisible to japicmp, ArchUnit-style tests, or any bytecode scanner. With CLASS retention (no runtime loading cost, a few bytes per member) we can wire it straight into the build: the japicmp plugin we already use (0.4.6) supports annotationIncludes, so we could add a second, strict task variant per module — annotationIncludes = ['@org.apache.groovy.lang.annotation.GroovyABI'], failOnModification = true — making the annotated subset an enforced CI gate while the broad report stays advisory. It would also finally give the long-standing richReport TODO in subprojects/binary-compatibility/build.gradle its custom rule.

2. Prior art supports the combination. Gradle's own build does exactly this: japicmp plus annotation-driven rules (@Incubating, @Deprecated) plus an accepted-public-api-changes.json escape hatch for intentional breaks. Most tellingly, they have @UsedByScanPlugin — an annotation marking internal code that external bytecode links against — which is the direct analogue of what you're proposing.

3. Completeness could be attacked mechanically. Rather than hand-auditing (which you rightly note was a lot of work), we could compile a corpus (our own test suite, indy on and off), harvest all INVOKE*/GETSTATIC targets landing in org.codehaus.*/org.apache.groovy.*, and diff against the annotated set. That could even become a build-time check so the set can't silently rot.

4. Lifecycle of a dropped guarantee. Worth deciding now: when we intentionally drop an ABI member at a major version, do we just delete the annotation/member, or record it (a removedIn attribute, or an acceptance file à la Gradle)? Related: the since archaeology is only worth polishing if we also document the support-window policy it implies (e.g. "kept for N major versions after since") — otherwise a plain marker would serve.

[daniellansun]

Just a few thoughts after reading through the change. The idea makes a lot of sense to me — I only have some questions about how we keep the marker consistent and easy to maintain.

---

What works well

The problem is real: things the compiler (or transforms) call directly in the runtime are a compatibility surface that’s easy to miss. Classic call sites, ScriptBytecodeAdapter, indy bootstraps, transform helpers, GINQ/contracts/macro runtimes — all of that can outlive the compiler that produced the bytecode.

A few things in this commit already feel right:

• Putting @GroovyABI on the ScriptBytecodeAdapter class, then overriding only the newer methods (compoundAssign, packedClosure), is a nice pattern.
• Touching the subprojects (callsite, contracts, ginq, macro) and not only groovy-core matches where those calls actually land.
• Marking CallSite / CallSiteArray fits what we already say in COMPATIBILITY.md about classic linkage for Groovy 4/5 bytecode on 6.

---

Annotation shape and docs

Retention

@Retention(SOURCE)
public @interface GroovyABI {
    String since() default "";
}

With SOURCE retention the marker never makes it into the published jars, so japicmp (and anything else that only sees class files) can’t use it. That might be exactly what you want if this is mainly for people reading the source — if so, saying that somewhere would help.

If we later want a small check like “don’t drop a compiler-referenced member by accident,” CLASS or RUNTIME would make that easier (closer to how @Incubating works).

It might also be worth a short note in COMPATIBILITY.md so @GroovyABI sits next to Public API / @Internal / the binary-compat check, rather than living only in the annotation Javadoc.

since

A few small inconsistencies stood out:

• Five places on InvokerHelper use since="?".
• Version strings mix 1.0 / 1.0.0 and 2.0 / 2.0.0.
• Spacing varies: since = "1.6.0" vs since="4.0.0".
• The empty default (default "") leaves it unclear when since is required.

I’d lean toward one format (x.y.z), no ? in the tree (either fill it in or leave since off until we know), and a short rule for how this relates to existing @since in Javadoc when both are present.

Policy vs what’s annotated

The Javadoc says groovy.* isn’t annotated because it’s already public API, but we do annotate members on Closure, Reference, and Awaitable.

Either story is fine:

• leave groovy.* alone and reserve @GroovyABI for internal/runtime types the compiler still hard-wires, or
• use @GroovyABI even on public types when the compiler depends on that specific member.

I’d just pick one and make the text match the tree. Small Javadoc nits while we’re there: the notes use bare <li> without a list wrapper, and a @since on the annotation type itself would match the usual style.

---

Class-level vs every method

We already allow TYPE, and ScriptBytecodeAdapter uses that well. Elsewhere we stamp the same since on every overload:

Area	What happens
Maps.of(...)	~100 identical annotations on a huge file
CallSite	every call* / callSafe* / … overload
HashCodeHelper.updateHash	every overload
DTT box/unbox	every method; the class itself isn’t marked

Would it make sense to default to the type, and only mark members when since differs or when only part of the type is ABI? That would cut a lot of noise without losing the refinement pattern you already use on ScriptBytecodeAdapter. Maps is the clearest case — the per-overload marks don’t really say more than one class-level annotation would.

---

How complete does the inventory need to be?

A partial list is still useful as a start. The risk is that people read “no annotation” as “safe to change.” A few busy places show the gap:

Surface	Role	Currently
ScriptBytecodeAdapter	main classic bridge	class-level — good
DefaultTypeTransformation	box/unbox and casts (castToType, etc.)	box/unbox only
NumberMath	arithmetic / compare as well as bitwise	mostly bitwise / shift / mod
InvokerHelper	property / invoke helpers	a subset; several with ?
CallSiteArray	classic linkage (ctor + array / owner)	constructor only

Also, two annotations sit on private methods, which outside bytecode can’t call:

• AssertionRenderer.render() (private) — the public static render(String, ValueRecorder) is probably what we mean
• FieldValues.findField(...) (private helper)

Those look accidental; moving them to the public entry points (or dropping them) would keep the meaning clear.

Looking a bit further ahead

Keeping hundreds of hand-placed marks up to date will be hard. A lot of this surface is already “registered” in the compiler via MethodCaller / multi-adapters, indy bootstraps, and transform-generated calls. Even a simple test that checks known registration points against @GroovyABI would help later. No need to build that now — just something to keep in mind so the first cut doesn’t fight that idea.

For v1, two shapes both seem reasonable:

1. Narrow but complete — e.g. call sites + ScriptBytecodeAdapter + indy bootstraps, fully covered; or
2. Wide pass like now, but with type-level defaults, no ?, no private members, and a clear rule for what belongs (always-emitted bridges vs occasional DGM / static-compile paths).

Right now a few DGM / StringGroovyMethods / NumberMath bits sit next to the core bridges without saying why those and not others. One sentence of guidance would go a long way so we don’t slowly mark half the GDK.

---

A few edits that aren’t about @GroovyABI

These are easy to miss in the same commit:

File	Change
BinaryExpressionHelper	drops an unused assignment after boxing
ClosureWriter	unused callX import
StaticTypesCallSiteWriter	whitespace on "or", and drops case "implies":

The first two are harmless. The implies case changes STC dispatch for number×number implies (it no longer takes the optimized path). Even if that path is rare, it might be clearer in its own change — or with a short note and a test if we keep it here. Splitting pure inventory from compiler behaviour also makes history and reverts easier.

---

Optional next steps

Nothing urgent — more a suggested order if we iterate:

1. Spell out the contract briefly in COMPATIBILITY.md (what it means, class vs member, version format, how it relates to public API).
2. Tighten the annotation (retention, since rules, Javadoc, groovy.* wording).
3. Prefer type-level marks when everything shares the same since.
4. Clean the inventory: resolve/drop ?, fix private members, either finish the core bridges or document an explicit v1 subset.
5. Split (or call out) the unrelated compiler cleanups and the implies change.
6. Later: a lightweight check against known compiler registration points.

---

Closing

I like the direction. Making the compiler↔runtime surface visible will help future work. The main things I’d still like to settle are: who the annotation is for (humans only vs tooling), leaning on type-level marks so the inventory stays readable, and making the marked set either complete for a stated scope or clearly “work in progress,” so it stays a trustworthy signal.

[blackdrag]

I agree with most what you both said, but since this started to be a tiring piece of work (5h of finding methods initial commits, compare with tags and so on) I wanted first to get some feedback before proceeding.

Annotation shape and docs

Retention

[...}

With SOURCE retention the marker never makes it into the published jars, so japicmp (and anything else that only sees class files) can’t use it. That might be exactly what you want if this is mainly for people reading the source — if so, saying that somewhere would help.

If we later want a small check like “don’t drop a compiler-referenced member by accident,” CLASS or RUNTIME would make that easier (closer to how @Incubating works).

It might also be worth a short note in COMPATIBILITY.md so @GroovyABI sits next to Public API / @Internal / the binary-compat check, rather than living only in the annotation Javadoc.

agreed will do. The primary purpose was information for the developer, not for a tool, that is why I did not think of that.

since

A few small inconsistencies stood out:

* Five places on `InvokerHelper` use `since="?"`.

yeah, I have to investigate further on those.

* Version strings mix `1.0` / `1.0.0` and `2.0` / `2.0.0`.

the longer version should be always used, will change that.

* Spacing varies: `since = "1.6.0"` vs `since="4.0.0"`.

ah well... ok

* The empty default (`default ""`) leaves it unclear when `since` is required.

I had to first to work a bit with it and then let it settle. I think now since should be mandatory.

[...]

Policy vs what’s annotated

The Javadoc says groovy.* isn’t annotated because it’s already public API, but we do annotate members on Closure, Reference, and Awaitable.

yeah, those have to go.

[...]

Would it make sense to default to the type, and only mark members when since differs or when only part of the type is ABI? That would cut a lot of noise without losing the refinement pattern you already use on ScriptBytecodeAdapter. Maps is the clearest case — the per-overload marks don’t really say more than one class-level annotation would.

if it is every public method in that file, then we can mark it on type level using the earliest version and if there are later introduction we can use the later version directly on those. But what if we have mixed in public methods that are not used from bytecode? An internal marked class with bytecode ABI markers on some of the public methods is still allowed to change those public methods.

How complete does the inventory need to be?

A partial list is still useful as a start. The risk is that people read “no annotation” as “safe to change.” A few busy places show the gap:
Surface Role Currently
ScriptBytecodeAdapter main classic bridge class-level — good
DefaultTypeTransformation box/unbox and casts (castToType, etc.) box/unbox only
NumberMath arithmetic / compare as well as bitwise mostly bitwise / shift / mod
InvokerHelper property / invoke helpers a subset; several with ?
CallSiteArray classic linkage (ctor + array / owner) constructor only

I think castToType for example shows the problem. I did not mark it, because I did not verify its usage. I did not work through InvocationWriter for example. And while that uses castToType, it is from SBC. I did another search and could not find a usage by the compiler. So maybe it was in the past? Tranditionally there is SBC.castToType that uses that now. So most likely the method was never used directly by compiler produced bytecode. But can I be sure? No. It means digging deep into the commit history and trying to find the usage on an checkout of an old version most likely. The probability is high though it was never used from the compiler directly. Thus the annotations go on the box/unbox methods only, none on the class level.

[...]

Looking a bit further ahead

Keeping hundreds of hand-placed marks up to date will be hard. A lot of this surface is already “registered” in the compiler via MethodCaller / multi-adapters, indy bootstraps, and transform-generated calls. Even a simple test that checks known registration points against @GroovyABI would help later. No need to build that now — just something to keep in mind so the first cut doesn’t fight that idea.

A once placed marker does not need an update. The MethodCaller/multi-adapters help, but what if we decide to use a different method in the future? The old entry point is potentially lost and forgotten, then maybe removed and we broke older programs. Extracting StringDGM for example broke older programs because some of the string methods in there where used directly. That did happen in 3.0.0, and it took till 3.0.24 I think before we fixed it. transform generated did proof to be more difficult actually. Only following the callX usages did help, but sometimes the call is abstracted through several levels and it gets difficult fast. indy bootstraps are another good example. We broke that before. But it is only the bootstrap method we have to consider here, not methods we produce method handles for. Unless the handle is stored in the class bytecode, then we have to deal with handles as well. So far the bootstrap method works like an isolation layer here. Adding new bootstrap methods like some of the Groovy 6 methods are a different story. There are also for example for methods used by runtime generated classes. These are also not to be considered.

[...]

Right now a few DGM / StringGroovyMethods / NumberMath bits sit next to the core bridges without saying why those and not others. One sentence of guidance would go a long way so we don’t slowly mark half the GDK.

I tried to give those in the javadoc of the annotation, maybe needs to be improved.

A few edits that aren’t about @GroovyABI

These are easy to miss in the same commit:
File Change
BinaryExpressionHelper drops an unused assignment after boxing
ClosureWriter unused callX import
StaticTypesCallSiteWriter whitespace on "or", and drops case "implies":

The first two are harmless. The implies case changes STC dispatch for number×number implies (it no longer takes the optimized path). Even if that path is rare, it might be clearer in its own change — or with a short note and a test if we keep it here. Splitting pure inventory from compiler behaviour also makes history and reverts easier.

The implies drop should maybe be extracted. But fact is we do not compile 1==>1 to 1.implies(1). We compile that by converting the numbers to boolean (in a quite inefficient way btw.) and then handle it as boolean intrinsic. The path I removed is never visited. In other words the optimization was never in BinaryExpressionHelper. If I where wrong, there should have been failing tests. Though I have the feeling we lack tests with implies and static compilation and with anything but booleans as well.

Optional next steps

Nothing urgent — more a suggested order if we iterate:

1. Spell out the contract briefly in `COMPATIBILITY.md` (what it means, class vs member, version format, how it relates to public API).
2. Tighten the annotation (retention, `since` rules, Javadoc, `groovy.*` wording).
3. Prefer type-level marks when everything shares the same `since`.
4. Clean the inventory: resolve/drop `?`, fix private members, either finish the core bridges or document an explicit v1 subset.
5. Split (or call out) the unrelated compiler cleanups and the `implies` change.
6. Later: a lightweight check against known compiler registration points.

agreed I guess. Since the idea was well received so far I also have to add a JIRA issue.