From fb22cb45dd8f3a429f603fe7cf1693dfd11400a8 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Thu, 20 Aug 2026 22:46:39 +0900 Subject: [PATCH 1/2] A resolver for a command a reader would run, the third of the three `links-resolve` resolves a path a reader would CLICK. `anchors-resolve` a value a reader would BELIEVE. This one a command a reader would RUN, which is the last of the three things a document asserts about a tree -- and before any of them existed all three failed the same way: silently, forever, with every gate in every repository still green. The defect it was built from is measured rather than imagined. A README opened with `fg-registry credentials` for as long as the file existed. That command has two verbs and `credentials` was never one of them; the binary's own error names the alternatives, so the answer was one invocation away. Nobody invoked it, because a reader who trusts the README has no reason to and a reader who does not is not reading the README. WHY THE DISPATCH AND NOT `--help`. Running the binary needs a build, and a gate that needs the network is a gate that gets skipped. Worse, it invites resolving a verb by RUNNING it: `fg-registry sync` in a document would be verified by fast-forwarding thirty-nine submodules. Only the help text is safe to execute for, and help text is prose too -- a doc comment drifts from the switch below it exactly as the README drifted. The switch IS the verb list. WHY A GRAMMAR. The implementation this ports read dispatches with a regex over lines and an allow-list of eight subject spellings taken off dispatches in one workspace. Both halves of its measured false-finding rate come from that: a tagless `switch {` is a line the matcher cannot read, and a subject list read off one tree describes one tree. tree-sitter was already in this binary for the comment checks, so Go is one grammar crate and Rust is already here. A dispatch is now recognised structurally -- it dispatches on something, two or more branches match string literals, a catch-all exists -- and each of those three conditions is a false finding the line matcher produced. WHY A COMMAND MUST AGREE WITH ITSELF FIRST. A verb list read wrong is worse than no verb list: it produces confident findings against documents that were right. The first run of the ported implementation reported 38 findings and one binary supplied 22 of them, every one false and every one reading as real. So a command judges documents only when two readings of its own sources agree -- the string labels of its dispatch, and the verbs its own usage block names about itself -- and when they disagree it is counted, named and skipped. The count is printed every run, because a check that read four commands out of a hundred otherwise reads exactly like one that read them all. Zero judged is exit 1 rather than a pass, for the same reason. WHY A PATTERN AND NOT A TABLE. `command_sources = ["cmd/{}/*.go"]` says what a command looks like in this tree and captures the name out of the path. A list of command names would be a second copy of the tree, free to go stale, which is the class of defect this rule refuses in documents. The capture also bounds the union: a command's verbs come from the files its own pattern selected, so a sibling binary in the same repository cannot lend it verbs -- a repository-wide widening in the original resolved a flag-only command to its neighbour's verbs, and a document naming one of them would have passed. NO BUNDLED SET SHIPS IT. The rule needs a pattern describing one tree's layout, and a rule arriving from a set cannot be handed a parameter. A set carrying a layout would either impose one workspace's convention on every inheriting repository or ship a rule that refuses to run. Two limits are stated in the reference and asserted by a test rather than left to be found: a flag taking a separate value hides the verb behind it, and a binary whose name lives in a manifest rather than in its path is not discoverable by a path pattern. Closes #55. --- Cargo.lock | 11 + Cargo.toml | 5 + docs/REFERENCE.md | 111 ++++++++- src/commands.rs | 595 ++++++++++++++++++++++++++++++++++++++++++++++ src/config.rs | 79 +++++- src/guard/mod.rs | 13 +- src/main.rs | 1 + src/scan.rs | 189 ++++++++++++++- tests/scan_cli.rs | 274 +++++++++++++++++++++ 9 files changed, 1271 insertions(+), 7 deletions(-) create mode 100644 src/commands.rs diff --git a/Cargo.lock b/Cargo.lock index 8029cce..bbf2511 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -517,6 +517,16 @@ dependencies = [ "tree-sitter-language", ] +[[package]] +name = "tree-sitter-go" +version = "0.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8560a4d2f835cc0d4d2c2e03cbd0dde2f6114b43bc491164238d333e28b16ea" +dependencies = [ + "cc", + "tree-sitter-language", +] + [[package]] name = "tree-sitter-language" version = "0.1.7" @@ -597,6 +607,7 @@ dependencies = [ "serde_yaml_ng", "toml", "tree-sitter", + "tree-sitter-go", "tree-sitter-python", "tree-sitter-rust", "unicode-script", diff --git a/Cargo.toml b/Cargo.toml index 6e4b26e..18ccd78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,6 +65,11 @@ serde_yaml_ng = "0.10.0" tree-sitter = "0.26" tree-sitter-rust = "0.24" tree-sitter-python = "0.25" +# The grammar the doc-command resolver needs, and the one language whose +# dispatch this was measured against. Its star count reads marginal and +# mismeasures it: nobody stars a grammar, and it carries 11.2M downloads with +# 4.7M in the last ninety days. +tree-sitter-go = "0.25" [profile.release] strip = true diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index c64cbc0..7397310 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -610,6 +610,114 @@ of its letters. It is not a check on which language the prose is written in — it never was: `en` and `de` would both admit exactly Latin, which is why the field names scripts and not languages. +### `commands-resolve` — a command a reader would run + +The third resolver, and the last of the three things a document asserts about a +tree. `links-resolve` resolves a path a reader would **click**; +`anchors-resolve` a value a reader would **believe**; this one a command a +reader would **run**. All three are prose that happens to be checkable, and +before they existed all three failed the same way: silently, forever, with every +gate in every repository still green. + +The defect it was built from, measured rather than imagined. A `README.md` +opened with + +```text +fg-registry credentials +``` + +for as long as the file existed. That command has two verbs and `credentials` +was never one of them — the binary's own error names the alternatives — so the +answer was one invocation away, and nobody invoked it: a reader who trusts the +README has no reason to, and a reader who does not is not reading the README. + +```toml +[rule.doc-commands-resolve] +builtin = "commands-resolve" +message = "Name a verb the command dispatches on." +command_sources = ["cmd/{}/*.go", "scripts/{}.rs"] + +[rule.doc-commands-resolve.files] +glob = ["*.md"] +``` + +**`command_sources` is a pattern, not a table of names**, and that is the whole +design. `{}` stands for the command's name and is captured out of the path, so +the convention stays in the repository that has one and nothing about any +workspace's layout is compiled into the binary. A list of command names would be +a second copy of the tree, free to go stale, which is the class of defect this +rule exists to refuse in documents. The capture also **bounds** the union: a +command's verbs are read from the files its own pattern selected and no others, +so a sibling binary in the same repository cannot lend it verbs. + +**It parses the dispatch. It does not run `--help`.** Running the binary needs a +build — warm locally, cold in CI, and a gate that needs the network is one that +gets skipped — and it invites the far worse mistake of resolving a verb by +*running* it: `fg-registry sync` in a document would be "verified" by +fast-forwarding thirty-nine submodules. Only the help text is safe to execute +for, and help text is prose too; a doc comment drifts from the switch below it +exactly as the README drifted. The switch **is** the verb list: `case "sync":` +is not a description of what the command accepts, it is the mechanism by which +it accepts it, and it cannot be stale without also being broken. + +**A command must agree with itself before it judges anyone.** A verb list read +wrong is worse than no verb list: it produces confident findings against +documents that were right. Measured — the first run of the implementation this +ports reported 38 findings, one binary supplied 22 of them by dispatching in a +form the parser could not read, and every one of those was false and read as +real. So a command judges documents only when two independent readings of its +own sources agree: the string labels of its dispatch, and the verbs its own +usage block names about itself. When they disagree the command is **counted, +named and skipped**, never guessed at — and the count is printed every run, +because a check that read four commands out of a hundred otherwise reads exactly +like one that read them all. + +```text +doc-commands-resolve: 3 command(s) discovered, 2 judged, 1 skipped +doc-commands-resolve: not judged: session (no dispatch this parse can read) +``` + +**Zero judged is exit `1`, not a pass.** A renamed directory, a typo in the +glob, and a grammar that stopped matching all arrive in the same state, and +without this they are indistinguishable from a tree whose every documented verb +resolves. + +A dispatch is read with tree-sitter — already in this binary for the comment +checks — and is recognised structurally rather than by a list of subject +spellings: + +| condition | what it rejects | +|---|---| +| it dispatches on something | a Go tagless `switch {`, whose arms are booleans and not verbs | +| at least two branches match string literals | a match over an enum, whose variants are not things a reader types | +| a catch-all branch exists | a lookup that never has to answer for a word it does not know | + +Go and Rust. A third language is a grammar dependency and one row. + +What it deliberately does not do: + +- **Only code spans**, fenced or inline, and only where the command is the + **first token** of the span. An invocation begins with the binary; a sentence + that happens to contain the same two words in a row does not, and neither does + a column of an ASCII diagram. Both were false findings on the first run, and + narrowing to the shape of an actual instruction is what keeps a gate from + crying wrong — a gate that cries wrong gets waived. +- **No bundled set ships it.** The rule needs a `command_sources` pattern that + describes one tree's layout, and a rule arriving from a set cannot be handed a + parameter. A set carrying a layout would either impose one workspace's + convention on every inheriting repository or ship a rule that refuses to run. + +Two limits worth knowing before adopting it: + +- A flag that takes a **separate** value hides the verb behind it: + `fg-registry --workspace here sync` reads `here`. Nothing in the text + distinguishes a flag's value from a verb — only the command's own flag table + does, and reading that is a second parse with a second way to be wrong. + `--flag=value` is unambiguous and passes through. +- A binary whose name lives in a manifest rather than in its path is not + discoverable by a path pattern. `src/bin/{}.rs` works; a single-binary crate + whose name is set in `Cargo.toml` over `src/main.rs` does not. + ## `uphold guard` — the guards `uphold guard --stage STAGE` runs the guards that have something to say at @@ -793,7 +901,8 @@ later has been reporting a clean tree the whole way. The same mechanism holds be `exclude_cfg_test` is read only by the content searches (`regexp`, `values` — its job is dropping a matched *line* inside a `#[cfg(test)]` block, and no other check has one), `require_any_link` / `allow_outside_repo` are read only -by `links-resolve`, and `require_any_anchor` only by `anchors-resolve`. +by `links-resolve`, `require_any_anchor` only by `anchors-resolve`, and +`command_sources` only by `commands-resolve`. **Which bytes a guard reads: the index, unless a push says otherwise.** At a push there is no index at all — the artifact is the pushed commit's whole tree diff --git a/src/commands.rs b/src/commands.rs new file mode 100644 index 0000000..c4eb2df --- /dev/null +++ b/src/commands.rs @@ -0,0 +1,595 @@ +//! A document that tells a reader to run ` ` must name a verb the +//! command dispatches on. +//! +//! The sibling of `links-resolve`, for the other half of what a document asserts +//! about a tree. That one resolves a path a reader would CLICK; this resolves a +//! command a reader would RUN. Both are prose that happens to be checkable, and +//! before either existed both failed the same way: silently, forever, with every +//! gate in every repository still green. +//! +//! The defect it was built from, measured rather than imagined. A `README.md` +//! opened with `fg-registry credentials` for as long as the file existed. That +//! command has two verbs and `credentials` was never one of them -- the binary's +//! own error names the alternatives -- so the answer was one invocation away. +//! Nobody invoked it, because a reader who trusts the README has no reason to and +//! a reader who does not is not reading the README. +//! +//! WHY THE DISPATCH AND NOT `--help`. +//! +//! Running the binary needs a build -- warm locally, cold in CI, and a gate that +//! needs the network is a gate that gets skipped -- and it invites the far worse +//! mistake of resolving a verb by RUNNING it: `fg-registry sync` in a document +//! would be "verified" by fast-forwarding thirty-nine submodules. Only the help +//! text is safe to execute for, and help text is prose too. A doc comment drifts +//! from the switch below it exactly as the README drifted. +//! +//! The switch IS the verb list. `case "sync":` is not a description of what the +//! command accepts; it is the mechanism by which it accepts it, and it cannot be +//! stale without also being broken. +//! +//! WHY A COMMAND MUST AGREE WITH ITSELF BEFORE IT JUDGES ANYONE. +//! +//! A verb list read wrong is worse than no verb list: it produces confident +//! findings against documents that were right. Measured on the implementation +//! this ports -- the first run reported 38 findings, one binary supplied 22 of +//! them by dispatching in a form the parser could not read while an unrelated +//! switch in the same binary supplied a plausible-looking verb list, and every +//! one of those findings was false and read as real. +//! +//! So a command judges documents only when two independent readings of its own +//! sources agree: the string labels of its dispatch, and the verbs its own usage +//! block names about itself. When they disagree the parse is not trusted, and the +//! command is counted, named and skipped rather than guessed at. That count is +//! reported every run, which is what keeps a check that read four commands out of +//! a hundred from reading like one that read all of them. +//! +//! WHY A GRAMMAR AND NOT A LINE MATCHER. The implementation this replaces read +//! dispatches with a regex over lines and an allow-list of eight subject +//! spellings taken off dispatches in one workspace. Both halves of its measured +//! false-finding rate come from that: a tagless `switch {` is a line the matcher +//! cannot read, and a subject list read off one tree is a list that describes one +//! tree. A grammar answers the question structurally, and the crate was already +//! in this binary for [`crate::comments`]. + +use std::collections::{BTreeMap, BTreeSet}; + +use regex::Regex; +use tree_sitter::{Node, Parser}; + +/// The languages a command's dispatch can be read from. +/// +/// Two, and the second is here to keep the first from being a special case: a +/// third is a grammar dependency and one row in [`Language::of_path`]. Go +/// because it is the language the defect was measured in; Rust because this +/// binary is written in it, so the rule can be pointed at its own tree. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Language { + Go, + Rust, +} + +/// Where the string labels of a dispatch live in one grammar. +/// +/// A table rather than a function per language, for the reason +/// `parameterize-do-not-enumerate` gives: the two readings differ only by node +/// names, so a second walk written out for the second language would be one unit +/// with an unextracted parameter -- and the next grammar would need an author +/// instead of a row. +#[derive(Debug, Clone, Copy)] +struct Shape { + /// The node that dispatches: a Go expression switch, a Rust match. + switch: &'static str, + /// The field holding what is being dispatched ON. Absent on a Go tagless + /// `switch {`, which is exactly the form that has to be skipped rather than + /// read wrong. + subject: &'static str, + /// One branch of it. + arm: &'static str, + /// The field of a branch holding the labels it matches. + labels: &'static str, + /// The literal node a label is, when the label is a string. + string: &'static str, + /// The node a catch-all branch is, where the grammar gives it one of its + /// own. Go spells `default:` as a different node from `case`; Rust spells it + /// as an ordinary arm whose pattern binds instead of matching a literal, and + /// is `None` here because the empty-label test already finds it. + default: Option<&'static str>, +} + +impl Language { + /// The language of a repository-relative path, or `None` for a file whose + /// dispatch this cannot read. + pub(crate) fn of_path(path: &str) -> Option { + match path.rsplit_once('.') { + Some((_, "go")) => Some(Self::Go), + Some((_, "rs")) => Some(Self::Rust), + _ => None, + } + } + + fn grammar(self) -> tree_sitter::Language { + match self { + Self::Go => tree_sitter_go::LANGUAGE.into(), + Self::Rust => tree_sitter_rust::LANGUAGE.into(), + } + } + + const fn shape(self) -> Shape { + match self { + Self::Go => Shape { + switch: "expression_switch_statement", + subject: "value", + arm: "expression_case", + labels: "value", + string: "interpreted_string_literal", + default: Some("default_case"), + }, + Self::Rust => Shape { + switch: "match_expression", + subject: "value", + arm: "match_arm", + labels: "pattern", + string: "string_literal", + default: None, + }, + } + } +} + +/// A string literal with its quotes taken off. +/// +/// The grammars name the content node differently and both wrap it in the +/// delimiters, so the delimiters are trimmed rather than the content node looked +/// up by a name that would be a third thing to keep in [`Shape`]. +fn unquoted(node: Node<'_>, source: &str) -> Option { + let text = node.utf8_text(source.as_bytes()).ok()?; + let inner = text + .strip_prefix('"') + .and_then(|rest| rest.strip_suffix('"'))?; + (!inner.is_empty() && !inner.contains('\\')).then(|| inner.to_owned()) +} + +/// Every string label of one branch. +fn labels_of(arm: Node<'_>, shape: Shape, source: &str) -> Vec { + let Some(pattern) = arm.child_by_field_name(shape.labels) else { + return Vec::new(); + }; + let mut found = Vec::new(); + let mut cursor = pattern.walk(); + let mut stack = vec![pattern]; + while let Some(node) = stack.pop() { + if node.kind() == shape.string { + if let Some(text) = unquoted(node, source) { + found.push(text); + } + continue; + } + stack.extend(node.children(&mut cursor)); + } + found +} + +/// The verbs one dispatch offers, or `None` if this node is not a dispatch. +/// +/// Three structural conditions, and each one is a false finding the line matcher +/// this replaces produced: +/// +/// * it dispatches on SOMETHING. A Go tagless `switch {` is a chain of boolean +/// arms, not a lookup, and reading its arms as verbs is where 22 of one run's +/// 38 findings came from. +/// * at least two branches match STRING literals. A match over an enum is a +/// state machine, and its variants are not things a reader types. +/// * a catch-all branch exists. A command dispatching on a word the user chose +/// has to answer for a word it does not know, and a lookup with no default is +/// almost never one. +/// +/// The conditions are deliberately structural rather than a list of subject +/// spellings. An allow-list of subjects read off one workspace's dispatches is a +/// list that describes that workspace, and the agreement gate above this is what +/// disciplines a generous reading -- not a narrow one that silently misses a +/// dispatch spelled a way nobody had seen yet. +fn dispatch_labels(node: Node<'_>, shape: Shape, source: &str) -> Option> { + node.child_by_field_name(shape.subject)?; + let mut cursor = node.walk(); + let mut verbs: BTreeSet = BTreeSet::new(); + let mut arms = 0usize; + let mut catch_all = false; + let mut stack = vec![node]; + while let Some(current) = stack.pop() { + for child in current.children(&mut cursor) { + if shape.default == Some(child.kind()) { + catch_all = true; + continue; + } + if child.kind() == shape.arm { + let labels = labels_of(child, shape, source); + if labels.is_empty() { + catch_all = true; + } else { + arms += 1; + verbs.extend(labels); + } + continue; + } + // A Go switch keeps its cases in a block, and a Rust match keeps its + // arms in a `match_block`. Descending rather than naming the block + // kind keeps [`Shape`] to what actually differs. + if child.kind() != shape.switch { + stack.push(child); + } + } + } + (arms >= 2 && catch_all).then_some(verbs) +} + +/// Every verb the sources of one command dispatch on. +/// +/// Unioned across the declared sources, because a `main` that delegates keeps +/// its dispatch one package over and both files are the command's own. What +/// bounds the union is the DECLARATION: a pattern names one command's sources, +/// so a sibling binary in the same repository cannot lend it verbs. That +/// collision is not hypothetical -- a repository-wide widening in the +/// implementation this ports resolved a flag-only command to the verbs of its +/// neighbour, and a document naming one of them would have passed. +pub(crate) fn dispatched(sources: &[(String, String)]) -> BTreeSet { + let mut verbs: BTreeSet = BTreeSet::new(); + for (path, text) in sources { + let Some(language) = Language::of_path(path) else { + continue; + }; + let mut parser = Parser::new(); + if parser.set_language(&language.grammar()).is_err() { + continue; + } + let Some(tree) = parser.parse(text.as_bytes(), None) else { + continue; + }; + let shape = language.shape(); + let mut cursor = tree.root_node().walk(); + let mut stack = vec![tree.root_node()]; + while let Some(node) = stack.pop() { + if node.kind() == shape.switch { + if let Some(found) = dispatch_labels(node, shape, text) { + verbs.extend(found); + } + } + stack.extend(node.children(&mut cursor)); + } + } + // `-h` and `--help` sit in the same switch as the real verbs and are not + // verbs a document would be wrong to name. + verbs.retain(|verb| !verb.starts_with('-')); + verbs +} + +/// What may sit to the left of a command name and still leave the text an +/// invocation: a comment leader, a shell prompt, and the path the binary is +/// reached by. Anything else means the name is a word in a sentence. +fn lead() -> &'static Regex { + static LEAD: std::sync::OnceLock = std::sync::OnceLock::new(); + #[expect( + clippy::unwrap_used, + reason = "a literal pattern that compiles or does not, decided at the first call and not by any input" + )] + LEAD.get_or_init(|| Regex::new(r"^[\s>]*(?://+|#+|\*)?\s*[$%]?\s*(?:\./)?").unwrap()) +} + +/// ` [flags] `, anchored where the lead ends. +fn invocation(command: &str) -> Option { + Regex::new(&format!( + r"^{}(?:\.sh)?\s+(?:-{{1,2}}[A-Za-z0-9][^\s]*\s+)*([a-z][a-z0-9-]*)", + regex::escape(command) + )) + .ok() +} + +/// The verbs a command's own sources name in a USAGE position. +/// +/// The second reading, and deliberately the same first-token rule the documents +/// are held to, applied to the command's own usage block. A sentence such as +/// "Command fg-registry operates on the workspace" is not in a usage position and +/// contributes nothing; `//\tfg-registry sync [options]` in a doc comment is, and +/// contributes `sync`. +pub(crate) fn documented(command: &str, sources: &[(String, String)]) -> BTreeSet { + let Some(pattern) = invocation(command) else { + return BTreeSet::new(); + }; + let mut verbs = BTreeSet::new(); + for (_, text) in sources { + for line in text.lines() { + if let Some(verb) = first_verb(line, &pattern) { + verbs.insert(verb); + } + } + } + verbs +} + +/// The verb this text invokes, if it invokes one at all. +/// +/// Anchored after the lead, because an invocation BEGINS with the binary. +/// Searching anywhere in the text is what matched a command name inside an +/// ordinary sentence and inside a column of an ASCII diagram, and both were false +/// findings on the first run of the implementation this ports. +fn first_verb(text: &str, pattern: &Regex) -> Option { + let start = lead().find(text).map_or(0, |found| found.end()); + let rest = text.get(start..)?; + let captured = pattern.captures(rest)?; + captured.get(1).map(|verb| verb.as_str().to_owned()) +} + +/// One place a document tells a reader to run something. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Mention { + /// 1-based, matching every other line number this crate reports. + pub line: u64, + pub command: String, + pub verb: String, +} + +/// `(line number, code)` for every fenced-block line and every inline span. +/// +/// Prose is not read at all. An instruction is written in a code span, and a +/// sentence that happens to contain the same two words in a row is not an +/// instruction -- matching it is how this check would earn a blanket waiver. +fn code_spans(text: &str) -> Vec<(u64, String)> { + let mut spans: Vec<(u64, String)> = Vec::new(); + let mut fence: Option = None; + for (index, line) in text.lines().enumerate() { + let number = index as u64 + 1; + let trimmed = line.trim_start(); + let opener = trimmed + .starts_with("```") + .then_some('`') + .or_else(|| trimmed.starts_with("~~~").then_some('~')); + if let Some(marker) = opener { + match fence { + None => fence = Some(marker), + Some(open) if open == marker => fence = None, + Some(_) => {} + } + continue; + } + if fence.is_some() { + spans.push((number, line.to_owned())); + continue; + } + // An inline span is non-greedy and single-line: one that opened and never + // closed on its line is not a span. + let mut rest = line; + while let Some(open) = rest.find('`') { + let Some(after) = rest.get(open + 1..) else { + break; + }; + let Some(close) = after.find('`') else { + break; + }; + if let Some(code) = after.get(..close) { + spans.push((number, code.to_owned())); + } + let Some(remainder) = after.get(close + 1..) else { + break; + }; + rest = remainder; + } + } + spans +} + +/// Every invocation a document names, for the commands whose verbs are trusted. +pub(crate) fn mentions(text: &str, commands: &BTreeMap>) -> Vec { + let mut found = Vec::new(); + for command in commands.keys() { + // Cheap rejection first. Most documents name no command at all, and + // compiling a pattern per command per file would be the cost this + // avoids by asking a substring question first. + if !text.contains(command.as_str()) { + continue; + } + let Some(pattern) = invocation(command) else { + continue; + }; + for (line, code) in code_spans(text) { + if let Some(verb) = first_verb(&code, &pattern) { + found.push(Mention { + line, + command: command.clone(), + verb, + }); + } + } + } + found.sort_by(|left, right| { + left.line + .cmp(&right.line) + .then_with(|| left.command.cmp(&right.command)) + }); + found +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sources(path: &str, text: &str) -> Vec<(String, String)> { + vec![(path.to_owned(), text.to_owned())] + } + + fn verbs(named: &[&str]) -> BTreeSet { + named.iter().map(|verb| (*verb).to_owned()).collect() + } + + fn offering(command: &str, named: &[&str]) -> BTreeMap> { + let mut commands = BTreeMap::new(); + commands.insert(command.to_owned(), verbs(named)); + commands + } + + const GO_DISPATCH: &str = r#" +package main + +func main() { + args := os.Args[1:] + switch args[0] { + case "sync": + sync() + case "services": + services() + default: + usage() + } +} +"#; + + #[test] + fn a_go_switch_on_a_subcommand_is_the_verb_list() { + let found = dispatched(&sources("cmd/fg-registry/main.go", GO_DISPATCH)); + assert_eq!(found, verbs(&["services", "sync"])); + } + + /// The form that supplied 22 of one run's 38 false findings. + /// + /// A tagless `switch {` is a chain of boolean arms and not a lookup, so its + /// arms are not verbs. The line matcher this replaces could not see the + /// difference; the grammar reports the missing subject. + #[test] + fn a_tagless_switch_offers_no_verbs_rather_than_wrong_ones() { + let tagless = r" +package main + +func main() { + switch { + case ready(): + go run() + case waiting(): + wait() + default: + stop() + } +} +"; + assert!(dispatched(&sources("cmd/session/main.go", tagless)).is_empty()); + } + + /// A match over an enum is a state machine, and its variants are not things + /// a reader types into a shell. + #[test] + fn a_match_with_no_string_labels_is_not_a_dispatch() { + let states = r" +fn step(state: State) -> State { + match state { + State::Idle => State::Running, + State::Running => State::Done, + _ => state, + } +} +"; + assert!(dispatched(&sources("src/main.rs", states)).is_empty()); + } + + /// A lookup on a word the user chose has to answer for a word it does not + /// know. One without a catch-all is almost never a dispatch. + #[test] + fn a_string_match_with_no_catch_all_is_not_a_dispatch() { + let exhaustive = r#" +fn label(kind: &str) -> &str { + match kind { + "a" => "first", + "b" => "second", + other => other, + } +} +"#; + // `other` binds rather than matching a literal, so it IS the catch-all + // and this one resolves. The negative case is the same body with the + // final arm removed, which does not compile in Rust and so cannot be + // the shape a real dispatch takes. + assert!(!dispatched(&sources("src/main.rs", exhaustive)).is_empty()); + } + + #[test] + fn a_rust_match_on_a_subcommand_is_the_verb_list() { + let rust = r#" +fn run(first: &str) -> Result<()> { + match first { + "scan" => scan(), + "guard" => guard(), + other => Err(unknown(other)), + } +} +"#; + let verbs = dispatched(&sources("src/main.rs", rust)); + assert!(verbs.contains("scan"), "{verbs:?}"); + assert!(verbs.contains("guard"), "{verbs:?}"); + } + + #[test] + fn a_usage_block_reads_under_the_same_first_token_rule() { + let go = r" +package main + +// fg-registry sync [options] +// fg-registry services +// +// Command fg-registry operates on the workspace's own registries. +func main() {} +"; + let named = documented("fg-registry", &sources("cmd/fg-registry/main.go", go)); + assert!(named.contains("sync"), "{named:?}"); + assert!(named.contains("services"), "{named:?}"); + // The prose sentence is not in a usage position, so it contributes + // nothing. Without the first-token rule it would contribute `operates`, + // the two readings would disagree, and the command would skip itself + // out over a sentence that was perfectly correct. + assert!(!named.contains("operates"), "{named:?}"); + } + + #[test] + fn only_a_code_span_is_read_and_only_at_its_first_token() { + let document = "\ +Read it with: + +``` +fg-registry credentials +``` + +The very fg-registry session it captures with is not an instruction, and +neither is `see fg-registry sync` in the middle of a span. +"; + let commands = offering("fg-registry", &["sync"]); + let found = mentions(document, &commands); + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!( + found.first().map(|one| (one.verb.as_str(), one.line)), + Some(("credentials", 4)) + ); + } + + /// A flag before the verb does not hide it, and a flag that takes a + /// SEPARATE value does. + /// + /// Stated as a test rather than left to be discovered, because the failure + /// direction matters: `--workspace here sync` reads `here` as the verb, and + /// `here` is not in the verb list, so the rule reports a document that was + /// right. Nothing in the text distinguishes a flag's value from a verb -- + /// only the command's own flag table does, and reading that is a second + /// parse with a second way to be wrong. The narrow form is what is + /// supported; `--flag=value` is unambiguous and passes through. + #[test] + fn a_flag_before_the_verb_does_not_hide_it_unless_it_takes_a_value() { + let commands = offering("fg-registry", &["sync"]); + for span in [ + "`fg-registry --verbose sync`", + "`fg-registry --workspace=here sync`", + ] { + let found = mentions(&format!("{span}\n"), &commands); + assert_eq!(found.len(), 1, "{span}: {found:?}"); + assert!(found.iter().any(|m| m.verb == "sync"), "{span}: {found:?}"); + } + let valued = mentions("`fg-registry --workspace here sync`\n", &commands); + assert!( + valued.iter().any(|m| m.verb == "here"), + "the limit, asserted so it cannot change silently: {valued:?}" + ); + } +} diff --git a/src/config.rs b/src/config.rs index bd86dc6..c47ab8b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -549,6 +549,24 @@ pub(crate) struct Rule { #[serde(default)] pub require_any_anchor: Option, + /// commands-resolve: where a command's own sources live, as globs in which + /// `{}` stands for the command's name. + /// + /// A PATTERN and not a table of names, and the difference is the whole + /// design. `["*/cmd/{}/**/*.go"]` says what a command looks like in this + /// tree; a list of command names would be a second copy of the tree, free to + /// go stale, which is the class of defect this rule exists to refuse. The + /// convention stays in the repository that has one, and nothing about one + /// workspace's layout is compiled into the binary. + /// + /// The capture also BOUNDS the union. A command's verbs are read from the + /// files its own pattern selects and no others, so a sibling binary in the + /// same repository cannot lend it verbs -- a widening that read the whole + /// repository resolved a flag-only command to its neighbour's verbs, and a + /// document naming one of them would have passed. + #[serde(default)] + pub command_sources: Option>, + // -- settings the built-ins read ---------------------------------------- // // These were environment variables, every one of them, because git-guards @@ -663,6 +681,7 @@ impl Rule { require_any_link: None, allow_outside_repo: None, require_any_anchor: None, + command_sources: None, private_owners: None, private_owners_from: None, public_repos: None, @@ -783,6 +802,11 @@ impl Rule { self.allow_outside_repo.unwrap_or(false) } + /// The `command_sources` patterns, empty where none were written. + pub(crate) fn command_sources(&self) -> &[String] { + self.command_sources.as_deref().unwrap_or_default() + } + pub(crate) fn require_any_anchor(&self) -> bool { self.require_any_anchor.unwrap_or(false) } @@ -921,6 +945,50 @@ impl Rule { /// be renamed: while `kind` decided, `max_lines` beside `kind = "pattern"` /// was a limit that looked enforced and was not. And a rule that names no /// place runs nowhere, which reads exactly like a rule that passes. + /// Hold `commands-resolve` to the one field it cannot work without. + /// + /// Split out of `validate` rather than written inline, because it is the + /// only one of the three resolver knobs that is REQUIRED: the link and + /// anchor floors are refused where they are written beside a check that + /// cannot read them, and this is refused where it is MISSING as well. + fn validate_command_sources(&self) -> Result<()> { + // The command resolver's own knob, and unlike the two above it is + // REQUIRED rather than merely exclusive. A `commands-resolve` with no + // pattern discovers no command, judges nothing, and reports a clean + // tree -- which is the shape every check here is written to refuse. + if self.builtin() == Some("commands-resolve") { + let patterns = self.command_sources.as_deref().unwrap_or_default(); + if patterns.is_empty() { + return Err(Fatal::new(format!( + "rule {:?}: `commands-resolve` needs `command_sources`, one or more \ + globs in which `{{}}` stands for a command's name -- \ + `command_sources = [\"*/cmd/{{}}/**/*.go\"]`. Without one it \ + discovers no command, judges nothing, and reports a clean tree.", + self.id + ))); + } + for pattern in patterns { + if pattern.matches("{}").count() != 1 { + return Err(Fatal::new(format!( + "rule {:?}: `command_sources` entry {pattern:?} does not contain \ + exactly one `{{}}`. The placeholder is what names the command, and \ + a pattern without one selects files belonging to no command while \ + a pattern with two names two.", + self.id + ))); + } + } + } else if self.command_sources.is_some() { + return Err(Fatal::new(format!( + "rule {:?}: `command_sources` is read by the `commands-resolve` built-in \ + and nothing else -- on this rule the field would be read by nothing and \ + would look like configuration that works", + self.id + ))); + } + Ok(()) + } + fn validate(&self) -> Result<()> { let set: Vec<&str> = [ self.regexp.is_some().then_some("regexp"), @@ -1098,19 +1166,24 @@ impl Rule { ))); } + self.validate_command_sources()?; + // A built-in that reads files and names a hook belongs to the guard: // `seams()` routes it there, and `guard::evaluate` has no arm for this // one. It would be installed, collected by `at_hook`, counted inside // "N guard(s) passed", and never run -- the same defect the refusals // below name, arriving through the one door they leave open. - if self.builtin() == Some("anchors-resolve") && !self.hooks().is_empty() { + if matches!(self.builtin(), Some("anchors-resolve" | "commands-resolve")) + && !self.hooks().is_empty() + { return Err(Fatal::new(format!( - "rule {:?}: `anchors-resolve` reads the tree rather than what git is about \ + "rule {:?}: `{}` reads the tree rather than what git is about \ to do, so it runs in `uphold scan` and at no git hook. `git.hooks` here \ would install a seam that never dispatches it, and the rule would be \ counted as having passed.\n\ Drop `git.hooks`; the `uphold-scan` hook is what runs it at a hook.", - self.id + self.id, + self.builtin().unwrap_or_default() ))); } diff --git a/src/guard/mod.rs b/src/guard/mod.rs index 79875c0..c023f73 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -70,6 +70,12 @@ pub(crate) const EVERY_BUILTIN: &[&str] = &[ // against a parsed YAML/TOML/JSON document, which is by definition a check // a content search cannot make. "anchors-resolve", + // The third resolver, for the last of the three things a document asserts + // about a tree: `links-resolve` a path a reader would CLICK, + // `anchors-resolve` a value a reader would BELIEVE, this a command a reader + // would RUN. Scan-dispatched like both, and the comparison is against a + // parsed dispatch, which no content search reaches. + "commands-resolve", ]; /// The moment a guard is being asked about. @@ -427,7 +433,10 @@ mod tests { // config can declare, validate, install, and never run. let source = include_str!("mod.rs"); for id in EVERY_BUILTIN { - if matches!(*id, "links-resolve" | "anchors-resolve") { + if matches!( + *id, + "links-resolve" | "anchors-resolve" | "commands-resolve" + ) { // Read the tree rather than git, so `scan` dispatches them and // this match never sees them. continue; @@ -437,7 +446,7 @@ mod tests { "{id} is in EVERY_BUILTIN with no dispatch arm" ); } - assert_eq!(EVERY_BUILTIN.len(), 15); + assert_eq!(EVERY_BUILTIN.len(), 16); } #[test] diff --git a/src/main.rs b/src/main.rs index ca59e4b..0d0f202 100644 --- a/src/main.rs +++ b/src/main.rs @@ -75,6 +75,7 @@ mod anchors; mod audit; mod catalog; mod check; +mod commands; mod comments; mod config; mod engine; diff --git a/src/scan.rs b/src/scan.rs index fd49800..9d1889c 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -8,12 +8,40 @@ use std::sync::OnceLock; use regex::Regex; use unicode_script::{Script, UnicodeScript}; -use crate::config::{Check, Policy, Rule}; +use crate::config::{Check, Files, Policy, Rule}; use crate::engine::{self, Hit, Query}; use crate::error::{Fatal, Result}; use crate::report::{body_for, Failure}; use crate::selection::{normalize_rel, not_text_paths, Selection}; +/// The command name a `command_sources` pattern captures out of a path. +/// +/// Built from the two halves the placeholder splits the pattern into, so the +/// name is read from the same string that selected the file. `*` and `**` become +/// what they mean to a glob rather than what they mean to a regex, and the +/// placeholder becomes one path segment -- a command's name is a directory or a +/// file stem, never a path. +fn command_name_pattern(before: &str, after: &str) -> Option { + fn as_regex(part: &str) -> String { + let mut out = String::new(); + let mut rest = part; + while let Some(index) = rest.find('*') { + if let Some(literal) = rest.get(..index) { + out.push_str(®ex::escape(literal)); + } + let doubled = rest.get(index..index + 2) == Some("**"); + out.push_str(if doubled { ".*" } else { "[^/]*" }); + let Some(remainder) = rest.get(index + if doubled { 2 } else { 1 }..) else { + return out; + }; + rest = remainder; + } + out.push_str(®ex::escape(rest)); + out + } + Regex::new(&format!("^{}([^/]+){}$", as_regex(before), as_regex(after))).ok() +} + /// Explains a baseline entry that no longer matches. /// /// Separate from the rule's own message, which explains the prohibition -- a @@ -111,6 +139,7 @@ impl<'a> Scan<'a> { failures.extend(match rule.builtin().unwrap_or_default() { "links-resolve" => self.link_failures(rule)?, "anchors-resolve" => self.anchor_failures(rule)?, + "commands-resolve" => self.command_failures(rule)?, // A guard built-in's `[rule.files]` is not read by // nothing: `guard::scope::in_file_scope` reads it, to // scope the guard to part of the tree. So the question @@ -621,6 +650,164 @@ impl<'a> Scan<'a> { Ok(vec![Failure::new(&rule.id, rule.message(), body)]) } + // -- documented commands ------------------------------------------------ + + /// The sources of every command one `command_sources` pattern discovers. + /// + /// The pattern selects the files and the `{}` names the command, so the two + /// halves cannot disagree: a file is a command's source exactly when the + /// pattern that named the command selected it. A table of names beside a + /// glob would be two statements of one fact, which is the shape this rule + /// exists to refuse in documents. + fn command_sources(&self, rule: &Rule) -> Result>> { + let mut discovered: BTreeMap> = BTreeMap::new(); + for pattern in rule.command_sources() { + let Some((before, after)) = pattern.split_once("{}") else { + continue; + }; + // The concrete glob the selection machinery is asked for, with the + // placeholder widened to one path segment. Selection is reused + // rather than reimplemented so a source file is found under the + // same ignore rules, the same symlink policy and the same + // unreadable-path accounting as every other file this scan reads. + let mut probe = Rule::synthetic(&rule.id, Check::Builtin); + probe.files = Some(Files { + glob: vec![format!("{before}*{after}")], + ..Files::default() + }); + let name_of = command_name_pattern(before, after); + for relative in self.select(&probe)? { + let Some(name) = name_of + .as_ref() + .and_then(|matcher| matcher.captures(&relative)) + .and_then(|captured| captured.get(1)) + .map(|found| found.as_str().to_owned()) + else { + continue; + }; + let text = match std::fs::read_to_string(self.root.join(&relative)) { + Ok(text) => text, + // A source that is not text declares no dispatch. An I/O + // failure is a file NOBODY READ, which `link_failures` + // separates here for the same reason. + Err(error) if error.kind() == std::io::ErrorKind::InvalidData => continue, + Err(error) => return Err(Fatal::at(&self.root.join(&relative), error)), + }; + discovered.entry(name).or_default().push((relative, text)); + } + } + Ok(discovered) + } + + /// Fail every document that tells a reader to run a verb no command offers. + /// + /// The agreement gate is the half that decides whether this is usable at + /// all. A command judges documents only when its dispatch and its own usage + /// block tell the same story; when they disagree the parse is not trusted + /// and the command is counted, named and skipped. That count is printed + /// every run, because a check that read four commands out of a hundred and + /// says nothing about the other ninety-six reads exactly like one that read + /// them all. + fn command_failures(&self, rule: &Rule) -> Result> { + let discovered = self.command_sources(rule)?; + let mut trusted: BTreeMap> = BTreeMap::new(); + let mut skipped: Vec = Vec::new(); + + for (name, sources) in &discovered { + let dispatched = crate::commands::dispatched(sources); + if dispatched.is_empty() { + skipped.push(format!("{name} (no dispatch this parse can read)")); + continue; + } + let disagreed: Vec = crate::commands::documented(name, sources) + .into_iter() + .filter(|verb| !dispatched.contains(verb)) + .collect(); + if !disagreed.is_empty() { + skipped.push(format!( + "{name} (its own usage names {}, which its parsed dispatch does not offer)", + disagreed.join(", ") + )); + continue; + } + trusted.insert(name.clone(), dispatched); + } + + // Said on every run, clean or not. The denominator is the difference + // between "every documented verb resolves" and "every documented verb + // this could read resolves", and only one of those is what happened. + println!( + "{}: {} command(s) discovered, {} judged, {} skipped", + rule.id, + discovered.len(), + trusted.len(), + skipped.len() + ); + for note in &skipped { + println!("{}: not judged: {note}", rule.id); + } + + if trusted.is_empty() { + // Not a pass. Zero commands judged is the state a broken pattern, a + // renamed directory and a grammar that stopped matching all arrive + // in, and it is indistinguishable from a clean tree without this. + return Ok(vec![Failure::new( + &rule.id, + rule.message(), + format!( + "discovered {} command(s) and could read the verbs of none, so no \ + document was judged. Either `command_sources` no longer describes \ + this tree, or every dispatch it found is in a form this parse cannot \ + read -- both of which report a clean tree while checking nothing.", + discovered.len() + ), + )]); + } + + let files = self.select(rule)?; + let mut hits: Vec = Vec::new(); + let mut detailed: Vec = Vec::new(); + for relative in &files { + let text = match std::fs::read_to_string(self.root.join(relative)) { + Ok(text) => text, + Err(error) if error.kind() == std::io::ErrorKind::InvalidData => continue, + Err(error) => return Err(Fatal::at(&self.root.join(relative), error)), + }; + for mention in crate::commands::mentions(&text, &trusted) { + let Some(offered) = trusted.get(&mention.command) else { + continue; + }; + if offered.contains(&mention.verb) { + continue; + } + let listed: Vec<&str> = offered.iter().map(String::as_str).collect(); + hits.push(Hit { + path: relative.clone(), + line: Some(mention.line), + text: format!("{} {}", mention.command, mention.verb), + }); + detailed.push(format!( + "{relative}:{}: tells a reader to run `{} {}`, and {} dispatches on: {}", + mention.line, + mention.command, + mention.verb, + mention.command, + listed.join(", ") + )); + } + } + + if hits.is_empty() { + return Ok(Vec::new()); + } + let body = if self.redact() { + crate::report::redacted_body(&hits) + } else { + detailed.join("\n") + }; + Ok(vec![Failure::new(&rule.id, rule.message(), body)]) + } + // -- anchors ------------------------------------------------------------ /// Fail every anchor whose source is gone, whose key is gone, or whose diff --git a/tests/scan_cli.rs b/tests/scan_cli.rs index 8decf33..b4e25ab 100644 --- a/tests/scan_cli.rs +++ b/tests/scan_cli.rs @@ -1588,3 +1588,277 @@ fn an_anchor_rule_wired_to_a_git_hook_is_refused() { stderr(&output) ); } + +// --- commands-resolve ------------------------------------------------------ +// +// The third resolver. `links-resolve` resolves a path a reader would click, +// `anchors-resolve` a value a reader would believe, this a command a reader +// would run. Every case here is about the half that decides whether the rule is +// usable at all: a verb list read wrong produces confident findings against +// documents that were right, so a command judges nothing until two readings of +// its own sources agree. + +/// A Go command with a real dispatch and a usage block that agrees with it. +const FG_REGISTRY: &str = r#"package main + +// fg-registry sync [options] +// fg-registry services + +func main() { + args := os.Args[1:] + switch args[0] { + case "sync": + sync() + case "services": + services() + default: + usage() + } +} +"#; + +const COMMANDS_POLICY: &str = r#" +[rule.doc-commands-resolve] +builtin = "commands-resolve" +message = "Name a verb the command dispatches on." +command_sources = ["cmd/{}/*.go"] + +[rule.doc-commands-resolve.files] +glob = ["*.md"] +"#; + +#[test] +fn a_document_naming_a_verb_the_command_does_not_dispatch_on_is_refused() { + // The defect this was built from, reproduced: a README opened with a verb + // the binary had never had, and every gate in every repository stayed green + // for as long as the file existed. + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write(&root, "cmd/fg-registry/main.go", FG_REGISTRY); + write( + &root, + "README.md", + "Read the metadata with:\n\n```\nfg-registry credentials\n```\n", + ); + + let output = scan(&root); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("README.md:4"), "{text}"); + assert!(text.contains("fg-registry credentials"), "{text}"); + // The verbs it DOES have, because a refusal that names the wrong verb and + // not the right ones sends the reader to the source anyway. + assert!(text.contains("services, sync"), "{text}"); +} + +#[test] +fn a_verb_the_command_really_dispatches_on_passes() { + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write(&root, "cmd/fg-registry/main.go", FG_REGISTRY); + write( + &root, + "README.md", + "Fast-forward with `fg-registry sync`.\n", + ); + + let output = scan(&root); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + // The denominator, printed on every run. "Every documented verb resolves" + // and "every documented verb this could read resolves" are different + // sentences, and only one of them is what happened. + assert!( + stdout(&output).contains("1 command(s) discovered, 1 judged, 0 skipped"), + "{}", + stdout(&output) + ); +} + +#[test] +fn a_command_whose_two_readings_disagree_judges_nothing_and_says_so() { + // The agreement gate. This command's usage block names a verb its dispatch + // does not offer, so one of the two readings is wrong and there is no way to + // tell which -- the rule refuses to guess, and refuses to condemn a document + // on the strength of a parse it cannot trust. + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write( + &root, + "cmd/fg-registry/main.go", + r#"package main + +// fg-registry credentials + +func main() { + switch os.Args[1] { + case "sync": + sync() + case "services": + services() + default: + usage() + } +} +"#, + ); + write( + &root, + "README.md", + "Read the metadata with:\n\n```\nfg-registry credentials\n```\n", + ); + + let output = scan(&root); + // Exit 1, and NOT because the document was judged: nothing could be judged, + // and zero commands judged is the state a broken pattern and a grammar that + // stopped matching both arrive in. + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let out = stdout(&output); + assert!(out.contains("0 judged, 1 skipped"), "{out}"); + assert!(out.contains("its own usage names credentials"), "{out}"); +} + +#[test] +fn a_tagless_switch_is_skipped_rather_than_read_as_a_verb_list() { + // The form that supplied 22 of one run's 38 false findings in the + // implementation this ports. Its arms are booleans, not verbs, and the + // grammar is what tells them apart. + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write( + &root, + "cmd/session/main.go", + r"package main + +func main() { + switch { + case ready(): + run() + default: + stop() + } +} +", + ); + write(&root, "README.md", "Try `session claim`.\n"); + + let output = scan(&root); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let out = stdout(&output); + assert!(out.contains("no dispatch this parse can read"), "{out}"); + // And the document was not condemned on the strength of it. + assert!( + !stderr(&output).contains("session claim"), + "{}", + stderr(&output) + ); +} + +#[test] +fn a_command_name_in_prose_is_not_an_instruction() { + // Only a code span, and only at its first token. An invocation begins with + // the binary; a sentence that happens to contain the same two words in a row + // does not, and matching one is how this rule would earn a blanket waiver. + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write(&root, "cmd/fg-registry/main.go", FG_REGISTRY); + write( + &root, + "README.md", + "The very fg-registry credentials it captures with are held elsewhere.\n\ + See `the fg-registry credentials note` for where.\n", + ); + + let output = scan(&root); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_pattern_that_discovers_nothing_is_refused_rather_than_reported_clean() { + // Zero commands is the state a renamed directory, a typo in the glob and a + // grammar that stopped matching all arrive in, and without this it is + // indistinguishable from a tree whose every documented verb resolves. + let root = workspace(); + write(&root, "policy/principles.toml", COMMANDS_POLICY); + write(&root, "README.md", "Nothing here.\n"); + + let output = scan(&root); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + assert!( + stderr(&output).contains("no document was judged"), + "{}", + stderr(&output) + ); +} + +#[test] +fn the_resolver_refuses_to_load_without_a_pattern_to_discover_with() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + r#" +[rule.doc-commands-resolve] +builtin = "commands-resolve" +message = "Name a real verb." + +[rule.doc-commands-resolve.files] +glob = ["*.md"] +"#, + ); + write(&root, "README.md", "x\n"); + + let output = scan(&root); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("needs `command_sources`"), + "{}", + stderr(&output) + ); +} + +#[test] +fn a_pattern_with_no_placeholder_names_no_command_and_is_refused() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + &COMMANDS_POLICY.replace("cmd/{}/*.go", "cmd/fg-registry/*.go"), + ); + write(&root, "cmd/fg-registry/main.go", FG_REGISTRY); + write(&root, "README.md", "x\n"); + + let output = scan(&root); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("exactly one"), + "{}", + stderr(&output) + ); +} + +#[test] +fn the_pattern_is_read_by_this_built_in_and_no_other() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + r#" +[rule.no-shouting] +regexp = "SHOUTING" +message = "do not shout" +command_sources = ["cmd/{}/*.go"] + +[rule.no-shouting.files] +glob = ["*.md"] +"#, + ); + write(&root, "README.md", "x\n"); + + let output = scan(&root); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("read by nothing"), + "{}", + stderr(&output) + ); +} From 1535c9e60455aae5825a842c8e886c181383cd7d Mon Sep 17 00:00:00 2001 From: HackingGate Date: Thu, 20 Aug 2026 22:54:49 +0900 Subject: [PATCH 2/2] Read the dispatch Go actually spells, and take usage lines rather than sentences Two defects, both found by pointing the rule at a real workspace of nineteen commands rather than at its own fixtures. A TAGLESS SWITCH IS NOT AUTOMATICALLY A CHAIN OF BOOLEANS. "Dispatches on something" was never the property worth testing. Go spells a dispatch that also guards its own argument count as `switch { case len(args) > 0 && args[0] == "serve": }`, and refusing to read it left a real command judged against a SUB-dispatch found in another file of the same package -- the `record|last` of one of its own verbs -- which produced three confident findings against a README that was right. That is the exact failure this rule exists to prevent, arriving through the guard against it. The condition is now: every branch names a literal. A tagless switch whose arms are `ready()` and `waiting()` still contributes nothing, which is the case the guard was written for. A SENTENCE THAT BEGINS WITH THE COMMAND'S NAME IS NOT A USAGE CLAIM. The two readings fail in opposite directions: a false document finding accuses a file that was right, while a false USAGE verb costs coverage in silence -- the readings disagree and the command then judges nothing at all. A command called `session` collected `for` and `in` out of three ordinary comments ("session in an already-running browser", "session for the ref"), disagreed with its own dispatch over words that are not verbs, and skipped itself out of every document in its repository. Any command whose name is also an English word has that waiting for it. So the usage reading now takes only lines whose remainder still reads as an invocation: a flag, a placeholder, an alternation, or nothing. Applied to the usage half and deliberately not to the documents, where narrowing would drop real findings. --- src/commands.rs | 135 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 116 insertions(+), 19 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index c4eb2df..e2714ef 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -174,14 +174,22 @@ fn labels_of(arm: Node<'_>, shape: Shape, source: &str) -> Vec { /// Three structural conditions, and each one is a false finding the line matcher /// this replaces produced: /// -/// * it dispatches on SOMETHING. A Go tagless `switch {` is a chain of boolean -/// arms, not a lookup, and reading its arms as verbs is where 22 of one run's -/// 38 findings came from. -/// * at least two branches match STRING literals. A match over an enum is a +/// * at least two branches carry STRING literals. A match over an enum is a /// state machine, and its variants are not things a reader types. /// * a catch-all branch exists. A command dispatching on a word the user chose /// has to answer for a word it does not know, and a lookup with no default is /// almost never one. +/// * where it dispatches on NOTHING -- a Go tagless `switch {` -- every branch +/// must carry a literal. This is the condition that took the longest to get +/// right, and it was measured rather than reasoned. A tagless switch whose +/// arms are `ready()` and `waiting()` is a chain of booleans and reading it as +/// a verb list is where 22 of one run's 38 findings came from. But a tagless +/// switch whose every arm is `len(args) > 0 && args[0] == "serve"` IS the +/// dispatch, spelled the way Go spells one that also guards its own argument +/// count -- and refusing to read it left a real command judged against a +/// SUB-dispatch found elsewhere in the same binary, which produced three +/// confident findings against a README that was right. "Has a subject" was +/// never the property worth testing; "every branch names a literal" is. /// /// The conditions are deliberately structural rather than a list of subject /// spellings. An allow-list of subjects read off one workspace's dispatches is a @@ -189,10 +197,11 @@ fn labels_of(arm: Node<'_>, shape: Shape, source: &str) -> Vec { /// disciplines a generous reading -- not a narrow one that silently misses a /// dispatch spelled a way nobody had seen yet. fn dispatch_labels(node: Node<'_>, shape: Shape, source: &str) -> Option> { - node.child_by_field_name(shape.subject)?; + let on_something = node.child_by_field_name(shape.subject).is_some(); let mut cursor = node.walk(); let mut verbs: BTreeSet = BTreeSet::new(); - let mut arms = 0usize; + let mut labelled = 0usize; + let mut unlabelled = 0usize; let mut catch_all = false; let mut stack = vec![node]; while let Some(current) = stack.pop() { @@ -204,9 +213,10 @@ fn dispatch_labels(node: Node<'_>, shape: Shape, source: &str) -> Option, shape: Shape, source: &str) -> Option= 2 && catch_all).then_some(verbs) + let readable = labelled >= 2 && catch_all && (on_something || unlabelled == 0); + readable.then_some(verbs) } /// Every verb the sources of one command dispatch on. @@ -297,25 +308,58 @@ pub(crate) fn documented(command: &str, sources: &[(String, String)]) -> BTreeSe let mut verbs = BTreeSet::new(); for (_, text) in sources { for line in text.lines() { - if let Some(verb) = first_verb(line, &pattern) { - verbs.insert(verb); + if let Some((verb, rest)) = invoked(line, &pattern) { + if usage_shaped(&rest) { + verbs.insert(verb); + } } } } verbs } -/// The verb this text invokes, if it invokes one at all. +/// Whether what follows the verb still reads as an invocation rather than a +/// sentence. +/// +/// Asked of the USAGE reading and deliberately not of the documents. The two +/// readings fail in opposite directions: a false document finding accuses a file +/// that was right, while a false USAGE verb costs coverage silently -- the two +/// readings disagree, and the command then judges nothing at all. +/// +/// Measured on a real tree rather than supposed. A command named `session` +/// collected `for` and `in` out of three ordinary comments -- "session in an +/// already-running browser", "session for the ref" -- disagreed with its own +/// dispatch over words that are not verbs, and skipped itself out of every +/// document in its repository. Any command whose name is also an English word +/// has that failure waiting for it. +/// +/// A usage line names arguments. What may follow a verb is a flag, a +/// placeholder, an alternation, or nothing; a run of ordinary lowercase words is +/// prose, and prose was never a usage claim to disagree with. +fn usage_shaped(rest: &str) -> bool { + rest.split_whitespace().all(|token| { + token.starts_with(['-', '<', '[', '{', '(']) + || token == "|" + || token == "..." + || token + .chars() + .all(|character| !character.is_ascii_lowercase()) + }) +} + +/// The verb this text invokes and what follows it, if it invokes one at all. /// /// Anchored after the lead, because an invocation BEGINS with the binary. /// Searching anywhere in the text is what matched a command name inside an /// ordinary sentence and inside a column of an ASCII diagram, and both were false /// findings on the first run of the implementation this ports. -fn first_verb(text: &str, pattern: &Regex) -> Option { +fn invoked(text: &str, pattern: &Regex) -> Option<(String, String)> { let start = lead().find(text).map_or(0, |found| found.end()); - let rest = text.get(start..)?; - let captured = pattern.captures(rest)?; - captured.get(1).map(|verb| verb.as_str().to_owned()) + let line = text.get(start..)?; + let captured = pattern.captures(line)?; + let verb = captured.get(1)?; + let after = line.get(verb.end()..).unwrap_or_default().to_owned(); + Some((verb.as_str().to_owned(), after)) } /// One place a document tells a reader to run something. @@ -390,7 +434,7 @@ pub(crate) fn mentions(text: &str, commands: &BTreeMap> continue; }; for (line, code) in code_spans(text) { - if let Some(verb) = first_verb(&code, &pattern) { + if let Some((verb, _)) = invoked(&code, &pattern) { found.push(Mention { line, command: command.clone(), @@ -447,11 +491,41 @@ func main() { assert_eq!(found, verbs(&["services", "sync"])); } + /// A tagless switch whose every arm names a literal IS the dispatch. + /// + /// Go spells a dispatch that also guards its own argument count this way, + /// and it is not rare. Measured on a real binary: refusing to read it left + /// the command judged against a SUB-dispatch found in another file of the + /// same package -- `record|last`, belonging to one of its verbs -- and the + /// rule reported three confident findings against a README that was right. + #[test] + fn a_tagless_switch_whose_arms_all_name_a_literal_is_still_a_dispatch() { + let guarded = r#" +package main + +func main() { + args := os.Args[1:] + switch { + case len(args) > 0 && args[0] == "serve": + serve(args[1:]) + case len(args) > 0 && args[0] == "login-audit": + loginAudit(args[1:]) + default: + run(args) + } +} +"#; + assert_eq!( + dispatched(&sources("cmd/session/main.go", guarded)), + verbs(&["login-audit", "serve"]) + ); + } + /// The form that supplied 22 of one run's 38 false findings. /// - /// A tagless `switch {` is a chain of boolean arms and not a lookup, so its - /// arms are not verbs. The line matcher this replaces could not see the - /// difference; the grammar reports the missing subject. + /// A tagless `switch {` whose arms carry no literal at all is a chain of + /// booleans and not a lookup, so its arms are not verbs. The line matcher + /// this replaces could not see the difference. #[test] fn a_tagless_switch_offers_no_verbs_rather_than_wrong_ones() { let tagless = r" @@ -544,6 +618,29 @@ func main() {} assert!(!named.contains("operates"), "{named:?}"); } + /// The usage reading takes usage lines and not sentences that start with + /// the command's name. + /// + /// Measured on a real tree: a command called `session` collected `for` and + /// `in` out of ordinary comments, disagreed with its own dispatch over words + /// that are not verbs, and skipped itself out of every document in its + /// repository. Any command whose name is also an English word has that + /// waiting for it. + #[test] + fn a_sentence_beginning_with_the_command_name_is_not_a_usage_claim() { + let prose = r"package main + +// session in an already-running browser, using the vaulted passkey and +// session for the ref. The real implementation drives the pool. +// +// session serve +// session login-audit +func main() {} +"; + let named = documented("session", &sources("cmd/session/main.go", prose)); + assert_eq!(named, verbs(&["login-audit", "serve"])); + } + #[test] fn only_a_code_span_is_read_and_only_at_its_first_token() { let document = "\