diff --git a/.claude/rules/build-and-release.md b/.claude/rules/build-and-release.md index e40a390..800a207 100644 --- a/.claude/rules/build-and-release.md +++ b/.claude/rules/build-and-release.md @@ -115,13 +115,33 @@ experimental type to be marked too, and that cascade ends with the stable core i tier. `SegmentObserver` is that cascade caught at one step: `RaboshOptions`' constructor names it, so marking the interface would have put `RaboshOptions(...)` behind an opt-in. It is stable, deliberately. -**The gate is `rabosh-samples` not opting in, and the ABI dumps are not the gate.** The JVM dump -format writes signature lines and never annotations — verified in the dumper, and confirmed by the -markers changing the committed dumps by exactly one entry, the annotation class itself — so a -declaration changing tier is invisible to `checkKotlinAbi`. What catches it is the samples module: -`:rabosh-api` and nothing else, `allWarningsAsErrors`, part of `build`, and the one module the -opt-in is deliberately withheld from. Do not "tidy" that asymmetry by giving every module the same -compiler options; `rabosh.kotlin-library`, `rabosh-testkit` and `rabosh-bench` opt in, samples do not. +**Two gates, and neither is `checkKotlinAbi`.** The JVM dump format writes signature lines and never +annotations — verified in the dumper, and confirmed by the markers changing the committed dumps by +exactly one entry, the annotation class itself — so a declaration changing tier is invisible to it. + +The first gate is **`rabosh-samples` not opting in**: `:rabosh-api` and nothing else, +`allWarningsAsErrors`, part of `build`, and the one module the opt-in is deliberately withheld from. +Do not "tidy" that asymmetry by giving every module the same compiler options; +`rabosh.kotlin-library`, `rabosh-testkit` and `rabosh-bench` opt in, samples do not. + +The second is **`checkApiTiers`**, and it exists because the first one only sees what a sample +happens to call. Module-wide opt-in blinds the compiler to a public signature that *names* an +experimental type without carrying the marker — a consumer meeting it gets handed an experimental +type with nothing having asked them to opt in. `ApiTierAudit` reads the marker set from the +**sources** and the surface from the **committed dumps**, both derived and neither listed, for the +`PublishedModules` reason: a hand-maintained list of experimental types would disagree with the +annotations exactly once, silently, in the direction of not reporting a leak. It is a root task, +because the leak is cross-module — a type marked in `rabosh-index` leaks through a signature in +`rabosh-query`'s dump — and it hangs off the root `check`, so `./gradlew build` runs it. + +Three things about it that are decisions. **An annotation the scanner cannot attribute is a failure, +not a shorter set**: under-reporting is the only failure mode that matters, because an audit that +misses a leak passes and passing is what it is read for. **Nesting is followed by indentation rather +than by counting braces**, because Kotlin string templates put braces inside string literals and a +counter needs a lexer to be right; the comparison is *strictly* less than the declaration's column, so +a sibling `private class` declared beside a marked function is not mistaken for its enclosing scope — +which the first version did, reporting `IndexCatalog.read` as a leak. And **a missing dump is skipped +rather than failed**, because `checkKotlinAbi` already owns that and owning it twice is worse. ## Native access: the flag nobody needs diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c98a8..afd343a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,23 @@ else may change in any release. That claim lives in [STABILITY.md](STABILITY.md) ### Added +- **`checkApiTiers` — the stability tiers are now held by a gate rather than by a script.** The + module-wide opt-in that keeps the engine from needing several hundred `@OptIn`s also blinds the + compiler to a public signature that *names* an experimental type without carrying the marker, and + an ABI dump writes signatures and never annotations — so between them nothing was checking that + `STABILITY.md`'s claim was still true. Run by hand it had already found four such leaks. + + `ApiTierAudit` reads the marker set from the **sources** and the surface from the **committed + dumps**, both derived and neither listed: a hand-maintained list of experimental types would + disagree with the annotations exactly once, silently, in the direction of not reporting a leak. It + is a root task, because a type marked in one module leaks through another module's dump, and it + hangs off `check`, so `./gradlew build` runs it. + + Promoting it found a bug the hand-written version never had to have: attributing a marked member to + the *most recent* type declaration rather than the enclosing one put `IndexCatalog.read` inside a + `private class` two hundred lines above it. Nesting now follows indentation, and the sibling case is + a test. + - **`Rabosh.checkpoint(target)` — a consistent copy, taken while you are writing.** The recipe it replaces was *stop writing and copy the directory*, which a desktop application cannot do because it is the writer. The database is flushed, a snapshot is pinned, and the copy is of what that snapshot diff --git a/CLAUDE.md b/CLAUDE.md index b97e9da..a9d5fe2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,10 @@ them at the latest stable release; do not adopt pre-releases (e.g. Kotlin `-Beta dumps live at `/api/.api`. - **`build-logic/` is an included build, and its tests are not part of the root `build`** — CI runs `./gradlew -p build-logic check` as its own step. +- **`checkApiTiers` is the gate `checkKotlinAbi` cannot be**: a dump carries signatures and never + annotations, and module-wide opt-in blinds the compiler, so a public signature exposing a + `@RaboshExperimental` type without carrying the marker is invisible to both. It is a root task and + runs under `build`. - **`gradle.properties` stays `0.1.0-SNAPSHOT`**: `release.yml` derives the release version from the git tag and nowhere else. Do not "fix" it to a release number. - **The format claim lives in `COMPATIBILITY.md` and the API claim in `STABILITY.md`, each in one diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 88efe68..509e25f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,6 +53,13 @@ and commit the result. The tasks are `checkKotlinAbi` and `updateKotlinAbi`; the that Kotlin still registers are deprecated shims and using them is a build that breaks on the next upgrade for no benefit today. +**A dump says nothing about stability tiers**, so a second check runs beside it. `checkApiTiers` — +also part of `build` — fails when a public signature exposes a `@RaboshExperimental` type without +carrying the marker itself. If it names your declaration, either mark it or move the type into the +stable core and say so in [STABILITY.md](STABILITY.md); the one thing not to do is leave a consumer +holding an experimental type that nothing asked them to opt in to. It reads the marker set from the +sources, so adding a marker is all that is needed to teach it. + **2. A new dependency needs agreement first, and the runtime scope is closed.** "No runtime dependencies at all" is a claim the README makes, so it has to stay true — the JSON parser, the compressed bitmap, the HyperLogLog, the bloom filter and the property-test harness are all in-repo diff --git a/README.md b/README.md index f4858fd..aa4d9f1 100644 --- a/README.md +++ b/README.md @@ -636,6 +636,7 @@ Requires JDK 25. ./gradlew build # compile, test, and check the public ABI against the committed dumps ./gradlew test # tests only ./gradlew updateKotlinAbi # after an intentional public API change +./gradlew checkApiTiers # no unmarked signature exposes an experimental type (part of build) ./gradlew dokkaGenerate # the aggregated API site, into build/dokka/html ``` diff --git a/STABILITY.md b/STABILITY.md index f3aedf4..bd3fb81 100644 --- a/STABILITY.md +++ b/STABILITY.md @@ -122,16 +122,25 @@ they do **not** carry is the markers: the JVM dump format writes signature lines annotations, and the synthetic method Kotlin emits for an annotated property is filtered out as synthetic. A declaration changing tier is invisible to it. -**`rabosh-samples` is what holds the tiers.** It depends on `:rabosh-api` and nothing else, it +**`rabosh-samples` is what holds the stable core.** It depends on `:rabosh-api` and nothing else, it compiles with `allWarningsAsErrors`, it is part of `./gradlew build`, and — unlike every other module in the repository — it deliberately does **not** opt in to `@RaboshExperimental`. It is therefore a real consumer compiling against the stable core with no opt-in. A stable declaration that silently acquires the marker fails there, and so does a sample that reaches past the facade. That asymmetry is load-bearing and should not be tidied away by giving every module the same build configuration. -Verified by breaking it, which is this repository's standing rule for a check nobody has watched +**`checkApiTiers` holds the other direction**, which a sample cannot: that no unmarked public +signature *exposes* an experimental type. Module-wide opt-in means the compiler permits exactly that +inside the library, so a consumer could be handed a `ColumnReader` by a method carrying no marker at +all — the tier statement above quietly ceasing to be true. The audit reads the marker set from the +sources and the surface from the committed dumps, both derived rather than listed, and runs at the +root because the leak is cross-module. It found four leaks the first time it was run by hand. + +Verified by breaking them, which is this repository's standing rule for a check nobody has watched fail: adding `db.store.flush()` to a sample fails `./gradlew build` with the opt-in error naming the -marker, and commenting out the opt-in in `rabosh.kotlin-library` fails the published modules. +marker; commenting out the opt-in in `rabosh.kotlin-library` fails the published modules; and removing +the marker from `IndexCatalog.read` fails `checkApiTiers` naming the method, the type it exposes and +the module. ## Reporting diff --git a/build-logic/src/main/kotlin/app/oreshkov/rabosh/build/ApiTierAudit.kt b/build-logic/src/main/kotlin/app/oreshkov/rabosh/build/ApiTierAudit.kt new file mode 100644 index 0000000..89db54c --- /dev/null +++ b/build-logic/src/main/kotlin/app/oreshkov/rabosh/build/ApiTierAudit.kt @@ -0,0 +1,328 @@ +package app.oreshkov.rabosh.build + +import java.io.File + +/** + * Whether any declaration outside the experimental tier exposes a type that is inside it. + * + * **This exists because the compiler cannot do it, and the reason is worth stating precisely.** + * `STABILITY.md` marks the entrances to the experimental tier rather than every member, and the + * published modules opt in to their own marker module-wide so that the engine's internal use of its + * own internals does not need several hundred `@OptIn`s. That opt-in is what blinds the compiler: + * inside the library every use of an experimental type is permitted, *including a use in a public + * signature that carries no marker of its own*. A consumer meeting that signature gets an + * experimental type handed to them with nothing having asked them to opt in — the tier statement + * quietly stops being true, and `checkKotlinAbi` cannot see it because the JVM dump format writes + * signatures and never annotations. + * + * The gap is real rather than theoretical: it was found by hand in phase 23 with four leaks in it, + * and phase 24 added public surface to every module in the chain with nothing but a script in a + * private directory standing behind the claim. This is that script, promoted to a gate. + * + * **Both halves are derived, neither is listed.** The experimental set comes from the *sources* — + * every `@RaboshExperimental` and the declaration under it — and the surface comes from the + * *committed ABI 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. + * + * **A scanner that does not understand an annotation is the defect this class exists to prevent, one + * level up**, so [markedIn] counts the annotations it found against the declarations it attributed + * and reports the difference rather than carrying on with a short set. Under-reporting is the only + * failure mode that matters here: an audit that misses a leak passes, and passing is what it is + * consulted for. + * + * Plain Kotlin over [File] with unit tests, for the reason [BenchmarkRunReport] and + * [CentralBundleReport] are: an included build's tests are run by `./gradlew -p build-logic check`, + * and a decision that fails the build is worth testing. + */ +object ApiTierAudit { + + /** The opt-in marker, by simple name. Its package is not needed: nothing else is called this. */ + const val MARKER: String = "RaboshExperimental" + + /** What the sources say is experimental. */ + class Tier( + /** Simple names of types the marker is on. */ + val types: Set, + /** `Type.member` for members the marker is on; a constructor is spelled `Type.constructor`. */ + val members: Set, + ) { + val isEmpty: Boolean get() = types.isEmpty() && members.isEmpty() + + override fun toString(): String = "Tier(${types.size} type(s), ${members.size} member(s))" + } + + /** One public signature naming an experimental type without being marked itself. */ + class Leak( + val module: String, + val owner: String, + val member: String, + val referenced: String, + ) { + override fun toString(): String = + "$module: $owner.$member exposes $referenced, which is @$MARKER, without being marked itself" + } + + /** + * The declarations [MARKER] is applied to, read from the Kotlin sources under [modules]. + * + * Two spellings are recognised because both are in use: the annotation on its own line above a + * declaration, and inline as `class Foo @RaboshExperimental constructor(`. A third would be a + * silent hole, which is what [UnreadableMarker] is for. + * + * @throws IllegalStateException if an annotation was found that no declaration could be + * attributed to. See the class documentation: a short set is worse than a failure. + */ + fun markedIn(modules: List): Tier { + val types = sortedSetOf() + val members = sortedSetOf() + val unreadable = ArrayList() + + for (module in modules) { + val sourceRoot = File(module, "src/main/kotlin") + if (!sourceRoot.isDirectory) continue + for (file in sourceRoot.walkTopDown()) { + if (!file.isFile || file.extension != "kt") continue + scan(file, types, members, unreadable) + } + } + + check(unreadable.isEmpty()) { + "the tier audit found ${unreadable.size} @$MARKER annotation(s) it could not attribute to a " + + "declaration, so its experimental set is incomplete and would under-report:\n" + + unreadable.joinToString("\n") { " $it" } + + "\nTeach ApiTierAudit the spelling, or spell the declaration the way the others are." + } + return Tier(types, members) + } + + /** + * Every unmarked public signature in [dumps] that names a type in [tier]. + * + * Empty is the passing answer. The check runs over the *committed* dumps rather than over + * compiled classes because the dumps are the artefact `checkKotlinAbi` already maintains: a + * signature that is not in them is not published, and one that is has been reviewed in a diff. + */ + fun leaks(dumps: Map, tier: Tier): List { + if (tier.isEmpty) return emptyList() + val found = ArrayList() + for ((module, dump) in dumps) { + if (!dump.isFile) continue + var owner = "" + var ownerNames = emptyList() + var ownerIsExperimental = false + + for (line in dump.readLines()) { + if (line.isBlank() || line == "}") continue + if (!line.startsWith("\t")) { + owner = binaryNameOf(line) ?: "" + ownerNames = simpleNamesOf(owner) + ownerIsExperimental = ownerNames.any { it in tier.types } + // A supertype list is on this line too, so an unmarked class implementing an + // experimental interface is caught here rather than by any member below it. + // Matched as a *bare* binary name rather than as a descriptor: a header spells a + // supertype `: app/oreshkov/…/Bitmap`, with none of the `L…;` a signature has. + if (!ownerIsExperimental) { + for (referenced in experimentalTypesIn(line.substringAfter(owner), tier, descriptors = false)) { + found += Leak(module, simpleOwner(ownerNames), "(supertype)", referenced) + } + } + continue + } + if (ownerIsExperimental) continue + + val member = memberNameOf(line) ?: continue + if (isMarked(ownerNames, member, tier)) continue + for (referenced in experimentalTypesIn(line, tier)) { + found += Leak(module, simpleOwner(ownerNames), member, referenced) + } + } + } + return found.distinctBy { "${it.module}|${it.owner}|${it.member}|${it.referenced}" } + } + + /** The published modules' dumps, by module name. The universe is [PublishedModules]'. */ + fun dumpsUnder(root: File): Map = + PublishedModules.under(root).associateWith { File(root, "$it/api/$it.api") } + + /** The published modules' directories. */ + fun modulesUnder(root: File): List = + PublishedModules.under(root).map { File(root, it) } + + // --- reading the sources ---------------------------------------------------------------------- + + private val TYPE_DECLARATION = + Regex("""^\s*(?:public |internal |private |protected )?(?:[\w@.]+ )*?(class|interface|object)\s+(\w+)""") + + private val FUNCTION_DECLARATION = Regex("""\bfun\s+(?:<[^>]*>\s*)?(\w+)\s*[(<]""") + private val PROPERTY_DECLARATION = Regex("""\b(?:val|var)\s+(\w+)\s*[:=]""") + private val INLINE_CONSTRUCTOR = Regex("""@$MARKER\s+constructor\s*\(""") + + /** One open type declaration and the column it was declared at. */ + private class Scope(val name: String, val indent: Int) + + /** + * Scans one file, tracking which type each declaration is inside. + * + * **Nesting is followed by indentation rather than by counting braces**, and the choice is a + * decision. Kotlin string templates put `{` and `}` inside string literals, so a brace counter + * needs a lexer to be right and is wrong in a way nobody notices until it mis-attributes one + * member. Indentation needs no lexer and is exact for any code laid out the way this repository's + * is: a declaration at column *n* is inside the nearest type declared at a column below *n*. + * + * Getting this wrong was demonstrated rather than imagined — the first version took the *most + * recent* type declaration, and `IndexCatalog.read` came out attributed to a `private class` + * declared two hundred lines above it, which reports a marked member as a leak. A false positive + * is what gets a gate switched off, so it is worth the ten lines. + * + * A `companion object` re-pushes its **outer** name: in a dump its members appear on + * `Foo$Companion` and statically on `Foo`, and attributing them to `Foo` is what makes both + * spellings match the one source declaration. + */ + private fun scan(file: File, types: MutableSet, members: MutableSet, unreadable: MutableList) { + val lines = file.readLines() + val scopes = ArrayList() + + fun enclosing(indent: Int): String = scopes.lastOrNull { it.indent < indent }?.name.orEmpty() + + for ((index, line) in lines.withIndex()) { + val indent = line.indexOfFirst { !it.isWhitespace() } + if (indent >= 0) { + val declared = typeNameOf(line) + val companion = line.contains("companion object") + if (declared != null || companion) { + while (scopes.isNotEmpty() && scopes.last().indent >= indent) scopes.removeLast() + scopes += Scope(declared ?: enclosing(indent), indent) + } + } + + if (!line.contains("@$MARKER")) continue + val at = if (indent >= 0) indent else 0 + + // Inline: `public class SchemaCatalog @RaboshExperimental constructor(`. + if (INLINE_CONSTRUCTOR.containsMatchIn(line)) { + val owner = typeNameOf(line) ?: enclosing(at) + if (owner.isEmpty()) { + unreadable += "${file.name}:${index + 1} — inline constructor with no type on the line" + } else { + members += "$owner.constructor" + } + continue + } + // An import, a KDoc reference, or an `@OptIn` naming it: not an application of it. + if (!isAnnotationApplication(line)) continue + + val declaration = declarationAfter(lines, index) + if (declaration == null) { + unreadable += "${file.name}:${index + 1} — no declaration follows the annotation" + continue + } + val type = typeNameOf(declaration) + if (type != null) { + types += type + continue + } + val name = FUNCTION_DECLARATION.find(declaration)?.groupValues?.get(1) + ?: PROPERTY_DECLARATION.find(declaration)?.groupValues?.get(1) + // The declaration's own column, and the comparison is **strictly** less than it. A + // sibling declared at the same column — `private class CountedObservation` beside + // `public fun read` — is not an enclosing scope, and treating it as one is exactly the + // mis-attribution this tracking exists to avoid. + val owner = enclosing(declaration.indexOfFirst { !it.isWhitespace() }.coerceAtLeast(0)) + if (name == null || owner.isEmpty()) { + unreadable += "${file.name}:${index + 1} — cannot name the declaration: ${declaration.trim()}" + } else { + members += "$owner.$name" + } + } + } + + /** + * Whether this line *applies* the marker rather than merely mentioning it. + * + * An `import`, a KDoc line and an `@OptIn(RaboshExperimental::class)` all contain the word. Only + * the first token being the annotation means it is being applied here. + */ + private fun isAnnotationApplication(line: String): Boolean { + val trimmed = line.trim() + if (trimmed.startsWith("import ") || trimmed.startsWith("*") || trimmed.startsWith("//")) return false + return trimmed.startsWith("@$MARKER") + } + + /** The next line that is neither blank, a comment, nor another annotation. */ + private fun declarationAfter(lines: List, from: Int): String? { + for (index in from + 1 until lines.size) { + val trimmed = lines[index].trim() + if (trimmed.isEmpty() || trimmed.startsWith("//") || trimmed.startsWith("*") || + trimmed.startsWith("/*") || trimmed.startsWith("@") + ) { + continue + } + return lines[index] + } + return null + } + + private fun typeNameOf(line: String): String? { + if (line.contains("companion object")) return null + val match = TYPE_DECLARATION.find(line) ?: return null + // `object : Foo {` and `enum class` both land here; only a named declaration counts. + return match.groupValues[2].takeIf { it.isNotEmpty() } + } + + // --- reading the dumps ------------------------------------------------------------------------ + + /** `public final class app/oreshkov/rabosh/index/Bitmap : … {` -> the binary name. */ + private fun binaryNameOf(header: String): String? = + Regex("""\b(app/oreshkov/[\w/$]+)""").find(header)?.groupValues?.get(1) + + /** `app/oreshkov/rabosh/query/Predicate$And` -> `[Predicate, And]`. */ + private fun simpleNamesOf(binaryName: String): List = + binaryName.substringAfterLast('/').split('$').filter { it.isNotEmpty() } + + private fun simpleOwner(names: List): String = names.joinToString(".").ifEmpty { "?" } + + /** `\tpublic final fun getStore ()L…;` -> `getStore`. Fields and constructors included. */ + private fun memberNameOf(line: String): String? = + Regex("""\b(?:fun|field)\s+([\w$<>]+)""").find(line)?.groupValues?.get(1) + + /** + * Whether [member] of a class named by [ownerNames] carries the marker in the sources. + * + * The dump's spelling and the source's differ in three ways and all three are normalised here: a + * property is `getFoo`/`setFoo` against `foo`, a constructor is `` against `constructor`, + * and a default-argument bridge is `foo$default` against `foo`. Every enclosing name is tried, so + * a member on `Foo$Companion` matches a source declaration attributed to `Foo`. + */ + private fun isMarked(ownerNames: List, member: String, tier: Tier): Boolean { + val candidates = LinkedHashSet() + candidates += member + candidates += member.substringBefore("\$default") + if (member == "") candidates += "constructor" + for (prefix in listOf("get", "set")) { + if (member.length > prefix.length && member.startsWith(prefix) && member[prefix.length].isUpperCase()) { + candidates += member.removePrefix(prefix).replaceFirstChar { it.lowercase() } + } + } + return ownerNames.any { owner -> candidates.any { "$owner.$it" in tier.members } } + } + + /** + * Every experimental type this line names, in order of appearance. + * + * Two spellings, because the dump has two. A *signature* carries JVM descriptors — + * `Lapp/oreshkov/rabosh/index/Bitmap;` — and matching those with delimiters is what stops + * `…/BitmapView` being read as `Bitmap`. A *class header* carries bare binary names after the + * colon, with no `L` and no `;`, so the supertype case has to ask for the looser form; there the + * `$`-split of a whole name is the boundary instead. + */ + private fun experimentalTypesIn(line: String, tier: Tier, descriptors: Boolean = true): List { + val pattern = if (descriptors) Regex("""L(app/oreshkov/[\w/$]+);""") else Regex("""\b(app/oreshkov/[\w/$]+)""") + return pattern.findAll(line) + .flatMap { simpleNamesOf(it.groupValues[1]).asSequence() } + .filter { it in tier.types } + .distinct() + .toList() + } +} diff --git a/build-logic/src/main/kotlin/rabosh.api-tiers.gradle.kts b/build-logic/src/main/kotlin/rabosh.api-tiers.gradle.kts new file mode 100644 index 0000000..50b28b9 --- /dev/null +++ b/build-logic/src/main/kotlin/rabosh.api-tiers.gradle.kts @@ -0,0 +1,66 @@ +import app.oreshkov.rabosh.build.ApiTierAudit + +// Applied at the root and nowhere else, because the question is cross-module: a type marked +// experimental in `rabosh-index` can leak through a signature in `rabosh-query`'s dump, and no +// per-module task can see both. +// +// This is the gate `checkKotlinAbi` cannot be. The JVM dump format writes signatures and never +// annotations, so a declaration changing tier is invisible to it — and the published modules opt in +// to their own marker module-wide, so it is invisible to the *compiler* too. Between them that +// leaves the tier statement in `STABILITY.md` held up by nothing, which is what this fixes. + +/** + * Fails when a public signature exposes an experimental type without being marked itself. + * + * The rule lives in [ApiTierAudit] rather than here, for the reason `CentralBundleReport` and + * `BenchmarkRunReport` do: a decision that fails a build is worth unit tests, and an included build's + * tests are run by `./gradlew -p build-logic check` rather than by the root `build`. + * + * Both inputs are declared, so this is up to date when neither the sources nor the dumps have moved — + * and it re-runs when either has, which is exactly when the answer can change. + */ +val checkApiTiers = tasks.register("checkApiTiers") { + group = "verification" + description = "Checks that no unmarked public signature exposes a @RaboshExperimental type." + + val root = layout.projectDirectory.asFile + val modules = ApiTierAudit.modulesUnder(root) + val dumps = ApiTierAudit.dumpsUnder(root) + + inputs.files(dumps.values.filter { it.isFile }).withPropertyName("abiDumps") + inputs.files(modules.map { File(it, "src/main/kotlin") }.filter { it.isDirectory }) + .withPropertyName("sources") + .withPathSensitivity(PathSensitivity.RELATIVE) + // Nothing is produced, so up-to-date needs somewhere to record that it ran. + outputs.file(layout.buildDirectory.file("reports/api-tiers/checked.txt")) + + val report = layout.buildDirectory.file("reports/api-tiers/checked.txt") + + doLast { + val tier = ApiTierAudit.markedIn(modules) + val leaks = ApiTierAudit.leaks(dumps, tier) + + val summary = buildString { + appendLine("experimental tier: $tier") + appendLine("dumps checked: ${dumps.size}") + appendLine("leaks: ${leaks.size}") + for (leak in leaks) appendLine(" $leak") + } + report.get().asFile.apply { parentFile.mkdirs() }.writeText(summary) + + if (leaks.isNotEmpty()) { + throw GradleException( + "${leaks.size} public signature(s) expose a @${ApiTierAudit.MARKER} type without carrying " + + "the marker, so a consumer reaches the experimental tier with nothing asking them to " + + "opt in:\n" + leaks.joinToString("\n") { " $it" } + + "\n\nMark the declaration, or move the type into the stable core and say so in " + + "STABILITY.md. See ApiTierAudit for why the compiler cannot report this.", + ) + } + logger.lifecycle("API tiers: ${dumps.size} dump(s) checked against $tier, no leaks.") + } +} + +// `check` is the root's, from `base` by way of the Dokka plugin, and `build` depends on it — so this +// runs under the `./gradlew build` that CI already invokes rather than needing a step of its own. +tasks.named("check") { dependsOn(checkApiTiers) } diff --git a/build-logic/src/main/kotlin/rabosh.kotlin-library.gradle.kts b/build-logic/src/main/kotlin/rabosh.kotlin-library.gradle.kts index f8f1f3b..e098600 100644 --- a/build-logic/src/main/kotlin/rabosh.kotlin-library.gradle.kts +++ b/build-logic/src/main/kotlin/rabosh.kotlin-library.gradle.kts @@ -32,6 +32,11 @@ kotlin { * silently changes tier fails there, and so does a sample that reaches past the facade. The ABI * dumps cannot do this job: the JVM dump format writes signatures only and never annotations, so * a tier change is invisible to `checkKotlinAbi`. + * + * What a sample cannot catch is a public signature that *exposes* an experimental type without + * carrying the marker — this opt-in permits exactly that inside the library, and a sample only + * sees what it happens to call. `checkApiTiers` at the root is that half, and the two together + * are what hold `STABILITY.md`'s claim up. */ compilerOptions { optIn.add("app.oreshkov.rabosh.RaboshExperimental") diff --git a/build-logic/src/test/kotlin/app/oreshkov/rabosh/build/ApiTierAuditTest.kt b/build-logic/src/test/kotlin/app/oreshkov/rabosh/build/ApiTierAuditTest.kt new file mode 100644 index 0000000..59c84f6 --- /dev/null +++ b/build-logic/src/test/kotlin/app/oreshkov/rabosh/build/ApiTierAuditTest.kt @@ -0,0 +1,342 @@ +package app.oreshkov.rabosh.build + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.api.io.TempDir +import java.io.File + +/** + * The tier audit, against fixtures rather than against the repository. + * + * **A check that reports nothing has to be shown capable of reporting something**, which is most of + * this file: every "clean" case sits beside the same shape with one thing changed so that it leaks. + * An audit whose parser quietly stopped understanding the codebase would go green for ever, and going + * green is the only thing anybody reads it for. + */ +class ApiTierAuditTest { + + @TempDir + lateinit var root: File + + // --- reading the sources ---------------------------------------------------------------------- + + @Test + fun `a marker on a type, a member and a constructor is found in each spelling`() { + module( + "rabosh-index", + "Bitmap.kt" to """ + package app.oreshkov.rabosh.index + + import app.oreshkov.rabosh.RaboshExperimental + + @RaboshExperimental + public class Bitmap private constructor(private val keys: IntArray) { + public fun cardinality(): Int = 0 + } + """, + "IndexCatalog.kt" to """ + package app.oreshkov.rabosh.index + + public class IndexCatalog @RaboshExperimental constructor( + public val directory: String, + ) { + /** Doc mentioning @RaboshExperimental should not count. */ + @RaboshExperimental + public fun read(handle: Int): Bitmap = Bitmap() + + public fun indexes(): List = emptyList() + } + """, + ) + + val tier = ApiTierAudit.markedIn(listOf(File(root, "rabosh-index"))) + + assertEquals(setOf("Bitmap"), tier.types) + assertEquals(setOf("IndexCatalog.constructor", "IndexCatalog.read"), tier.members) + } + + /** An import and an `@OptIn` both contain the word and neither applies it. */ + @Test + fun `mentioning the marker is not applying it`() { + module( + "rabosh-core", + "DocumentStore.kt" to """ + package app.oreshkov.rabosh.core + + import app.oreshkov.rabosh.RaboshExperimental + + /** + * See [RaboshExperimental] for what this means. + */ + @OptIn(RaboshExperimental::class) + public class DocumentStore { + public fun put(key: String) {} + } + """, + ) + + assertTrue(ApiTierAudit.markedIn(listOf(File(root, "rabosh-core"))).isEmpty) + } + + /** + * A marked member is attributed to the type it is **inside**, not to the last one declared. + * + * The shape is `IndexCatalog`'s, and it is here because the first version of this scanner got it + * wrong: it took the most recent type declaration, so `read` came out as + * `CountedObservation.read` and the audit reported a marked member as a leak. A false positive is + * what gets a gate switched off, so the sibling case is pinned rather than assumed. + */ + @Test + fun `a member is attributed to the type it is inside, not the last one declared`() { + module( + "rabosh-index", + "IndexCatalog.kt" to """ + package app.oreshkov.rabosh.index + + public class IndexCatalog { + + private class CountedObservation(private val count: Int) { + fun observe() {} + } + + @RaboshExperimental + public fun read(handle: Int): Bitmap = Bitmap() + } + """, + ) + + val tier = ApiTierAudit.markedIn(listOf(File(root, "rabosh-index"))) + assertEquals(setOf("IndexCatalog.read"), tier.members) + } + + /** A companion's members belong to the outer type, because that is where the dump puts them. */ + @Test + fun `a companion member is attributed to its outer type`() { + module( + "rabosh-core", + "DocumentStore.kt" to """ + package app.oreshkov.rabosh.core + + public class DocumentStore { + + private class Pinned(val live: Set) + + public companion object { + @RaboshExperimental + public fun open(directory: String): DocumentStore = DocumentStore() + } + } + """, + ) + + val tier = ApiTierAudit.markedIn(listOf(File(root, "rabosh-core"))) + assertEquals(setOf("DocumentStore.open"), tier.members) + } + + /** + * A spelling the scanner cannot attribute is a **failure**, never a shorter set. + * + * This is the direction that matters. An audit that silently dropped an annotation would build a + * short experimental set and then report no leaks against it — passing, for the worst reason. + */ + @Test + fun `an annotation the scanner cannot attribute fails loudly`() { + module( + "rabosh-core", + "Odd.kt" to """ + package app.oreshkov.rabosh.core + + public class Odd { + @RaboshExperimental + } + """, + ) + + val failure = assertThrows { + ApiTierAudit.markedIn(listOf(File(root, "rabosh-core"))) + } + assertTrue("could not attribute" in failure.message!!, failure.message) + assertTrue("Odd.kt" in failure.message!!, failure.message) + } + + // --- reading the dumps ------------------------------------------------------------------------ + + private val tier = ApiTierAudit.Tier( + types = setOf("Bitmap", "ColumnReader"), + members = setOf("Rabosh.store", "IndexCatalog.read", "DocumentStore.open", "SchemaCatalog.constructor"), + ) + + @Test + fun `a marked member returning an experimental type is not a leak`() { + val dump = dump( + "rabosh-index", + """ + public final class app/oreshkov/rabosh/index/IndexCatalog { + public final fun read (I)Lapp/oreshkov/rabosh/index/Bitmap; + public final fun indexes ()Ljava/util/List; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, tier)) + } + + /** The same dump with the marker taken off `read`: the audit must now see it. */ + @Test + fun `an unmarked member returning an experimental type is a leak`() { + val dump = dump( + "rabosh-index", + """ + public final class app/oreshkov/rabosh/index/IndexCatalog { + public final fun read (I)Lapp/oreshkov/rabosh/index/Bitmap; + } + """, + ) + val leaks = ApiTierAudit.leaks(dump, ApiTierAudit.Tier(tier.types, emptySet())) + + assertEquals(1, leaks.size, leaks.toString()) + assertEquals("IndexCatalog", leaks.single().owner) + assertEquals("read", leaks.single().member) + assertEquals("Bitmap", leaks.single().referenced) + } + + /** A member *taking* an experimental type leaks exactly as one returning it does. */ + @Test + fun `an experimental parameter is a leak too`() { + val dump = dump( + "rabosh-query", + """ + public final class app/oreshkov/rabosh/query/Plan { + public final fun intersect (Lapp/oreshkov/rabosh/index/Bitmap;)V + } + """, + ) + assertEquals(1, ApiTierAudit.leaks(dump, tier).size) + } + + /** Everything inside an experimental class is already behind the opt-in that got you there. */ + @Test + fun `members of an experimental class are not leaks`() { + val dump = dump( + "rabosh-index", + """ + public final class app/oreshkov/rabosh/index/Bitmap { + public final fun copy ()Lapp/oreshkov/rabosh/index/Bitmap; + public final fun reader ()Lapp/oreshkov/rabosh/index/ColumnReader; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, tier)) + } + + /** An unmarked class *implementing* an experimental interface is a leak on the header line. */ + @Test + fun `an experimental supertype is a leak`() { + val dump = dump( + "rabosh-index", + """ + public final class app/oreshkov/rabosh/index/Mask : app/oreshkov/rabosh/index/Bitmap { + public final fun size ()I + } + """, + ) + val leaks = ApiTierAudit.leaks(dump, tier) + assertEquals(1, leaks.size, leaks.toString()) + assertEquals("(supertype)", leaks.single().member) + } + + /** + * The three spellings the dump and the sources disagree on, each pinned. + * + * A property is `getStore` against `store`; a constructor is `` against `constructor`; a + * default-argument bridge is `read$default` against `read`. Getting any of them wrong makes the + * audit report a marked declaration as a leak, which is the failure that gets a gate switched off. + */ + @Test + fun `the dump's spellings are normalised to the source's`() { + val dump = dump( + "rabosh-api", + """ + public final class app/oreshkov/rabosh/api/Rabosh { + public final fun getStore ()Lapp/oreshkov/rabosh/index/Bitmap; + } + + public final class app/oreshkov/rabosh/catalog/SchemaCatalog { + public fun (Lapp/oreshkov/rabosh/index/Bitmap;)V + } + + public final class app/oreshkov/rabosh/index/IndexCatalog { + public static synthetic fun read${'$'}default (Lapp/oreshkov/rabosh/index/Bitmap;)Lapp/oreshkov/rabosh/index/Bitmap; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, tier)) + } + + /** A companion's members appear on `Foo${'$'}Companion` and are declared inside `Foo`. */ + @Test + fun `a companion member matches a marker attributed to its outer type`() { + val dump = dump( + "rabosh-core", + """ + public final class app/oreshkov/rabosh/core/DocumentStore${'$'}Companion { + public final fun open (Ljava/lang/String;)Lapp/oreshkov/rabosh/index/Bitmap; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, tier)) + } + + /** A signature naming nothing experimental is never a leak, however much else it names. */ + @Test + fun `an ordinary signature is not a leak`() { + val dump = dump( + "rabosh-core", + """ + public final class app/oreshkov/rabosh/core/Key { + public final fun successor ()Lapp/oreshkov/rabosh/core/Key; + public static final fun of (Ljava/lang/String;)Lapp/oreshkov/rabosh/core/Key; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, tier)) + } + + /** With nothing marked there is nothing to leak, and the audit says so rather than scanning. */ + @Test + fun `an empty tier reports nothing`() { + val dump = dump( + "rabosh-index", + """ + public final class app/oreshkov/rabosh/index/IndexCatalog { + public final fun read (I)Lapp/oreshkov/rabosh/index/Bitmap; + } + """, + ) + assertEquals(emptyList(), ApiTierAudit.leaks(dump, ApiTierAudit.Tier(emptySet(), emptySet()))) + } + + /** A missing dump is not a failure here: `checkKotlinAbi` owns that, and owning it twice is worse. */ + @Test + fun `a module with no dump is skipped`() { + assertEquals( + emptyList(), + ApiTierAudit.leaks(mapOf("rabosh-ghost" to File(root, "nowhere.api")), tier), + ) + } + + // --- fixtures --------------------------------------------------------------------------------- + + private fun module(name: String, vararg sources: Pair) { + val directory = File(root, "$name/src/main/kotlin/app/oreshkov/rabosh") + directory.mkdirs() + for ((file, text) in sources) File(directory, file).writeText(text.trimIndent()) + } + + private fun dump(module: String, text: String): Map { + val file = File(root, "$module.api") + file.writeText(text.trimIndent()) + return mapOf(module to file) + } +} diff --git a/build.gradle.kts b/build.gradle.kts index b8b5848..e5c8304 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -7,7 +7,9 @@ plugins { // Applied: a Central deployment is one archive over every published module, so it is assembled // and checked from the only project that can see all of them. The aggregated API documentation - // is one site over the same set, for the same reason. + // is one site over the same set, for the same reason — and so is the stability-tier audit, since + // a type marked experimental in one module leaks through a signature in another's dump. id("rabosh.central-bundle") id("rabosh.api-docs") + id("rabosh.api-tiers") }