From d34caca8a4e45a0cab125a5367ed1ee699aee8bf Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:54:35 +0000 Subject: [PATCH 1/9] feat(cli): add usage diff for spec compatibility checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A CLI is a public API, and a spec is the only machine-readable statement of what that API is — which makes "did this release break somebody" a question about two files rather than about a changelog somebody remembered to write. clap#918 has been open since 2017 asking for the export this reads. `usage diff old.usage.kdl new.usage.kdl` classifies every difference into one of three categories, drawn by one rule: - breaking: a command line that worked before now fails, binds differently, or resolves to a different value - compatible: the interface gained something or relaxed a rule, so every old command line still means what it meant - metadata: nothing about parsing moved — help text, headings, hidden-ness, effect, deprecation It exits 1 on a breaking change, so a release job gates on it, and either spec may be `-`, so the released file can be compared against what the binary being built says about itself. Two silences are deliberate. `version` and `long_version` are never reported: a release bumps them, and a check that fires on every release does not get left switched on — tak sets `spec.version = None` by hand today for exactly this reason. Derived strings are never reported either, because they restate what the declarations already say. `unknown_flags` is compared where it is declared rather than as the value in force at each command. Comparing effective values reported one edited root node 54 times against hk's fixture, once per descendant that inherited it. `lint`'s `--format` enum moves to a shared module, since `diff` answers in the same two formats and two copies is how the two drift apart. Co-Authored-By: Claude Opus 5 --- cli/assets/fig.ts | 37 + cli/assets/usage.1 | 43 + cli/src/cli/diff.rs | 2409 ++++++++++++++++++++++++++++++ cli/src/cli/mod.rs | 2 + cli/tests/diff.rs | 165 ++ cli/usage.usage.kdl | 15 + docs/.vitepress/config.mts | 1 + docs/cli/diff.md | 111 ++ docs/cli/reference/commands.json | 87 ++ docs/cli/reference/diff.md | 53 + docs/cli/reference/index.md | 1 + 11 files changed, 2924 insertions(+) create mode 100644 cli/src/cli/diff.rs create mode 100644 cli/tests/diff.rs create mode 100644 docs/cli/diff.md create mode 100644 docs/cli/reference/diff.md diff --git a/cli/assets/fig.ts b/cli/assets/fig.ts index 06851f8a6..24d99f27d 100644 --- a/cli/assets/fig.ts +++ b/cli/assets/fig.ts @@ -161,6 +161,43 @@ const completionSpec: Fig.Spec = { isVariadic: true, }, }, + { + name: "diff", + description: + "Compare two usage specs and report what changed about the interface", + options: [ + { + name: ["-f", "--format"], + description: "Output format", + isRepeatable: false, + args: { + name: "format", + suggestions: ["text", "json"], + }, + }, + { + name: ["-b", "--breaking"], + description: "Report only breaking changes", + isRepeatable: false, + }, + { + name: "--exit-zero", + description: "Exit 0 even when there are breaking changes", + isRepeatable: false, + }, + ], + args: [ + { + name: "old", + description: + 'The spec as it was, typically the released one, use "-" to read from stdin', + }, + { + name: "new", + description: 'The spec as it is now, use "-" to read from stdin', + }, + ], + }, { name: ["exec", "x"], description: diff --git a/cli/assets/usage.1 b/cli/assets/usage.1 index 4d8a39797..6f4fb325d 100644 --- a/cli/assets/usage.1 +++ b/cli/assets/usage.1 @@ -26,6 +26,9 @@ Generate shell completion candidates for a partial command line \fIAliases: \fRcw .RE .TP +\fBdiff\fR +Compare two usage specs and report what changed about the interface +.TP \fBexec\fR Execute a script, parsing args and exposing them as environment variables .RS @@ -155,6 +158,46 @@ Current word index .TP \fB\fR User's input from the command line +.SH "USAGE DIFF" +Compare two usage specs and report what changed about the interface + +Findings are grouped into breaking changes (a command line that used to work +now fails, binds differently, or resolves to a different value), compatible +changes (the interface gained something or relaxed a rule), and metadata +changes (help text, effect, deprecation — nothing about parsing). + +Exits 1 when there is a breaking change, so a release job can gate on it, and +either spec may be "\-": + + mycli \-\-usage\-spec | usage diff released.usage.kdl \- + +`version` is ignored on purpose: a release bumps it, and a check that fires +every release does not get left switched on. +.PP +\fBUsage:\fR usage diff [OPTIONS] +.PP +\fBOptions:\fR +.PP +.TP +\fB\-f, \-\-format\fR \fI\fR +Output format +.RS +\fIDefault: \fRtext +.RE +.TP +\fB\-b, \-\-breaking\fR +Report only breaking changes +.TP +\fB\-\-exit\-zero\fR +Exit 0 even when there are breaking changes +\fBArguments:\fR +.PP +.TP +\fB\fR +The spec as it was, typically the released one, use "\-" to read from stdin +.TP +\fB\fR +The spec as it is now, use "\-" to read from stdin .SH "USAGE EXEC" Execute a script, parsing args and exposing them as environment variables .PP diff --git a/cli/src/cli/diff.rs b/cli/src/cli/diff.rs new file mode 100644 index 000000000..b8eaad4a0 --- /dev/null +++ b/cli/src/cli/diff.rs @@ -0,0 +1,2409 @@ +//! Compare two specs and say what changed about the interface. +//! +//! A CLI is a public API, and a spec is the only machine-readable statement of what +//! that API is — which makes "did this release break somebody" a question about two +//! files rather than a question about a changelog somebody remembered to write. +//! clap#918 has been open since 2017 asking for the export this reads. +//! +//! Every finding lands in one of three categories, and the line between them is one +//! rule: **breaking** means a command line that worked against the old spec now +//! fails, binds differently, or resolves to a different value. **compatible** means +//! the interface gained something or relaxed a rule — every old command line still +//! means what it meant. **metadata** means nothing about parsing moved: help text, +//! headings, declaration order, hidden-ness, `effect`, deprecation notices. +//! +//! Two deliberate silences. `version` and `long_version` are never reported: a +//! release bumps them, and a compatibility check that fires on every release is one +//! nobody leaves running — tak sets `spec.version = None` by hand today for exactly +//! this reason. Derived strings (`usage`, `full_cmd`, `help_first_line`) are not +//! reported either, because they restate what the declarations already say. + +use std::path::PathBuf; + +use usage::{Spec, SpecArg, SpecCommand, SpecFlag}; + +use crate::cli::generate::parse_file_or_stdin; +use crate::cli::OutputFormat; +use usage::spec::choices::SpecChoices; +use usage::spec::config::{SpecConfig, SpecConfigProp, SpecConfigValue}; +use usage::spec::group::SpecGroup; +use usage::spec::unknown_flags::UnknownFlags; + +/// Compare two usage specs and report what changed about the interface +/// +/// Findings are grouped into breaking changes (a command line that used to work +/// now fails, binds differently, or resolves to a different value), compatible +/// changes (the interface gained something or relaxed a rule), and metadata +/// changes (help text, effect, deprecation — nothing about parsing). +/// +/// Exits 1 when there is a breaking change, so a release job can gate on it, and +/// either spec may be "-": +/// +/// mycli --usage-spec | usage diff released.usage.kdl - +/// +/// `version` is ignored on purpose: a release bumps it, and a check that fires +/// every release does not get left switched on. +#[derive(usage_rs::Args)] +#[usage(effect = "read", verbatim_doc_comment)] +pub struct Diff { + /// The spec as it was, typically the released one, use "-" to read from stdin + old: PathBuf, + + /// The spec as it is now, use "-" to read from stdin + new: PathBuf, + + /// Output format + #[usage(long, short, default = "text", value_enum)] + format: OutputFormat, + + /// Report only breaking changes + #[usage(long, short)] + breaking: bool, + + /// Exit 0 even when there are breaking changes + #[usage(long)] + exit_zero: bool, +} + +/// How much a change costs whoever is calling the CLI. +/// +/// Ordered, so a report can put what matters first without a second table: +/// breaking before compatible before metadata. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] +#[serde(rename_all = "lowercase")] +pub enum Category { + /// A command line that worked before now fails, binds differently, or + /// resolves to a different value. + Breaking, + /// The interface gained something, or relaxed a rule. Every old command line + /// still means what it meant. + Compatible, + /// Nothing about parsing moved. + Metadata, +} + +impl std::fmt::Display for Category { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Category::Breaking => write!(f, "breaking"), + Category::Compatible => write!(f, "compatible"), + Category::Metadata => write!(f, "metadata"), + } + } +} + +/// One difference between two specs. +/// +/// The same shape as a lint issue, so a reader who has seen `usage lint --format +/// json` output knows how to read this one. +#[derive(Debug, Clone, serde::Serialize)] +pub struct SpecChange { + pub category: Category, + pub code: String, + pub message: String, + pub location: String, +} + +impl std::fmt::Display for SpecChange { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{} [{}] at {}: {}", + self.category, self.code, self.location, self.message + ) + } +} + +impl usage_rs::Run for Diff { + type Output = miette::Result<()>; + + fn run(self) -> Self::Output { + if self.old.as_os_str() == "-" && self.new.as_os_str() == "-" { + miette::bail!("only one of the two specs can be read from stdin"); + } + let old = parse_file_or_stdin(&self.old)?; + let new = parse_file_or_stdin(&self.new)?; + let mut changes = diff_specs(&old, &new); + if self.breaking { + changes.retain(|c| c.category == Category::Breaking); + } + + match self.format { + OutputFormat::Text => self.print_text(&changes), + OutputFormat::Json => self.print_json(&changes)?, + } + + if !self.exit_zero && changes.iter().any(|c| c.category == Category::Breaking) { + std::process::exit(1); + } + Ok(()) + } +} + +impl Diff { + fn print_text(&self, changes: &[SpecChange]) { + if changes.is_empty() { + if self.breaking { + println!("No breaking changes."); + } else { + println!("No interface changes."); + } + return; + } + + for change in changes { + println!("{change}"); + } + + let count = |category: Category| changes.iter().filter(|c| c.category == category).count(); + println!(); + println!( + "Found {} breaking, {} compatible, {} metadata change(s)", + count(Category::Breaking), + count(Category::Compatible), + count(Category::Metadata), + ); + } + + fn print_json(&self, changes: &[SpecChange]) -> miette::Result<()> { + let json = serde_json::to_string_pretty(changes) + .map_err(|e| miette::miette!("Failed to serialize changes: {}", e))?; + println!("{json}"); + Ok(()) + } +} + +/// Where findings accumulate, so every comparison function is a `&mut self` push +/// rather than a `Vec` returned and concatenated by its caller. +#[derive(Default)] +struct Changes { + changes: Vec, +} + +impl Changes { + fn push(&mut self, category: Category, code: &str, location: &str, message: String) { + self.changes.push(SpecChange { + category, + code: code.to_string(), + message, + location: location.to_string(), + }); + } + + fn breaking(&mut self, code: &str, location: &str, message: String) { + self.push(Category::Breaking, code, location, message); + } + + fn compatible(&mut self, code: &str, location: &str, message: String) { + self.push(Category::Compatible, code, location, message); + } + + fn metadata(&mut self, code: &str, location: &str, message: String) { + self.push(Category::Metadata, code, location, message); + } +} + +/// Compare two specs. +/// +/// Breaking findings come first, then compatible, then metadata; within a category +/// the order is the walk order — root, its flags, its arguments, then each +/// subcommand — which is the order the spec declares them in. +pub fn diff_specs(old: &Spec, new: &Spec) -> Vec { + let mut c = Changes::default(); + + let root = if new.bin.is_empty() { + &new.name + } else { + &new.bin + }; + let root = root.clone(); + + if old.bin != new.bin { + c.breaking( + "bin-changed", + &root, + format!("binary name changed from '{}' to '{}'", old.bin, new.bin), + ); + } + if old.name != new.name { + c.metadata( + "name-changed", + &root, + format!("spec name changed from '{}' to '{}'", old.name, new.name), + ); + } + + match (&old.default_subcommand, &new.default_subcommand) { + (Some(was), None) => c.breaking( + "default-subcommand-removed", + &root, + format!( + "default subcommand '{was}' was removed, so a bare invocation no longer routes" + ), + ), + (None, Some(now)) => c.compatible( + "default-subcommand-added", + &root, + format!("a bare invocation now routes to '{now}'"), + ), + (Some(was), Some(now)) if was != now => c.breaking( + "default-subcommand-changed", + &root, + format!("default subcommand changed from '{was}' to '{now}'"), + ), + _ => {} + } + + if old.multicall && !new.multicall { + c.breaking( + "multicall-removed", + &root, + "multicall was removed, so argv[0] no longer selects an applet".to_string(), + ); + } else if !old.multicall && new.multicall { + c.compatible( + "multicall-added", + &root, + "argv[0] now selects an applet".to_string(), + ); + } + + for id in old.views.keys() { + if !new.views.contains_key(id) { + c.breaking( + "view-removed", + &root, + format!("view '{id}' was removed, so its executable no longer has a spec"), + ); + } + } + for id in new.views.keys() { + if !old.views.contains_key(id) { + c.compatible("view-added", &root, format!("view '{id}' was added")); + } + } + + if old.min_usage_version != new.min_usage_version { + c.metadata( + "min-usage-version-changed", + &root, + format!( + "min_usage_version changed from {} to {}", + option(&old.min_usage_version), + option(&new.min_usage_version) + ), + ); + } + + if [ + old.about != new.about, + old.about_long != new.about_long, + old.about_md != new.about_md, + old.license != new.license, + old.author != new.author, + old.repository != new.repository, + ] + .iter() + .any(|changed| *changed) + { + c.metadata("about-changed", &root, "root metadata changed".to_string()); + } + + // The spec-level declaration, compared once at the root rather than as an effective + // value at every command that inherits it: 54 identical findings for one edited node + // is not a report anybody reads. + diff_unknown_flags(old.unknown_flags, new.unknown_flags, &root, &mut c); + diff_command(&old.cmd, &new.cmd, &root, &mut c); + diff_config(&old.config, &new.config, &root, &mut c); + + // Stable: `sort_by_key` keeps the walk order inside each category, so a reader + // can still follow the tree down while reading the breaking block first. + c.changes.sort_by_key(|change| change.category); + c.changes +} + +fn diff_command(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + diff_names(old, new, path, c); + diff_command_props(old, new, path, c); + diff_flags(old, new, path, c); + diff_args(&old.args, &new.args, path, c); + diff_groups(&old.groups, &new.groups, path, c); + diff_mounts(old, new, path, c); + diff_subcommands(old, new, path, c); +} + +fn diff_names(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + let was: Vec = old + .aliases + .iter() + .chain(&old.hidden_aliases) + .cloned() + .collect(); + let now: Vec = new + .aliases + .iter() + .chain(&new.hidden_aliases) + .cloned() + .collect(); + for alias in only_in(&was, &now) { + c.breaking( + "alias-removed", + path, + format!("alias '{alias}' was removed"), + ); + } + for alias in only_in(&now, &was) { + c.compatible("alias-added", path, format!("alias '{alias}' was added")); + } + // A visible alias that became hidden still parses; only help and completion + // stop offering it. + for alias in only_in(&old.aliases, &new.aliases) { + if new.hidden_aliases.contains(alias) { + c.metadata( + "alias-hidden", + path, + format!("alias '{alias}' is no longer shown in help"), + ); + } + } +} + +fn diff_command_props(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + if !old.subcommand_required && new.subcommand_required { + c.breaking( + "subcommand-now-required", + path, + "a subcommand is now required, so a bare invocation fails".to_string(), + ); + } else if old.subcommand_required && !new.subcommand_required { + c.compatible( + "subcommand-no-longer-required", + path, + "a bare invocation is now accepted".to_string(), + ); + } + + if !old.arg_required_else_help && new.arg_required_else_help { + c.breaking( + "arg-required-else-help-added", + path, + "a bare invocation now prints help instead of running".to_string(), + ); + } else if old.arg_required_else_help && !new.arg_required_else_help { + c.compatible( + "arg-required-else-help-removed", + path, + "a bare invocation now runs instead of printing help".to_string(), + ); + } + + if old.external_subcommand && !new.external_subcommand { + c.breaking( + "external-subcommand-removed", + path, + "an unmatched word is no longer forwarded".to_string(), + ); + } else if !old.external_subcommand && new.external_subcommand { + c.compatible( + "external-subcommand-added", + path, + "an unmatched word is now forwarded".to_string(), + ); + } + + match (&old.restart_token, &new.restart_token) { + (Some(was), None) => c.breaking( + "restart-token-removed", + path, + format!("restart token '{was}' was removed, so the words after it are no longer a fresh command line"), + ), + (None, Some(now)) => c.compatible( + "restart-token-added", + path, + format!("restart token '{now}' was added"), + ), + (Some(was), Some(now)) if was != now => c.breaking( + "restart-token-changed", + path, + format!("restart token changed from '{was}' to '{now}'"), + ), + _ => {} + } + + diff_unknown_flags(old.unknown_flags, new.unknown_flags, path, c); + + if !old.args_conflicts_with_subcommands && new.args_conflicts_with_subcommands { + c.breaking( + "args-conflicts-with-subcommands-added", + path, + "arguments and a subcommand can no longer appear together".to_string(), + ); + } + if !old.subcommand_precedence_over_arg && new.subcommand_precedence_over_arg { + c.breaking( + "subcommand-precedence-added", + path, + "a word matching a subcommand name now routes instead of binding as an argument" + .to_string(), + ); + } + if old.disable_help_flag != new.disable_help_flag { + let (code, category, message) = if new.disable_help_flag { + ( + "help-flag-disabled", + Category::Breaking, + "--help is no longer accepted", + ) + } else { + ( + "help-flag-enabled", + Category::Compatible, + "--help is now accepted", + ) + }; + c.push(category, code, path, message.to_string()); + } + if old.disable_version_flag != new.disable_version_flag { + let (code, category, message) = if new.disable_version_flag { + ( + "version-flag-disabled", + Category::Breaking, + "--version is no longer accepted", + ) + } else { + ( + "version-flag-enabled", + Category::Compatible, + "--version is now accepted", + ) + }; + c.push(category, code, path, message.to_string()); + } + + if !old.hide && new.hide { + c.metadata( + "command-hidden", + path, + "command is no longer documented".to_string(), + ); + } else if old.hide && !new.hide { + c.metadata( + "command-unhidden", + path, + "command is now documented".to_string(), + ); + } + + if effect_of(&old.effect) != effect_of(&new.effect) { + c.metadata( + "effect-changed", + path, + format!( + "effect changed from {} to {}", + effect_of(&old.effect), + effect_of(&new.effect) + ), + ); + } + + diff_deprecation( + old.deprecated.as_deref(), + new.deprecated.as_deref(), + path, + "command", + c, + ); + + if [ + old.help != new.help, + old.help_long != new.help_long, + old.help_md != new.help_md, + old.before_help != new.before_help, + old.after_help != new.after_help, + old.help_heading != new.help_heading, + ] + .iter() + .any(|changed| *changed) + { + c.metadata("help-changed", path, "help text changed".to_string()); + } +} + +fn diff_flags(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + // Which new flags an old one has already been paired with, so a rename is not also + // reported as an addition. + let mut paired: Vec = vec![false; new.flags.len()]; + + for was in &old.flags { + // By internal name first, which is what the derive and the KDL both key on. + if let Some(position) = new.flags.iter().position(|f| f.name == was.name) { + paired[position] = true; + diff_flag(was, &new.flags[position], path, c); + continue; + } + // Then by spelling: the name is not something a caller can type, so a flag whose + // spellings moved onto a differently-named declaration was renamed rather than + // removed. Compared in full afterwards, so a spelling that really did disappear + // is still reported. + let spellings = flag_spellings(was); + let renamed = new + .flags + .iter() + .enumerate() + .find(|(position, f)| { + !paired[*position] && flag_spellings(f).iter().any(|s| spellings.contains(s)) + }) + .map(|(position, _)| position); + match renamed { + Some(position) => { + paired[position] = true; + let now = &new.flags[position]; + c.metadata( + "flag-renamed", + path, + format!("flag '{}' was renamed to '{}'", was.name, now.name), + ); + diff_flag(was, now, path, c); + } + _ => c.breaking( + "flag-removed", + path, + format!("flag '{}' ({}) was removed", was.name, spellings.join(", ")), + ), + } + } + + for (position, now) in new.flags.iter().enumerate() { + if paired[position] { + continue; + } + let spellings = flag_spellings(now); + if now.required { + c.breaking( + "required-flag-added", + path, + format!( + "required flag '{}' was added, so an invocation without it fails", + spellings.join(", ") + ), + ); + } else { + c.compatible( + "flag-added", + path, + format!("flag '{}' was added", spellings.join(", ")), + ); + } + } +} + +fn diff_flag(old: &SpecFlag, new: &SpecFlag, path: &str, c: &mut Changes) { + let subject = format!("flag '{}'", primary_spelling(new)); + let was = flag_spellings(old); + let now = flag_spellings(new); + for spelling in only_in(&was, &now) { + c.breaking( + "flag-spelling-removed", + path, + format!("{subject} no longer answers to '{spelling}'"), + ); + } + for spelling in only_in(&now, &was) { + c.compatible( + "flag-spelling-added", + path, + format!("{subject} now answers to '{spelling}'"), + ); + } + + if !old.required && new.required { + c.breaking( + "flag-now-required", + path, + format!("{subject} is now required"), + ); + } else if old.required && !new.required { + c.compatible( + "flag-no-longer-required", + path, + format!("{subject} is no longer required"), + ); + } + + match (&old.arg, &new.arg) { + (None, Some(arg)) => c.breaking( + "flag-value-added", + path, + format!( + "{subject} now takes a value <{}>, so the bare flag no longer parses", + arg.name + ), + ), + (Some(arg), None) => c.breaking( + "flag-value-removed", + path, + format!( + "{subject} no longer takes a value, so the word after it binds elsewhere than <{}>", + arg.name + ), + ), + (Some(was), Some(now)) => diff_arg(was, now, path, &subject, c), + (None, None) => {} + } + + if !old.var && new.var { + c.compatible( + "flag-now-variadic", + path, + format!("{subject} now takes more than one value"), + ); + } else if old.var && !new.var { + c.breaking( + "flag-no-longer-variadic", + path, + format!("{subject} takes only one value"), + ); + } + diff_var_max(old.var_max, new.var_max, path, &subject, c); + + if !old.require_equals && new.require_equals { + c.breaking( + "require-equals-added", + path, + format!("{subject} now requires its value attached with '='"), + ); + } else if old.require_equals && !new.require_equals { + c.compatible( + "require-equals-removed", + path, + format!("{subject} now accepts a detached value"), + ); + } + + if old.value_optional && !new.value_optional { + c.breaking( + "flag-value-now-mandatory", + path, + format!("{subject} no longer accepts being given without a value"), + ); + } else if !old.value_optional && new.value_optional { + c.compatible( + "flag-value-now-optional", + path, + format!("{subject} now accepts being given without a value"), + ); + } + + if old.count != new.count { + let (code, message) = if new.count { + ( + "flag-now-counting", + "repeats now count instead of binding a value", + ) + } else { + ("flag-no-longer-counting", "repeats no longer count") + }; + c.breaking(code, path, format!("{subject}: {message}")); + } + + if old.bool_value && !new.bool_value { + c.breaking( + "bool-value-removed", + path, + format!("{subject} no longer accepts '=true' / '=false'"), + ); + } else if !old.bool_value && new.bool_value { + c.compatible( + "bool-value-added", + path, + format!("{subject} now accepts '=true' / '=false'"), + ); + } + + if old.global && !new.global { + c.breaking( + "flag-no-longer-global", + path, + format!("{subject} is no longer accepted on subcommands"), + ); + } else if !old.global && new.global { + c.compatible( + "flag-now-global", + path, + format!("{subject} is now accepted on subcommands"), + ); + } + + match (&old.negate, &new.negate) { + (Some(was), None) => c.breaking( + "negation-removed", + path, + format!("{subject} no longer answers to its negated spelling '{was}'"), + ), + (None, Some(now)) => c.compatible( + "negation-added", + path, + format!("{subject} now answers to '{now}'"), + ), + (Some(was), Some(now)) if was != now => c.breaking( + "negation-changed", + path, + format!("{subject} negated spelling changed from '{was}' to '{now}'"), + ), + _ => {} + } + + match (&old.default_missing, &new.default_missing) { + (Some(was), None) => c.breaking( + "default-missing-removed", + path, + format!("{subject} no longer binds '{was}' when given without a value"), + ), + (None, Some(now)) => c.compatible( + "default-missing-added", + path, + format!("{subject} binds '{now}' when given without a value"), + ), + (Some(was), Some(now)) if was != now => c.breaking( + "default-missing-changed", + path, + format!("{subject} binds '{now}' rather than '{was}' when given without a value"), + ), + _ => {} + } + + diff_defaults(&old.default, &new.default, path, &subject, c); + diff_env( + old.env.as_deref(), + new.env.as_deref(), + &old.env_fallback, + &new.env_fallback, + &old.deprecated_env, + &new.deprecated_env, + path, + &subject, + c, + ); + + diff_restricting( + &old.conflicts, + &new.conflicts, + path, + &subject, + "conflict", + c, + ); + diff_restricting( + &old.requires, + &new.requires, + path, + &subject, + "requirement", + c, + ); + diff_restricting( + &old.required_if, + &new.required_if, + path, + &subject, + "required_if", + c, + ); + diff_restricting( + &required_if_eq(&old.required_if_eq), + &required_if_eq(&new.required_if_eq), + path, + &subject, + "required_if_eq", + c, + ); + diff_restricting( + &required_if_eq(&old.required_if_eq_all), + &required_if_eq(&new.required_if_eq_all), + path, + &subject, + "required_if_eq_all", + c, + ); + diff_restricting( + &old.required_unless_all, + &new.required_unless_all, + path, + &subject, + "required_unless_all", + c, + ); + diff_restricting( + &requires_if(&old.requires_if), + &requires_if(&new.requires_if), + path, + &subject, + "requires_if", + c, + ); + diff_relaxing( + &old.required_unless, + &new.required_unless, + path, + &subject, + "required_unless", + c, + ); + diff_relaxing( + &old.overrides, + &new.overrides, + path, + &subject, + "override", + c, + ); + + if !old.exclusive && new.exclusive { + c.breaking( + "constraint-added", + path, + format!("{subject} is now exclusive, so it cannot appear beside any other flag"), + ); + } else if old.exclusive && !new.exclusive { + c.compatible( + "constraint-removed", + path, + format!("{subject} is no longer exclusive"), + ); + } + + if action_of(old.action) != action_of(new.action) { + c.breaking( + "flag-action-changed", + path, + format!( + "{subject} action changed from {} to {}", + action_of(old.action), + action_of(new.action) + ), + ); + } + + if effect_of(&old.effect) != effect_of(&new.effect) { + c.metadata( + "effect-changed", + path, + format!( + "{subject} effect changed from {} to {}", + effect_of(&old.effect), + effect_of(&new.effect) + ), + ); + } + + diff_deprecation( + old.deprecated.as_deref(), + new.deprecated.as_deref(), + path, + &subject, + c, + ); + + if !old.hide && new.hide { + c.metadata("hidden", path, format!("{subject} is no longer documented")); + } else if old.hide && !new.hide { + c.metadata("unhidden", path, format!("{subject} is now documented")); + } + + if [ + old.help != new.help, + old.help_long != new.help_long, + old.help_md != new.help_md, + old.help_heading != new.help_heading, + ] + .iter() + .any(|changed| *changed) + { + c.metadata("help-changed", path, format!("{subject} help text changed")); + } +} + +fn diff_args(old: &[SpecArg], new: &[SpecArg], path: &str, c: &mut Changes) { + for (position, was) in old.iter().enumerate() { + match new.get(position) { + Some(now) => { + if was.name != now.name { + // The slot still binds the same word, so nothing a caller types + // changes — but the name is what help, docs and `usage exec`'s + // environment variables are keyed on. + c.metadata( + "arg-renamed", + path, + format!( + "argument {} was renamed from <{}> to <{}>", + position + 1, + was.name, + now.name + ), + ); + } + let subject = format!("argument <{}>", now.name); + diff_arg(was, now, path, &subject, c); + } + None => c.breaking( + "arg-removed", + path, + format!("argument <{}> was removed", was.name), + ), + } + } + for now in new.iter().skip(old.len()) { + if now.required { + c.breaking( + "required-arg-added", + path, + format!( + "required argument <{}> was added, so an invocation without it fails", + now.name + ), + ); + } else { + c.compatible( + "arg-added", + path, + format!("argument <{}> was added", now.name), + ); + } + } +} + +/// The shared half of an argument comparison: a positional and a flag's value are +/// the same [`SpecArg`], so `--jobs ` losing its choices reports like `` does. +fn diff_arg(old: &SpecArg, new: &SpecArg, path: &str, subject: &str, c: &mut Changes) { + if !old.required && new.required { + c.breaking( + "arg-now-required", + path, + format!("{subject} is now required"), + ); + } else if old.required && !new.required { + c.compatible( + "arg-no-longer-required", + path, + format!("{subject} is no longer required"), + ); + } + + if old.var && !new.var { + c.breaking( + "arg-no-longer-variadic", + path, + format!("{subject} takes only one value, so extra words are now unexpected"), + ); + } else if !old.var && new.var { + c.compatible( + "arg-now-variadic", + path, + format!("{subject} now takes more than one value"), + ); + } + diff_var_max(old.var_max, new.var_max, path, subject, c); + if old.var_min < new.var_min { + c.breaking( + "var-min-raised", + path, + format!( + "{subject} now needs at least {} values", + new.var_min.unwrap_or(0) + ), + ); + } else if old.var_min > new.var_min { + c.compatible( + "var-min-lowered", + path, + format!( + "{subject} now needs at least {} values", + new.var_min.unwrap_or(0) + ), + ); + } + + if old.value_names != new.value_names { + c.metadata( + "value-names-changed", + path, + format!("{subject} value placeholders changed"), + ); + } + + if old.delimiter != new.delimiter { + c.breaking( + "delimiter-changed", + path, + match (old.delimiter, new.delimiter) { + (Some(was), None) => format!("{subject} no longer splits values on '{was}'"), + (None, Some(now)) => format!("{subject} now splits values on '{now}'"), + (Some(was), Some(now)) => { + format!("{subject} splits values on '{now}' rather than '{was}'") + } + (None, None) => unreachable!("compared unequal"), + }, + ); + } + + if double_dash_of(&old.double_dash) != double_dash_of(&new.double_dash) { + c.breaking( + "double-dash-changed", + path, + format!( + "{subject} double_dash policy changed from {} to {}", + double_dash_of(&old.double_dash), + double_dash_of(&new.double_dash) + ), + ); + } + + if old.value_terminator != new.value_terminator { + c.breaking( + "value-terminator-changed", + path, + format!( + "{subject} value terminator changed from {} to {}", + option(&old.value_terminator), + option(&new.value_terminator) + ), + ); + } + + if old.allow_negative_numbers && !new.allow_negative_numbers { + c.breaking( + "allow-negative-numbers-removed", + path, + format!("{subject} no longer accepts a negative number"), + ); + } else if !old.allow_negative_numbers && new.allow_negative_numbers { + c.compatible( + "allow-negative-numbers-added", + path, + format!("{subject} now accepts a negative number"), + ); + } + + diff_choices(old.choices.as_ref(), new.choices.as_ref(), path, subject, c); + diff_defaults(&old.default, &new.default, path, subject, c); + diff_env( + old.env.as_deref(), + new.env.as_deref(), + &old.env_fallback, + &new.env_fallback, + &old.deprecated_env, + &new.deprecated_env, + path, + subject, + c, + ); + + if old.validate != new.validate { + c.breaking( + "validate-changed", + path, + format!( + "{subject} validation expression changed from {} to {}", + option(&old.validate), + option(&new.validate) + ), + ); + } + + diff_restricting(&old.conflicts, &new.conflicts, path, subject, "conflict", c); + diff_restricting( + &old.requires, + &new.requires, + path, + subject, + "requirement", + c, + ); + diff_restricting( + &old.required_if, + &new.required_if, + path, + subject, + "required_if", + c, + ); + diff_restricting( + &required_if_eq(&old.required_if_eq), + &required_if_eq(&new.required_if_eq), + path, + subject, + "required_if_eq", + c, + ); + diff_restricting( + &required_if_eq(&old.required_if_eq_all), + &required_if_eq(&new.required_if_eq_all), + path, + subject, + "required_if_eq_all", + c, + ); + diff_restricting( + &old.required_unless_all, + &new.required_unless_all, + path, + subject, + "required_unless_all", + c, + ); + diff_relaxing( + &old.required_unless, + &new.required_unless, + path, + subject, + "required_unless", + c, + ); + + if effect_of(&old.effect) != effect_of(&new.effect) { + c.metadata( + "effect-changed", + path, + format!( + "{subject} effect changed from {} to {}", + effect_of(&old.effect), + effect_of(&new.effect) + ), + ); + } + + if !old.hide && new.hide { + c.metadata("hidden", path, format!("{subject} is no longer documented")); + } else if old.hide && !new.hide { + c.metadata("unhidden", path, format!("{subject} is now documented")); + } + + if [ + old.help != new.help, + old.help_long != new.help_long, + old.help_md != new.help_md, + old.help_heading != new.help_heading, + ] + .iter() + .any(|changed| *changed) + { + c.metadata("help-changed", path, format!("{subject} help text changed")); + } +} + +fn diff_choices( + old: Option<&SpecChoices>, + new: Option<&SpecChoices>, + path: &str, + subject: &str, + c: &mut Changes, +) { + let was = accepted_values(old); + let now = accepted_values(new); + let old_strict = old.is_some_and(|ch| ch.strict); + let new_strict = new.is_some_and(|ch| ch.strict); + + if !old_strict && new_strict { + c.breaking( + "choices-now-strict", + path, + format!( + "{subject} now rejects values outside its declared set ({})", + now.join(", ") + ), + ); + } else if old_strict && !new_strict { + c.compatible( + "choices-no-longer-strict", + path, + format!("{subject} now accepts values outside its declared set"), + ); + } + + // Only a strict set can narrow: where anything is accepted, a value leaving + // the declared list stops being offered rather than stops being accepted. + for value in only_in(&was, &now) { + if new_strict { + c.breaking( + "choice-removed", + path, + format!("{subject} no longer accepts '{value}'"), + ); + } else { + c.metadata( + "choice-unlisted", + path, + format!("{subject} no longer offers '{value}', which it still accepts"), + ); + } + } + for value in only_in(&now, &was) { + c.compatible( + "choice-added", + path, + format!("{subject} now accepts '{value}'"), + ); + } + + if old.is_some_and(|ch| ch.ignore_case) && !new.is_some_and(|ch| ch.ignore_case) { + c.breaking( + "choices-case-sensitive", + path, + format!("{subject} now compares its values case-sensitively"), + ); + } else if !old.is_some_and(|ch| ch.ignore_case) && new.is_some_and(|ch| ch.ignore_case) { + c.compatible( + "choices-ignore-case", + path, + format!("{subject} now compares its values without regard to case"), + ); + } +} + +fn diff_groups(old: &[SpecGroup], new: &[SpecGroup], path: &str, c: &mut Changes) { + for was in old { + let Some(now) = new.iter().find(|g| g.name == was.name) else { + c.compatible( + "group-removed", + path, + format!( + "group '{}' was removed, so its rule no longer applies", + was.name + ), + ); + continue; + }; + let subject = format!("group '{}'", now.name); + if !was.required && now.required { + c.breaking( + "group-now-required", + path, + format!("{subject} now requires one of its members"), + ); + } else if was.required && !now.required { + c.compatible( + "group-no-longer-required", + path, + format!("{subject} no longer requires one of its members"), + ); + } + if was.multiple && !now.multiple { + c.breaking( + "group-now-exclusive", + path, + format!("{subject} members are now mutually exclusive"), + ); + } else if !was.multiple && now.multiple { + c.compatible( + "group-no-longer-exclusive", + path, + format!("{subject} members may now be given together"), + ); + } + // In an exclusive group a new member is a new conflict; where members may + // appear together, membership only decides what satisfies `required`. + for member in only_in(&now.members, &was.members) { + if now.multiple { + c.metadata( + "group-member-added", + path, + format!("{subject} gained member '{member}'"), + ); + } else { + c.breaking( + "group-member-added", + path, + format!( + "{subject} gained member '{member}', which now conflicts with the rest" + ), + ); + } + } + for member in only_in(&was.members, &now.members) { + if now.required { + c.breaking( + "group-member-removed", + path, + format!("{subject} lost member '{member}', which no longer satisfies it"), + ); + } else { + c.compatible( + "group-member-removed", + path, + format!("{subject} lost member '{member}'"), + ); + } + } + } + for now in new { + if old.iter().any(|g| g.name == now.name) { + continue; + } + let subject = format!("group '{}'", now.name); + if now.required || !now.multiple { + c.breaking( + "group-added", + path, + format!( + "{subject} was added over {}, constraining combinations that were valid", + now.members.join(", ") + ), + ); + } else { + c.metadata( + "group-added", + path, + format!("{subject} was added over {}", now.members.join(", ")), + ); + } + } +} + +fn diff_mounts(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + let was: Vec = old.mounts.iter().map(|m| m.run.clone()).collect(); + let now: Vec = new.mounts.iter().map(|m| m.run.clone()).collect(); + for run in only_in(&was, &now) { + c.breaking( + "mount-removed", + path, + format!("mount '{run}' was removed, so the commands it discovered are gone"), + ); + } + for run in only_in(&now, &was) { + c.compatible("mount-added", path, format!("mount '{run}' was added")); + } +} + +fn diff_subcommands(old: &SpecCommand, new: &SpecCommand, path: &str, c: &mut Changes) { + for (name, was) in &old.subcommands { + let child = format!("{path} {name}"); + match new.subcommands.get(name) { + Some(now) => diff_command(was, now, &child, c), + None => { + // A word that now selects some other command still works: rustup's + // `install` reaching `toolchain install` is a rename, not a removal. + match new.find_subcommand(name) { + Some(covering) => c.metadata( + "cmd-renamed", + path, + format!( + "command '{name}' was renamed to '{}', which still answers to '{name}'", + covering.name + ), + ), + None => { + c.breaking("cmd-removed", path, format!("command '{name}' was removed")) + } + } + } + } + } + for (name, now) in &new.subcommands { + if old.subcommands.contains_key(name) { + continue; + } + if old.find_subcommand(name).is_some() { + // The name used to be an alias of a sibling and is now a command of its + // own: what the word selects changed. + c.breaking( + "cmd-shadows-alias", + path, + format!("command '{name}' now takes a name that was an alias of another command"), + ); + continue; + } + c.compatible( + "cmd-added", + path, + format!("command '{}' was added", now.name), + ); + } +} + +/// The config block is interface too: a property is read from the environment and +/// the command line, and a released CLI that stops reading `MISE_JOBS` broke +/// somebody's shell profile as surely as a removed flag would have. +fn diff_config(old: &SpecConfig, new: &SpecConfig, path: &str, c: &mut Changes) { + for (key, was) in &old.props { + let Some(now) = new.props.get(key) else { + match was.renamed_to.as_deref() { + Some(to) => c.compatible( + "config-prop-renamed", + path, + format!("config property '{key}' was renamed to '{to}'"), + ), + None => c.breaking( + "config-prop-removed", + path, + format!("config property '{key}' was removed"), + ), + } + continue; + }; + diff_config_prop(key, was, now, path, c); + } + for key in new.props.keys() { + if !old.props.contains_key(key) { + c.compatible( + "config-prop-added", + path, + format!("config property '{key}' was added"), + ); + } + } + for name in old.sources.keys() { + if !new.sources.contains_key(name) { + c.breaking( + "config-source-removed", + path, + format!( + "config source '{name}' was removed, so what it supplied is no longer read" + ), + ); + } + } + for name in new.sources.keys() { + if !old.sources.contains_key(name) { + c.compatible( + "config-source-added", + path, + format!("config source '{name}' was added"), + ); + } + } + let old_files: Vec = old.files.iter().map(|f| f.path.clone()).collect(); + let new_files: Vec = new.files.iter().map(|f| f.path.clone()).collect(); + for file in only_in(&old_files, &new_files) { + c.breaking( + "config-file-removed", + path, + format!("config file '{file}' is no longer read"), + ); + } + for file in only_in(&new_files, &old_files) { + c.compatible( + "config-file-added", + path, + format!("config file '{file}' is now read"), + ); + } +} + +fn diff_config_prop( + key: &str, + old: &SpecConfigProp, + new: &SpecConfigProp, + path: &str, + c: &mut Changes, +) { + let subject = format!("config property '{key}'"); + + let old_type = config_type(old); + let new_type = config_type(new); + if old_type != new_type { + c.breaking( + "config-type-changed", + path, + format!("{subject} type changed from {old_type} to {new_type}"), + ); + } + + match (&old.default, &new.default) { + (Some(was), None) => c.breaking( + "config-default-removed", + path, + format!("{subject} no longer defaults to {}", config_value(was)), + ), + (None, Some(now)) => c.compatible( + "config-default-added", + path, + format!("{subject} now defaults to {}", config_value(now)), + ), + (Some(was), Some(now)) if was != now => c.breaking( + "config-default-changed", + path, + format!( + "{subject} default changed from {} to {}", + config_value(was), + config_value(now) + ), + ), + _ => {} + } + + let was_envs = config_envs(old); + let now_envs = config_envs(new); + for name in only_in(&was_envs, &now_envs) { + c.breaking( + "config-env-removed", + path, + format!("{subject} no longer reads ${name}"), + ); + } + for name in only_in(&now_envs, &was_envs) { + c.compatible( + "config-env-added", + path, + format!("{subject} now reads ${name}"), + ); + } + + for alias in only_in(&old.aliases, &new.aliases) { + c.breaking( + "config-alias-removed", + path, + format!("{subject} no longer answers to the key '{alias}'"), + ); + } + for alias in only_in(&new.aliases, &old.aliases) { + c.compatible( + "config-alias-added", + path, + format!("{subject} now answers to the key '{alias}'"), + ); + } + + let was_choices: Vec = old + .choices + .iter() + .map(|ch| config_value(&ch.value)) + .collect(); + let now_choices: Vec = new + .choices + .iter() + .map(|ch| config_value(&ch.value)) + .collect(); + for value in only_in(&was_choices, &now_choices) { + c.breaking( + "config-choice-removed", + path, + format!("{subject} no longer accepts {value}"), + ); + } + for value in only_in(&now_choices, &was_choices) { + c.compatible( + "config-choice-added", + path, + format!("{subject} now accepts {value}"), + ); + } + + if merge_of(&old.merge) != merge_of(&new.merge) { + c.breaking( + "config-merge-changed", + path, + format!( + "{subject} merge policy changed from {} to {}", + merge_of(&old.merge), + merge_of(&new.merge) + ), + ); + } + if scope_of(&old.scope) != scope_of(&new.scope) { + c.breaking( + "config-scope-changed", + path, + format!( + "{subject} scope changed from {} to {}", + scope_of(&old.scope), + scope_of(&new.scope) + ), + ); + } + + if old.optional.unwrap_or(false) && !new.optional.unwrap_or(false) { + c.breaking( + "config-now-required", + path, + format!("{subject} must now have a value"), + ); + } else if !old.optional.unwrap_or(false) && new.optional.unwrap_or(false) { + c.compatible( + "config-now-optional", + path, + format!("{subject} may now be absent"), + ); + } + + diff_deprecation( + old.deprecated.as_deref(), + new.deprecated.as_deref(), + path, + &subject, + c, + ); + + if old.help != new.help || old.long_help != new.long_help { + c.metadata("help-changed", path, format!("{subject} help text changed")); + } +} + +/// Deprecation is an announcement rather than a change of behaviour: the flag still +/// parses, so both directions are metadata. Reported all the same, because a +/// release note wants it and because `usage diff` is where the removal it promises +/// will later show up as breaking. +fn diff_deprecation( + old: Option<&str>, + new: Option<&str>, + path: &str, + subject: &str, + c: &mut Changes, +) { + match (old, new) { + (None, Some(_)) => c.metadata("deprecated", path, format!("{subject} was deprecated")), + (Some(_), None) => c.metadata( + "undeprecated", + path, + format!("{subject} is no longer deprecated"), + ), + _ => {} + } +} + +/// A default a caller was relying on. Adding one fills a hole — nothing was +/// resolved there before — while changing or removing one moves ground the caller +/// was already standing on. +fn diff_defaults(old: &[String], new: &[String], path: &str, subject: &str, c: &mut Changes) { + if old == new { + return; + } + match (old.is_empty(), new.is_empty()) { + (true, false) => c.compatible( + "default-added", + path, + format!("{subject} now defaults to {}", new.join(", ")), + ), + (false, true) => c.breaking( + "default-removed", + path, + format!("{subject} no longer defaults to {}", old.join(", ")), + ), + _ => c.breaking( + "default-changed", + path, + format!( + "{subject} default changed from {} to {}", + old.join(", "), + new.join(", ") + ), + ), + } +} + +#[allow(clippy::too_many_arguments)] +fn diff_env( + old_env: Option<&str>, + new_env: Option<&str>, + old_fallback: &[String], + new_fallback: &[String], + old_deprecated: &[String], + new_deprecated: &[String], + path: &str, + subject: &str, + c: &mut Changes, +) { + let was = env_names(old_env, old_fallback, old_deprecated); + let now = env_names(new_env, new_fallback, new_deprecated); + for name in only_in(&was, &now) { + c.breaking( + "env-removed", + path, + format!("{subject} no longer reads ${name}"), + ); + } + for name in only_in(&now, &was) { + c.compatible("env-added", path, format!("{subject} now reads ${name}")); + } + // Order decides which of several names wins, so a reordering changes what a + // shell with both of them set resolves to. + if was.len() == now.len() && was != now { + c.breaking( + "env-order-changed", + path, + format!("{subject} consults its environment names in a different order"), + ); + } +} + +/// A list whose entries each impose a rule: gaining one rejects command lines that +/// used to be valid, losing one accepts more. +fn diff_restricting( + old: &[String], + new: &[String], + path: &str, + subject: &str, + what: &str, + c: &mut Changes, +) { + for entry in only_in(new, old) { + c.breaking( + "constraint-added", + path, + format!("{subject} gained {what} '{entry}'"), + ); + } + for entry in only_in(old, new) { + c.compatible( + "constraint-removed", + path, + format!("{subject} lost {what} '{entry}'"), + ); + } +} + +/// The mirror of [`diff_restricting`]: entries that relieve a rule, so gaining one +/// accepts more. +fn diff_relaxing( + old: &[String], + new: &[String], + path: &str, + subject: &str, + what: &str, + c: &mut Changes, +) { + for entry in only_in(new, old) { + c.compatible( + "constraint-removed", + path, + format!("{subject} gained {what} '{entry}'"), + ); + } + for entry in only_in(old, new) { + c.breaking( + "constraint-added", + path, + format!("{subject} lost {what} '{entry}'"), + ); + } +} + +/// One `unknown_flags` declaration against its counterpart. +/// +/// Compared where it is written — the spec root, or the command that overrides it — +/// rather than as the value in force at each command. The parser resolves it by +/// inheritance, so comparing effective values reports one edited root node once per +/// descendant, and a child that merely restates what it inherited as a change. +fn diff_unknown_flags( + old: Option, + new: Option, + path: &str, + c: &mut Changes, +) { + let was_strict = matches!(old, Some(UnknownFlags::Error)); + let is_strict = matches!(new, Some(UnknownFlags::Error)); + if !was_strict && is_strict { + c.breaking( + "unknown-flags-strict", + path, + "an undeclared flag is now an error rather than a value".to_string(), + ); + } else if was_strict && !is_strict { + c.compatible( + "unknown-flags-lax", + path, + "an undeclared flag is now a value rather than an error".to_string(), + ); + } +} + +fn diff_var_max( + old: Option, + new: Option, + path: &str, + subject: &str, + c: &mut Changes, +) { + // `None` is unbounded, so it is the largest value rather than the smallest. + let ceiling = |v: Option| v.unwrap_or(usize::MAX); + if ceiling(new) < ceiling(old) { + c.breaking( + "var-max-lowered", + path, + format!( + "{subject} now takes at most {} values", + new.map_or("unlimited".to_string(), |v| v.to_string()) + ), + ); + } else if ceiling(new) > ceiling(old) { + c.compatible( + "var-max-raised", + path, + format!( + "{subject} now takes at most {} values", + new.map_or("unlimited".to_string(), |v| v.to_string()) + ), + ); + } +} + +fn only_in<'a>(these: &'a [String], those: &[String]) -> Vec<&'a String> { + these.iter().filter(|s| !those.contains(s)).collect() +} + +fn flag_spellings(flag: &SpecFlag) -> Vec { + flag.long + .iter() + .chain(&flag.hidden_aliases) + .map(|long| format!("--{long}")) + .chain( + flag.short + .iter() + .chain(&flag.hidden_short_aliases) + .map(|short| format!("-{short}")), + ) + .collect() +} + +/// What to call a flag in a message: its first visible spelling, falling back to +/// the internal name for a flag that has only hidden ones. +fn primary_spelling(flag: &SpecFlag) -> String { + flag.long + .first() + .map(|long| format!("--{long}")) + .or_else(|| flag.short.first().map(|short| format!("-{short}"))) + .unwrap_or_else(|| flag.name.clone()) +} + +fn accepted_values(choices: Option<&SpecChoices>) -> Vec { + let Some(choices) = choices else { + return vec![]; + }; + let mut values = choices.choices.clone(); + for detail in &choices.details { + if !values.contains(&detail.value) { + values.push(detail.value.clone()); + } + for alias in &detail.aliases { + if !values.contains(&alias.value) { + values.push(alias.value.clone()); + } + } + } + values +} + +fn env_names(env: Option<&str>, fallback: &[String], deprecated: &[String]) -> Vec { + env.map(str::to_string) + .into_iter() + .chain(fallback.iter().cloned()) + .chain(deprecated.iter().cloned()) + .collect() +} + +fn required_if_eq(entries: &[usage::spec::arg::SpecRequiredIfEq]) -> Vec { + entries + .iter() + .map(|e| format!("{}={}", e.selector, e.value)) + .collect() +} + +fn requires_if(entries: &[usage::spec::flag::SpecRequiresIf]) -> Vec { + entries + .iter() + .map(|e| format!("{}={}", e.value, e.requires)) + .collect() +} + +fn config_envs(prop: &SpecConfigProp) -> Vec { + // `env` restates `envs[0]` — the spec keeps both so a programmatically built prop + // serializes — so reading both would report the first name twice. + let named = if prop.envs.is_empty() { + prop.env.clone().into_iter().collect() + } else { + prop.envs.clone() + }; + named + .into_iter() + .chain(prop.deprecated_envs.iter().cloned()) + .collect() +} + +fn config_type(prop: &SpecConfigProp) -> String { + match &prop.value_type { + Some(value_type) => value_type.to_string(), + None => prop.data_type.to_string(), + } +} + +fn config_value(value: &SpecConfigValue) -> String { + match value { + SpecConfigValue::Bool(b) => b.to_string(), + SpecConfigValue::Int(i) => i.to_string(), + SpecConfigValue::Float(f) => f.to_string(), + SpecConfigValue::String(s) => format!("\"{s}\""), + } +} + +fn merge_of(merge: &usage::spec::config::SpecConfigMerge) -> &'static str { + use usage::spec::config::SpecConfigMerge::*; + match merge { + Replace => "replace", + Union => "union", + Deep => "deep", + } +} + +fn scope_of(scope: &usage::spec::config::SpecConfigScope) -> &'static str { + use usage::spec::config::SpecConfigScope::*; + match scope { + Any => "any", + Global => "global", + Env => "env", + } +} + +fn effect_of(effect: &Option) -> &'static str { + match effect { + Some(effect) => effect.as_str(), + None => "unset", + } +} + +fn action_of(action: usage::spec::flag::SpecFlagAction) -> &'static str { + action.as_str() +} + +fn double_dash_of(choice: &usage::spec::arg::SpecDoubleDashChoices) -> String { + choice.to_string() +} + +fn option(value: &Option) -> String { + value + .as_ref() + .map(|v| format!("'{v}'")) + .unwrap_or_else(|| "unset".to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn changes(old: &str, new: &str) -> Vec { + let old: Spec = old.parse().unwrap(); + let new: Spec = new.parse().unwrap(); + diff_specs(&old, &new) + } + + /// `category:code` per finding, which is what most of these tests assert on. + fn codes(old: &str, new: &str) -> Vec { + changes(old, new) + .iter() + .map(|c| format!("{}:{}", c.category, c.code)) + .collect() + } + + fn find<'a>(changes: &'a [SpecChange], code: &str) -> &'a SpecChange { + changes + .iter() + .find(|c| c.code == code) + .unwrap_or_else(|| panic!("no {code} in {changes:?}")) + } + + const BASE: &str = r#" +name "ex" +bin "ex" +flag "-j --jobs " help="jobs" +flag "-f --force" help="force" +arg "" help="file" +cmd "run" help="run" { + flag "--watch" help="watch" + arg "" help="task" +} + "#; + + #[test] + fn a_spec_against_itself_reports_nothing() { + assert!(codes(BASE, BASE).is_empty()); + } + + #[test] + fn version_is_never_reported() { + // A release bumps it. A compatibility check that fires on every release is one + // nobody leaves switched on, which is the whole reason this is silent. + let new = format!("{BASE}\nversion \"9.9.9\"\nlong_version \"9.9.9 (abc)\"\n"); + assert!(codes(BASE, &new).is_empty(), "{:?}", codes(BASE, &new)); + } + + #[test] + fn breaking_findings_sort_first() { + let new = r#" +name "ex" +bin "ex" +flag "-j --jobs " help="jobs" +flag "-f --force" help="force it" +flag "--quiet" help="quiet" +arg "" help="file" + "#; + let found = codes(BASE, new); + let first_metadata = found.iter().position(|c| c.starts_with("metadata:")); + let last_breaking = found.iter().rposition(|c| c.starts_with("breaking:")); + assert!( + last_breaking.unwrap() < first_metadata.unwrap(), + "{found:?}" + ); + } + + #[test] + fn a_removed_flag_is_breaking_and_a_lost_short_is_too() { + let new = r#" +name "ex" +bin "ex" +flag "--jobs " help="jobs" +arg "" help="file" +cmd "run" help="run" { + flag "--watch" help="watch" + arg "" help="task" +} + "#; + let found = changes(BASE, new); + assert_eq!( + find(&found, "flag-removed").message, + "flag 'force' (--force, -f) was removed" + ); + assert_eq!( + find(&found, "flag-spelling-removed").message, + "flag '--jobs' no longer answers to '-j'" + ); + } + + #[test] + fn a_flag_that_kept_every_spelling_was_renamed_not_removed() { + // The internal name is not something a caller can type, so putting a new + // spelling in front of `--jobs` renames the flag without breaking anybody — + // and the spelling it gained is still reported. + let old = r#" +name "ex" +bin "ex" +flag "-j --jobs " help="jobs" + "#; + let new = r#" +name "ex" +bin "ex" +flag "--parallelism -j --jobs " help="jobs" + "#; + let found = changes(old, new); + assert_eq!( + find(&found, "flag-renamed").message, + "flag 'jobs' was renamed to 'parallelism'" + ); + assert_eq!( + find(&found, "flag-spelling-added").message, + "flag '--parallelism' now answers to '--parallelism'" + ); + assert!( + found.iter().all(|c| c.category != Category::Breaking), + "{found:?}" + ); + } + + #[test] + fn a_new_required_flag_is_breaking_and_an_optional_one_is_not() { + let required = format!("{BASE}\nflag \"--token \" help=\"token\" required=#true\n"); + assert_eq!(codes(BASE, &required), ["breaking:required-flag-added"]); + let optional = format!("{BASE}\nflag \"--token \" help=\"token\"\n"); + assert_eq!(codes(BASE, &optional), ["compatible:flag-added"]); + } + + #[test] + fn a_narrowed_strict_choice_set_is_breaking_and_a_widened_one_is_not() { + let old = r#" +name "ex" +bin "ex" +flag "--color " help="color" { + choices "auto" "always" "never" +} + "#; + let narrowed = r#" +name "ex" +bin "ex" +flag "--color " help="color" { + choices "auto" "always" +} + "#; + let widened = r#" +name "ex" +bin "ex" +flag "--color " help="color" { + choices "auto" "always" "never" "if-tty" +} + "#; + assert_eq!(codes(old, narrowed), ["breaking:choice-removed"]); + assert_eq!(codes(old, widened), ["compatible:choice-added"]); + } + + #[test] + fn dropping_a_value_from_a_non_strict_set_only_stops_offering_it() { + // `strict=#false` accepts anything, so leaving the list costs a completion + // candidate rather than an accepted value. + let old = r#" +name "ex" +bin "ex" +flag "--backend " help="backend" { + choices "npm" "cargo" strict=#false +} + "#; + let new = r#" +name "ex" +bin "ex" +flag "--backend " help="backend" { + choices "npm" strict=#false +} + "#; + assert_eq!(codes(old, new), ["metadata:choice-unlisted"]); + } + + #[test] + fn a_default_is_free_to_gain_and_costly_to_move() { + let none = "name \"ex\"\nbin \"ex\"\nflag \"--jobs \" help=\"jobs\"\n"; + let four = "name \"ex\"\nbin \"ex\"\nflag \"--jobs \" help=\"jobs\" default=\"4\"\n"; + let eight = "name \"ex\"\nbin \"ex\"\nflag \"--jobs \" help=\"jobs\" default=\"8\"\n"; + assert_eq!(codes(none, four), ["compatible:default-added"]); + assert_eq!(codes(four, eight), ["breaking:default-changed"]); + assert_eq!(codes(four, none), ["breaking:default-removed"]); + } + + #[test] + fn a_positional_appended_after_the_last_one_is_only_breaking_when_required() { + let required = r#" +name "ex" +bin "ex" +arg "" help="file" +arg "" help="out" + "#; + let optional = r#" +name "ex" +bin "ex" +arg "" help="file" +arg "[out]" help="out" + "#; + let one = "name \"ex\"\nbin \"ex\"\narg \"\" help=\"file\"\n"; + assert_eq!(codes(one, required), ["breaking:required-arg-added"]); + assert_eq!(codes(one, optional), ["compatible:arg-added"]); + assert_eq!(codes(required, one), ["breaking:arg-removed"]); + } + + #[test] + fn renaming_a_positional_leaves_the_slot_alone() { + let old = "name \"ex\"\nbin \"ex\"\narg \"\" help=\"a file\"\n"; + let new = "name \"ex\"\nbin \"ex\"\narg \"\" help=\"a file\"\n"; + let found = changes(old, new); + assert_eq!(codes(old, new), ["metadata:arg-renamed"]); + assert_eq!( + find(&found, "arg-renamed").message, + "argument 1 was renamed from to " + ); + } + + #[test] + fn a_removed_command_is_breaking_unless_something_still_answers_to_it() { + let old = r#" +name "ex" +bin "ex" +cmd "install" help="install" + "#; + let gone = "name \"ex\"\nbin \"ex\"\n"; + let renamed = r#" +name "ex" +bin "ex" +cmd "add" help="install" { + alias "install" +} + "#; + assert_eq!(codes(old, gone), ["breaking:cmd-removed"]); + let found = changes(old, renamed); + assert_eq!( + find(&found, "cmd-renamed").message, + "command 'install' was renamed to 'add', which still answers to 'install'" + ); + assert!( + found.iter().all(|c| c.category != Category::Breaking), + "{found:?}" + ); + } + + #[test] + fn a_command_taking_over_a_siblings_alias_changes_what_the_word_selects() { + let old = r#" +name "ex" +bin "ex" +cmd "remove" help="remove" { + alias "rm" +} + "#; + let new = r#" +name "ex" +bin "ex" +cmd "remove" help="remove" +cmd "rm" help="something else" + "#; + let found = codes(old, new); + assert!( + found.contains(&"breaking:cmd-shadows-alias".to_string()), + "{found:?}" + ); + } + + #[test] + fn findings_are_located_by_command_path() { + let new = r#" +name "ex" +bin "ex" +flag "-j --jobs " help="jobs" +flag "-f --force" help="force" +arg "" help="file" +cmd "run" help="run" { + arg "" help="task" +} + "#; + let found = changes(BASE, new); + assert_eq!(find(&found, "flag-removed").location, "ex run"); + } + + #[test] + fn a_lost_environment_variable_is_breaking() { + let old = r#" +name "ex" +bin "ex" +flag "--jobs " help="jobs" env="EX_JOBS" { + env_fallback "EX_PARALLEL" +} + "#; + let new = "name \"ex\"\nbin \"ex\"\nflag \"--jobs \" help=\"jobs\" env=\"EX_JOBS\"\n"; + let found = changes(old, new); + assert_eq!( + find(&found, "env-removed").message, + "flag '--jobs' no longer reads $EX_PARALLEL" + ); + assert_eq!(codes(new, old), ["compatible:env-added"]); + } + + #[test] + fn a_gained_conflict_restricts_and_a_gained_override_relaxes() { + let base = r#" +name "ex" +bin "ex" +flag "--file " help="file" +flag "--stdin" help="stdin" + "#; + let conflicting = r#" +name "ex" +bin "ex" +flag "--file " help="file" conflicts="--stdin" +flag "--stdin" help="stdin" + "#; + let overriding = r#" +name "ex" +bin "ex" +flag "--file " help="file" overrides="--stdin" +flag "--stdin" help="stdin" + "#; + assert_eq!(codes(base, conflicting), ["breaking:constraint-added"]); + assert_eq!(codes(base, overriding), ["compatible:constraint-removed"]); + // And the same edits read the other way round. + assert_eq!(codes(conflicting, base), ["compatible:constraint-removed"]); + assert_eq!(codes(overriding, base), ["breaking:constraint-added"]); + } + + #[test] + fn a_new_exclusive_group_constrains_and_a_multiple_one_does_not() { + let base = r#" +name "ex" +bin "ex" +flag "--file " help="file" +flag "--url " help="url" + "#; + let exclusive = format!("{base}\ngroup \"input\" \"--file\" \"--url\"\n"); + let permissive = format!("{base}\ngroup \"input\" \"--file\" \"--url\" multiple=#true\n"); + assert_eq!(codes(base, &exclusive), ["breaking:group-added"]); + assert_eq!(codes(base, &permissive), ["metadata:group-added"]); + } + + #[test] + fn strict_unknown_flags_reject_what_used_to_bind() { + let old = "name \"ex\"\nbin \"ex\"\narg \"[words]\" help=\"words\" var=#true\n"; + let new = format!("{old}unknown_flags \"error\"\n"); + assert_eq!(codes(old, &new), ["breaking:unknown-flags-strict"]); + assert_eq!(codes(&new, old), ["compatible:unknown-flags-lax"]); + } + + #[test] + fn a_config_property_is_interface_too() { + let old = r#" +name "ex" +bin "ex" +config { + prop "jobs" type="uint" default=4 help="jobs" { + env "EX_JOBS" + } + prop "color" type="bool" default=#true help="color" +} + "#; + let new = r#" +name "ex" +bin "ex" +config { + prop "jobs" type="uint" default=8 help="jobs" +} + "#; + let found = changes(old, new); + assert_eq!( + find(&found, "config-default-changed").message, + "config property 'jobs' default changed from 4 to 8" + ); + assert_eq!( + find(&found, "config-env-removed").message, + "config property 'jobs' no longer reads $EX_JOBS" + ); + assert_eq!( + find(&found, "config-prop-removed").message, + "config property 'color' was removed" + ); + } + + #[test] + fn a_renamed_config_property_says_where_it_went() { + let old = r#" +name "ex" +bin "ex" +config { + prop "jobs" type="uint" help="jobs" renamed_to="parallelism" +} + "#; + let new = r#" +name "ex" +bin "ex" +config { + prop "parallelism" type="uint" help="jobs" +} + "#; + let found = codes(old, new); + assert!( + found.contains(&"compatible:config-prop-renamed".to_string()), + "{found:?}" + ); + assert!( + !found.iter().any(|c| c.starts_with("breaking:")), + "{found:?}" + ); + } + + #[test] + fn help_text_and_effect_are_metadata() { + let old = r#" +name "ex" +bin "ex" +cmd "rm" help="remove" effect="write" + "#; + let new = r#" +name "ex" +bin "ex" +cmd "rm" help="delete a thing" effect="destructive" + "#; + let found = codes(old, new); + assert_eq!(found.len(), 2, "{found:?}"); + assert!( + found.iter().all(|c| c.starts_with("metadata:")), + "{found:?}" + ); + } +} diff --git a/cli/src/cli/mod.rs b/cli/src/cli/mod.rs index d5232c8cb..305a7101b 100644 --- a/cli/src/cli/mod.rs +++ b/cli/src/cli/mod.rs @@ -5,6 +5,7 @@ use miette::Result; use usage_rs::{Cli as DeriveCli, Subcommands}; pub mod complete_word; +mod diff; mod exec; mod explain; pub(crate) mod generate; @@ -107,6 +108,7 @@ pub(crate) fn version() -> String { enum Command { Bash(shell::Bash), CompleteWord(complete_word::CompleteWord), + Diff(diff::Diff), Exec(exec::Exec), Explain(explain::Explain), Fish(shell::Fish), diff --git a/cli/tests/diff.rs b/cli/tests/diff.rs new file mode 100644 index 000000000..a5cf2d0e5 --- /dev/null +++ b/cli/tests/diff.rs @@ -0,0 +1,165 @@ +//! `usage diff` as a command: what it prints, what it exits with, and what it reads. +//! +//! The classification rules are unit-tested beside the comparison itself. What is +//! only observable from outside is here: the exit status a release job gates on, the +//! two output formats, and reading one of the two specs from stdin. + +use assert_cmd::Command; +use predicates::prelude::PredicateBooleanExt; +use predicates::str::contains; +use std::path::{Path, PathBuf}; + +const OLD: &str = r#" +name "ex" +bin "ex" +version "1.0.0" +flag "-j --jobs " help="how many at once" +flag "-f --force" help="force it" +arg "" help="the file" +cmd "run" help="run it" +"#; + +const NEW: &str = r#" +name "ex" +bin "ex" +version "2.0.0" +flag "--jobs " help="how many at once" +flag "-f --force" help="force it" +flag "--quiet" help="be quiet" +arg "" help="the file" +cmd "run" help="run it" +"#; + +fn usage_cmd() -> Command { + // `assert_cmd::Command` rather than the standard one: two of these cases feed a + // spec on stdin, which is what `write_stdin` is for. + Command::new(assert_cmd::cargo::cargo_bin!("usage")) +} + +/// A directory of this test's own, named for the case, so parallel tests cannot +/// read each other's fixtures. +fn fixtures(case: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("usage_diff_{case}_{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("old.usage.kdl"), OLD).unwrap(); + std::fs::write(dir.join("new.usage.kdl"), NEW).unwrap(); + dir +} + +fn old(dir: &Path) -> PathBuf { + dir.join("old.usage.kdl") +} + +fn new(dir: &Path) -> PathBuf { + dir.join("new.usage.kdl") +} + +#[test] +fn a_breaking_change_exits_one_so_a_release_job_can_gate_on_it() { + let dir = fixtures("gate"); + usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg(new(&dir)) + .assert() + .code(1) + .stdout(contains( + "breaking [flag-spelling-removed] at ex: flag '--jobs' no longer answers to '-j'", + )) + .stdout(contains( + "compatible [flag-added] at ex: flag '--quiet' was added", + )) + .stdout(contains( + "Found 1 breaking, 1 compatible, 0 metadata change(s)", + )); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn exit_zero_reports_without_failing() { + let dir = fixtures("exit_zero"); + usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg(new(&dir)) + .arg("--exit-zero") + .assert() + .success() + .stdout(contains("breaking [flag-spelling-removed]")); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn identical_specs_are_silent_and_succeed() { + let dir = fixtures("identical"); + usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg(old(&dir)) + .assert() + .success() + .stdout(contains("No interface changes.")); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn breaking_only_hides_the_rest() { + let dir = fixtures("breaking_only"); + usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg(new(&dir)) + .arg("--breaking") + .assert() + .code(1) + .stdout(contains("breaking [flag-spelling-removed]")) + .stdout(contains("flag-added").not()); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn json_is_a_list_a_program_can_read() { + let dir = fixtures("json"); + let output = usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg(new(&dir)) + .args(["--format", "json"]) + .output() + .unwrap(); + let changes: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let changes = changes.as_array().unwrap(); + assert_eq!(changes.len(), 2); + assert_eq!(changes[0]["category"], "breaking"); + assert_eq!(changes[0]["code"], "flag-spelling-removed"); + assert_eq!(changes[0]["location"], "ex"); + assert_eq!(changes[1]["category"], "compatible"); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn the_new_spec_can_come_from_stdin() { + // The shape a release job wants: the released spec on disk against what the + // binary being built says about itself, with no temporary file in between. + let dir = fixtures("stdin"); + usage_cmd() + .arg("diff") + .arg(old(&dir)) + .arg("-") + .write_stdin(NEW) + .assert() + .code(1) + .stdout(contains("breaking [flag-spelling-removed]")); + std::fs::remove_dir_all(&dir).unwrap(); +} + +#[test] +fn both_specs_cannot_be_stdin() { + usage_cmd() + .args(["diff", "-", "-"]) + .write_stdin(NEW) + .assert() + .failure() + .stderr(contains("only one of the two specs can be read from stdin")); +} diff --git a/cli/usage.usage.kdl b/cli/usage.usage.kdl index 10b532aff..ec641a510 100644 --- a/cli/usage.usage.kdl +++ b/cli/usage.usage.kdl @@ -44,6 +44,21 @@ cmd complete-word help="Generate shell completion candidates for a partial comma } arg "[WORDS]..." help="User's input from the command line" } +cmd diff help="Compare two usage specs and report what changed about the interface" effect=read { + long_help "Compare two usage specs and report what changed about the interface\n\nFindings are grouped into breaking changes (a command line that used to work\nnow fails, binds differently, or resolves to a different value), compatible\nchanges (the interface gained something or relaxed a rule), and metadata\nchanges (help text, effect, deprecation — nothing about parsing).\n\nExits 1 when there is a breaking change, so a release job can gate on it, and\neither spec may be \"-\":\n\n mycli --usage-spec | usage diff released.usage.kdl -\n\n`version` is ignored on purpose: a release bumps it, and a check that fires\nevery release does not get left switched on." + flag "-f --format" help="Output format" default=text { + arg { + choices { + choice text + choice json + } + } + } + flag "-b --breaking" help="Report only breaking changes" + flag --exit-zero help="Exit 0 even when there are breaking changes" + arg help="The spec as it was, typically the released one, use \"-\" to read from stdin" + arg help="The spec as it is now, use \"-\" to read from stdin" +} cmd exec help="Execute a script, parsing args and exposing them as environment variables" unknown_flags=value { alias x flag -h help="Show help" diff --git a/docs/.vitepress/config.mts b/docs/.vitepress/config.mts index 8a670a5f0..07af54909 100644 --- a/docs/.vitepress/config.mts +++ b/docs/.vitepress/config.mts @@ -94,6 +94,7 @@ export default defineConfig({ link: "/cli/", items: [ { text: "Completions", link: "/cli/completions" }, + { text: "Comparing Specs", link: "/cli/diff" }, { text: "Manpages", link: "/cli/manpages" }, { text: "Markdown", link: "/cli/markdown" }, { text: "SDK Generation", link: "/cli/sdk" }, diff --git a/docs/cli/diff.md b/docs/cli/diff.md new file mode 100644 index 000000000..3ccfa0962 --- /dev/null +++ b/docs/cli/diff.md @@ -0,0 +1,111 @@ +# Comparing two specs + +A CLI is a public API, and a spec is the only machine-readable statement of what that API is. +`usage diff` reads two of them and says what changed: + +```sh +usage diff released.usage.kdl current.usage.kdl +``` + +``` +breaking [flag-spelling-removed] at ex: flag '--jobs' no longer answers to '-j' +breaking [choice-removed] at ex: flag '--color' no longer accepts 'never' +breaking [cmd-removed] at ex: command 'old-thing' was removed +compatible [flag-added] at ex: flag '--quiet' was added +compatible [cmd-added] at ex: command 'new-thing' was added +metadata [help-changed] at ex: flag '--force' help text changed + +Found 3 breaking, 2 compatible, 1 metadata change(s) +``` + +It exits `1` when there is a breaking change, so a release job can gate on it. Nothing else +in the CLI ecosystem can answer this question, for a structural reason: +[clap#918](https://github.com/clap-rs/clap/issues/918) has been open since 2017 asking for the +export it would need. + +## The three categories + +One rule draws the lines. + +**breaking** — a command line that worked against the old spec now fails, binds differently, or +resolves to a different value. A removed flag, a lost short spelling, a narrowed `choices` set, +a positional that became required, a new `conflicts`, a default that moved. + +**compatible** — the interface gained something, or relaxed a rule. Every command line that +worked still works and still means the same thing. A new optional flag, a widened `choices` set, +a requirement dropped, a new subcommand. + +**metadata** — nothing about parsing moved: help text, `help_heading`, `display_order`, +hidden-ness, `effect`, deprecation notices, a renamed positional, an internal flag rename that +kept every spelling. + +The interesting cases are the ones where the same edit lands in different categories depending +on context: + +| edit | category | why | +| --------------------------------------------- | ---------- | --------------------------------------------------- | +| dropping a value from a strict `choices` | breaking | the value is now rejected | +| dropping a value from `choices strict=#false` | metadata | the value is still accepted, just no longer offered | +| appending an optional positional | compatible | no word that used to bind moves | +| appending a required positional | breaking | an invocation without it now fails | +| adding a `default` | compatible | nothing was resolved there before | +| changing or removing a `default` | breaking | it moves ground the caller was already standing on | +| gaining a `conflicts` | breaking | a combination that was valid is now rejected | +| gaining an `overrides` | compatible | a collision that was an error now resolves | +| gaining a group member, `multiple=#true` | metadata | membership only decides what satisfies `required` | +| gaining a group member, exclusive group | breaking | the new member conflicts with the rest | +| renaming a command that keeps an alias | metadata | the old word still selects it | +| renaming a command with no alias | breaking | the old word selects nothing | + +## Two deliberate silences + +**`version` is never reported.** A release bumps it, and a compatibility check that fires on +every release is one nobody leaves switched on. `long_version` is silent for the same reason. + +**Derived strings are never reported.** `usage`, `full_cmd` and `help_first_line` restate what +the declarations already say, so a change in one of them is reported at its source or not at all. + +A `mount` is compared as a declaration — added or removed — and not by what it discovers. +Resolving one means running the command it names, which reading two files should not do. + +## In CI + +The shape most releases want is the published spec against what the binary being built says +about itself. One of the two specs may be `-`: + +```sh +mycli --usage-spec | usage diff mycli.usage.kdl - +``` + +As a release gate: + +```yaml +- name: the CLI contract still holds + run: | + git show "$(git describe --tags --abbrev=0)":mycli.usage.kdl > released.usage.kdl + mycli --usage-spec | usage diff released.usage.kdl - --breaking +``` + +`--breaking` drops the compatible and metadata findings, which is what a gate wants to read. +`--exit-zero` reports without failing, for a job that comments on a pull request rather than +blocking it. `--format json` gives the same findings as a list of `{category, code, message, +location}` objects, so a script can act on a specific `code`. + +## Compare specs from the same generator + +A spec generated from a typed CLI says what the generator of the day could see. Comparing +one emitted by an older `clap_usage` against one emitted by a newer one reports everything +the newer emitter learned to express as an interface change — relationships that were always +enforced but never written down read as newly added constraints. + +Refreshing hk's checked-in fixture is this exactly: 329 `constraint-added` findings, none of +them a change to hk. The findings are a true reading of the two files, so the fix is not to +soften them but to compare like with like — the released spec against a spec emitted by the +same generator version, which is what a release job does anyway. + +## What `deprecated` is for + +A deprecation is metadata: the flag still parses, so it costs nobody anything today. What it +buys is that the removal it promises shows up here as `breaking` later, against a spec that +announced it first. `usage diff` is where a deprecation window is observed rather than +remembered. diff --git a/docs/cli/reference/commands.json b/docs/cli/reference/commands.json index cfb2453d2..5e2cbb909 100644 --- a/docs/cli/reference/commands.json +++ b/docs/cli/reference/commands.json @@ -164,6 +164,93 @@ "hidden_aliases": [], "examples": [] }, + "diff": { + "full_cmd": ["diff"], + "usage": "diff [FLAGS] ", + "subcommands": {}, + "args": [ + { + "name": "OLD", + "usage": "", + "help": "The spec as it was, typically the released one, use \"-\" to read from stdin", + "help_first_line": "The spec as it was, typically the released one, use \"-\" to read from stdin", + "required": true, + "double_dash": "Optional", + "hide": false + }, + { + "name": "NEW", + "usage": "", + "help": "The spec as it is now, use \"-\" to read from stdin", + "help_first_line": "The spec as it is now, use \"-\" to read from stdin", + "required": true, + "double_dash": "Optional", + "hide": false + } + ], + "flags": [ + { + "name": "format", + "usage": "-f --format ", + "help": "Output format", + "help_first_line": "Output format", + "short": ["f"], + "long": ["format"], + "hide": false, + "global": false, + "arg": { + "name": "FORMAT", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false, + "choices": { + "choices": ["text", "json"], + "details": [ + { + "value": "text" + }, + { + "value": "json" + } + ] + } + }, + "default": ["text"] + }, + { + "name": "breaking", + "usage": "-b --breaking", + "help": "Report only breaking changes", + "help_first_line": "Report only breaking changes", + "short": ["b"], + "long": ["breaking"], + "hide": false, + "global": false + }, + { + "name": "exit-zero", + "usage": "--exit-zero", + "help": "Exit 0 even when there are breaking changes", + "help_first_line": "Exit 0 even when there are breaking changes", + "short": [], + "long": ["exit-zero"], + "hide": false, + "global": false + } + ], + "mounts": [], + "effect": "read", + "unknown_flags": null, + "hide": false, + "args_override_self": true, + "help": "Compare two usage specs and report what changed about the interface", + "help_long": "Compare two usage specs and report what changed about the interface\n\nFindings are grouped into breaking changes (a command line that used to work\nnow fails, binds differently, or resolves to a different value), compatible\nchanges (the interface gained something or relaxed a rule), and metadata\nchanges (help text, effect, deprecation — nothing about parsing).\n\nExits 1 when there is a breaking change, so a release job can gate on it, and\neither spec may be \"-\":\n\n mycli --usage-spec | usage diff released.usage.kdl -\n\n`version` is ignored on purpose: a release bumps it, and a check that fires\nevery release does not get left switched on.", + "name": "diff", + "aliases": [], + "hidden_aliases": [], + "examples": [] + }, "exec": { "full_cmd": ["exec"], "usage": "exec [-h] [--help] …", diff --git a/docs/cli/reference/diff.md b/docs/cli/reference/diff.md new file mode 100644 index 000000000..6c12230bb --- /dev/null +++ b/docs/cli/reference/diff.md @@ -0,0 +1,53 @@ + + +# `usage diff` + +- **Usage**: `usage diff [FLAGS] ` +- **Effect**: read-only +- **Source code**: [`cli/src/cli/diff.rs`](https://github.com/jdx/usage/blob/main/cli/src/cli/diff.rs) + +Compare two usage specs and report what changed about the interface + +Findings are grouped into breaking changes (a command line that used to work +now fails, binds differently, or resolves to a different value), compatible +changes (the interface gained something or relaxed a rule), and metadata +changes (help text, effect, deprecation — nothing about parsing). + +Exits 1 when there is a breaking change, so a release job can gate on it, and +either spec may be "-": + +mycli --usage-spec | usage diff released.usage.kdl - + +`version` is ignored on purpose: a release bumps it, and a check that fires +every release does not get left switched on. + +## Arguments + +### `` + +The spec as it was, typically the released one, use "-" to read from stdin + +### `` + +The spec as it is now, use "-" to read from stdin + +## Flags + +### `-f --format ` + +Output format + +**Choices:** + +- `text` +- `json` + +**Default:** `text` + +### `-b --breaking` + +Report only breaking changes + +### `--exit-zero` + +Exit 0 even when there are breaking changes diff --git a/docs/cli/reference/index.md b/docs/cli/reference/index.md index 559ef0ac4..d230312e4 100644 --- a/docs/cli/reference/index.md +++ b/docs/cli/reference/index.md @@ -24,6 +24,7 @@ Outputs a `usage.kdl` spec for this CLI itself - [`usage bash [-h] [--help]