Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 75 additions & 2 deletions crates/batten/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
67 changes: 67 additions & 0 deletions crates/batten/tests/it/prose_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down