Hold the stability tiers with a gate instead of a script in a private directory - #15
Merged
Merged
Conversation
… directory Phase 23 split the Kotlin API into a stable core and an explicit `@RaboshExperimental` tier, and gave the published modules a module-wide opt-in so the engine's use of its own internals did not need several hundred `@OptIn`s. That opt-in has a consequence which was noticed then and left open: the compiler will not report a public signature that *names* an experimental type without carrying the marker. Inside the library every such use is permitted, so a consumer meets the signature and is handed an experimental type with nothing having asked them to opt in. An ABI dump cannot see it either — the JVM format writes signatures and never annotations. The gap is not theoretical. Run by hand in phase 23 it found four leaks, and phase 24 then added public surface to every module in the chain with nothing but an awk script in a private directory standing behind the claim. `ApiTierAudit` is that script, promoted: plain Kotlin over `File` beside `PublishedModules` and `CentralBundleReport`, with fifteen unit tests, and a root `checkApiTiers` task hung off `check` so `./gradlew build` runs it and CI needs no new step. Root rather than per-module because the leak is cross-module — a type marked in `rabosh-index` leaks through a signature in `rabosh-query`'s dump. Both halves are derived and neither is listed. The marker set comes from the sources, the surface from the committed dumps. A hand-maintained list of experimental types in a build script would be a second list free to disagree with the annotations, and it would disagree exactly once, silently, in the direction of not reporting a leak. That is the `PublishedModules` rule applied to a different question, and it is why the awk script is deleted rather than kept beside this one. Three decisions inside it. An annotation the scanner cannot attribute is a failure and never a shorter set, because under-reporting is the only failure mode that matters here: an audit that misses a leak passes, and passing is what it is read for. Nesting follows indentation rather than counting braces, because Kotlin string templates put braces inside string literals and a counter needs a lexer to be right. And a missing dump is skipped rather than failed, because `checkKotlinAbi` already owns that. Promoting it found a bug the hand-written version never had to have. The awk script carried a hard-coded type list, so it never had to work out which type a marked *member* belonged to; the derived version does, and its first attempt took the most recent type declaration above the annotation — which put `IndexCatalog.read` inside a `private class` declared two hundred lines earlier and reported a correctly marked method as a leak. A false positive is what gets a gate switched off, so the sibling case is a test rather than an assumption, and the comparison is now strictly less than the declaration's column. Verified by breaking it: removing the marker from `IndexCatalog.read` fails `checkApiTiers` naming the method, the type it exposes and the module. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #13 and #14, which both recorded this as their weakest link. It was going to ride along
on #14 but that merged first, so here it is on its own.
The gap
#13 split the Kotlin API into a stable core and an explicit
@RaboshExperimentaltier, and gave thepublished modules a module-wide opt-in so the engine's use of its own internals did not need
several hundred
@OptIns. That opt-in has a consequence:The compiler will not report a public signature that names an experimental type without carrying
the marker. Inside the library every such use is permitted. A consumer meeting the signature is
handed an experimental type with nothing having asked them to opt in, and
STABILITY.md's claimquietly stops being true.
checkKotlinAbicannot see it either — the JVM dump format writessignatures and never annotations.
Not theoretical: run by hand in #13 it found four leaks (
IndexCatalog.read,IndexCatalog.readColumn,SchemaCatalog.sketchOf,IndexQuery). #14 then added public surface toevery module in the chain with nothing but an awk script in a private directory standing behind the
claim.
What this is
ApiTierAudit— plain Kotlin overFilebesidePublishedModules,CentralBundleReportandBenchmarkRunReport, with 15 unit tests. Arabosh.api-tiersplugin applied at the rootregisters
checkApiTiersand hangs it off the rootcheck, so./gradlew buildruns it and CI needsno new step. Root rather than per-module because the leak is cross-module: a type marked in
rabosh-indexleaks through a signature inrabosh-query's dump.Both halves are derived and neither is listed. The marker set comes from the sources, the
surface from the committed dumps. A hand-maintained list of experimental types in a build script
would be a second list free to disagree with the annotations, and it would disagree exactly once,
silently, in the direction of not reporting a leak. That is the
PublishedModulesrule applied to adifferent question — and it is why the awk script is deleted rather than kept beside this one.
Three decisions inside it
is the only failure mode that matters: an audit that misses a leak passes, and passing is what it
is read for.
markedInthrows, naming the file and line.{and}insidestring literals, so a counter needs a lexer to be right and is wrong in a way nobody notices until
it mis-attributes one member.
checkKotlinAbiowns that, and owning it twicemeans two messages for one problem.
What promoting it found
A bug the hand-written version never had to have, which is the argument for promoting a script
rather than trusting one.
The awk script carried a hard-coded type list, so it never had to work out which type a marked
member belonged to. The derived version does, and its first attempt took the most recent type
declaration above the annotation — putting
IndexCatalog.readinside aprivate classdeclared twohundred lines earlier, and reporting a correctly marked method as a leak. A false positive is what
gets a gate switched off, so the comparison is now strictly less than the declaration's column and the
sibling case is a test of its own.
Two shapes the awk version also never handled, both now pinned: a class header spells a supertype as
a bare binary name with none of the
L…;a signature has; and the dump and the sources disagree onthree spellings —
getStore/store,<init>/constructor,read$default/read— each of whichwould report a marked declaration as a leak if got wrong.
Verification
./gradlew -p build-logic check(15 new tests),./gradlew build --rerun-tasksgreen on top ofmerged
main.@RaboshExperimentalfromIndexCatalog.readcheckApiTiersfails, naming the method,IndexReaderandrabosh-indexmarkedInthrows rather than returning a short setOne limit, stated rather than assumed
The audit checks what a dump can express.
List<Bitmap>erases toLjava/util/List;, so anexperimental type reachable only as a type argument is invisible to it. Nothing in the current surface
has that shape; if one is ever added the marker goes on by hand, and the phase note records this as
the reason to remember why.
🤖 Generated with Claude Code