diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index 7f636255e..742b7a93f 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -2570,8 +2570,28 @@ fn without_comments(path: &str, text: &str) -> String { let prefix = match path.rsplit_once('.').map(|(_, ext)| ext) { // Markdown is prose end to end; there is no non-comment remainder. Some("md") => return String::new(), - Some("rs") => "//", - Some("sh" | "bash" | "bats") => "#", + // `pkl` SITS WITH RUST RATHER THAN WITH THE OTHER DECLARATION + // LANGUAGES: it is Java-family, and `#` is not a comment marker there at + // all. One table for everything that merely looks `#`-ish is how a + // remainder silently keeps its real code and drops a commented line. + // + // The two `//` and the four `#` groups are one arm each because + // `clippy::match_same_arms` refuses a table that repeats a body, so the + // grouping a reader wants lives in these comments rather than in the + // arms. + Some("rs" | "pkl") => "//", + // The shell family, and THE DECLARATION LANGUAGES beside it — policy + // modules, config, pipeline and workflow documents. Only the shell half + // was in CLOUD-827's original table, which named the languages a + // consumer writes PROGRAMS in. A declarative consumer keeps most of its + // prose in the other half, and every one of those took the fall-through + // below, so a comment-only change to one read as a code change and the + // whole prose-only class could not fire over them. + // + // The fall-through's direction is still right for a genuinely unknown + // extension — it admits rather than blocks, and only blocking is + // unrecoverable by waiting. What was wrong is calling these unknown. + Some("sh" | "bash" | "bats" | "rego" | "toml" | "yml" | "yaml") => "#", // `mise-tasks/` programs carry no extension (CLOUD-865 renamed most to // `.sh`, but the pattern stays so a re-added extensionless task is still // read). The check is on the DIRECTORY, so it cannot claim a file @@ -4091,6 +4111,59 @@ mod tests { use super::*; + /// The declaration languages have their comments stripped, so a rule over + /// the remainder can see a comment-only change to one of them. + /// + /// This is the case CLOUD-827's table was missing: `.rego`, `.toml`, + /// `.yml`/`.yaml` and `.pkl` all took the unrecognised-extension + /// fall-through, whose remainder is the whole file, so every prose change to + /// a policy module, a config document or a workflow read as a code change. + /// + /// Fails by: removing an extension token from `without_comments`'s table. + /// Each assertion below reddens for exactly the token that went, which is + /// what stops one extension's presence standing in for its neighbours' — + /// they share two arms, so a per-extension case is the only granularity the + /// table itself does not provide. + #[test] + fn a_declaration_language_has_its_comments_stripped() { + for path in ["a/x.rego", "x.toml", "a/x.yml", "a/x.yaml"] { + assert_eq!( + without_comments(path, "# a comment\nreal = 1\n"), + "real = 1", + "`#` is the comment marker for {path}, so a comment-only change \ + must leave the remainder identical" + ); + } + // pkl is Java-family: `//`, and `#` is not a comment marker there at all. + // Reading it with the `#` table would keep a commented line in the + // remainder and drop a real one. + assert_eq!( + without_comments("x.pkl", "// a comment\nreal = 1\n"), + "real = 1", + "pkl comments are `//`, so the `#` table would misread every line" + ); + } + + /// And a genuinely unknown extension still takes the admitting direction. + /// + /// The paired half of the case above: widening the table must not turn the + /// fall-through into a guess. An extension nobody has classified has no + /// comments, so its whole content is the remainder and any change to it is a + /// code change — which ADMITS the branch rather than blocking it, and only + /// blocking is unrecoverable by waiting. + /// + /// Fails by: making the fall-through strip `#`, which would read a `.conf` + /// or `.ini` prose change as prose-only on a guess. + #[test] + fn an_unclassified_extension_still_reads_as_code() { + assert_eq!( + without_comments("thing.conf", "# a comment\nreal = 1\n"), + "# a comment\nreal = 1\n", + "an unrecognised extension has no comments, so the remainder is the \ + whole file and the gate admits rather than blocks" + ); + } + /// THE WINDOWS REGRESSION, TESTED AS A DECISION RATHER THAN A CONDITION. /// /// The failing condition — `canonicalize` returning a verbatim path — is one diff --git a/crates/batten/tests/it/prose_only.rs b/crates/batten/tests/it/prose_only.rs index a3ebc1aa5..d1d1dac39 100644 --- a/crates/batten/tests/it/prose_only.rs +++ b/crates/batten/tests/it/prose_only.rs @@ -370,6 +370,73 @@ fn an_unresolvable_base_says_nothing_rather_than_refusing() { admitted(&root); } +/// A comment-only change to a DECLARATION language is refused. +/// +/// The case this suite was missing for its whole life, and the reason it went +/// unnoticed is worth the paragraph. Every arm above is a `.rs`, a `.md`, a +/// `.bats` or an extensionless shell program — the languages a consumer writes +/// PROGRAMS in, which is the set the shell predecessor classified. This +/// repository writes most of its prose somewhere else entirely: `.rego` policy +/// modules, the config authority, `hk.pkl`, and the workflows. All four took +/// `without_comments`'s unrecognised-extension arm, whose remainder is the WHOLE +/// file, so a comment-only change to any of them read as a code change and this +/// gate could not fire over it. +/// +/// Measured 2026-09-04: a 121→71-comment trim of a policy module, no other +/// change in the branch, `batten check --rule prose-only` exit 0, a full +/// required matrix spent. That is exactly the instance CLOUD-827 exists to +/// price, and it walked past because the classifier had never been told what a +/// `.rego` comment looks like. +/// +/// One case per extension rather than one representative, because the arms are +/// independent table entries and `.pkl` in particular is `//` where the other +/// three are `#` — a single case would let one arm's presence stand in for the +/// rest. +/// +/// Fails by: removing the matching extension token from `without_comments`'s +/// table, which returns that file's whole text as its remainder and admits the +/// branch. +#[test] +fn a_comment_only_change_to_a_declaration_language_is_refused() { + for (path, before, after) in [ + ( + "policy/a.rego", + "# a note\nviolation contains 1 if { true }\n", + "# a different note\nviolation contains 1 if { true }\n", + ), + ( + "conf.toml", + "# a note\nversion = 1\n", + "# a different note\nversion = 1\n", + ), + ( + ".github/workflows/w.yml", + "# a note\non: push\n", + "# a different note\non: push\n", + ), + ( + "gate.pkl", + "// a note\nx = 1\n", + "// a different note\nx = 1\n", + ), + ] { + let stem = path.rsplit('/').next().unwrap_or(path); + let root = repo( + &format!("declaration-{stem}"), + &[(path, before)], + &Head { + written: &[(path, after)], + removed: &[], + }, + ); + assert_eq!( + findings(&root), + vec!["prose-only".to_owned()], + "a comment-only change to {path} must be priced as prose-only" + ); + } +} + // subsumed: "a Rust block comment is NOT read as prose" crates/batten/tests/it/prose_only.rs #[test] fn a_shell_program_carrying_no_extension_is_read_as_shell() {