diff --git a/README.md b/README.md index 5c7f681..ef0cb6a 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ dependable check . --format json # machine-readable output (also: text) dependable check . --fail-on vulnerable # exit non-zero for CI dependable check . --annotations always # GitHub Actions annotations + job summary dependable check . --manifest-glob 'services/*/Cargo.toml' # one slice of a monorepo +dependable check . --ecosystem rust # one ecosystem of a polyglot repo dependable list . # every project and what it declares (offline) dependable tree . # render the dependency tree (Rust) dependable fix . --dry-run # preview in-place upgrades @@ -268,6 +269,31 @@ conflicts with `--manifest`, which names one file and skips discovery altogether It is available on `fix` for a reason: without it, `dependable fix` would rewrite manifests that the matching `dependable check` deliberately left out. +To work on part of a *polyglot* repository, filter by ecosystem instead: + +```bash +dependable check . --ecosystem rust +dependable list . --ecosystem npm --ecosystem rust --format json +dependable fix . --ecosystem rust --dry-run +``` + +`--ecosystem` narrows discovery to the manifests belonging to the ecosystems you +name — `rust`, `go`, `npm`, `python`, `php`, `dart`, `csharp`, `elixir`, `jvm`. It +names the *ecosystem*, not a filename, so `--ecosystem npm` covers `package.json`, +`deno.json`, and `pnpm-workspace.yaml` alike. The flag is repeatable and a manifest +in any named ecosystem is kept, it is available on `check`, `list`, and `fix` for +the same reason `--manifest-glob` is, and it conflicts with `--manifest`. When it +selects nothing, `dependable` says which ecosystems it searched and which it found +instead, and still exits 0 — an unused ecosystem must not fail a per-ecosystem CI +matrix job. `list --format json` prints a valid `dependable.list/v1` document with +zero projects in that case, so a shard that pipes into `jq` gets something to +parse rather than empty output. + +It only ever **narrows** a run. Naming an ecosystem that `.dependable.toml` has +switched off does not switch it back on: `check --ecosystem jvm` under +`[jvm] enabled = false` discovers the manifests and then reports them as skipped, +exactly as it does without the flag. + #### Inherited versions A Cargo workspace declares shared versions once, at the root, and members opt in by diff --git a/crates/dependable-core/src/ecosystem.rs b/crates/dependable-core/src/ecosystem.rs index 8272d01..3b164d7 100644 --- a/crates/dependable-core/src/ecosystem.rs +++ b/crates/dependable-core/src/ecosystem.rs @@ -49,6 +49,32 @@ pub enum Ecosystem { } impl Ecosystem { + /// Every variant, in declaration order. + /// + /// Hand-written, because there is no stable way to enumerate an enum's + /// variants and `#[non_exhaustive]` puts an exhaustive match out of reach of + /// every other crate. What keeps it honest is *where it sits*: every method + /// below matches on `self` exhaustively, so adding a variant stops this file + /// compiling, and this list is in front of whoever fixes that. That is a + /// prompt, not a proof — nothing forces the list to grow, so keep it in step + /// with the enum by hand. + /// + /// It exists to be pinned against. A frontend that has to cover every + /// ecosystem — the `--ecosystem` values of the `dependable` binary, for one — + /// asserts its own coverage equals this list, so an ecosystem missing from it + /// is an ecosystem that silently reaches no user. + pub const ALL: [Self; 9] = [ + Ecosystem::Rust, + Ecosystem::Go, + Ecosystem::Npm, + Ecosystem::Python, + Ecosystem::Php, + Ecosystem::Dart, + Ecosystem::CSharp, + Ecosystem::Elixir, + Ecosystem::Jvm, + ]; + /// The `package.ecosystem` string used in OSV vulnerability queries. #[must_use] pub fn osv_name(self) -> &'static str { @@ -227,23 +253,9 @@ impl Ecosystem { mod tests { use super::*; - /// Every variant, so a new ecosystem cannot be added without being given - /// its pages. - const ALL: [Ecosystem; 9] = [ - Ecosystem::Rust, - Ecosystem::Go, - Ecosystem::Npm, - Ecosystem::Python, - Ecosystem::Php, - Ecosystem::Dart, - Ecosystem::CSharp, - Ecosystem::Elixir, - Ecosystem::Jvm, - ]; - #[test] fn every_ecosystem_can_name_a_page_for_a_package() { - for ecosystem in ALL { + for ecosystem in Ecosystem::ALL { let url = ecosystem.package_url("serde"); assert!(url.starts_with("https://"), "{ecosystem:?}: {url}"); assert!(url.contains("serde"), "{ecosystem:?}: {url}"); @@ -350,7 +362,11 @@ mod tests { // conflict resolution. (Ecosystem::Jvm, BareVersion::Minimum), ]; - assert_eq!(expected.len(), ALL.len(), "every variant must be listed"); + assert_eq!( + expected.len(), + Ecosystem::ALL.len(), + "every variant must be listed" + ); for (ecosystem, reading) in expected { assert_eq!(ecosystem.bare_version(), reading, "{ecosystem:?}"); } @@ -360,7 +376,7 @@ mod tests { /// terms of the other, and this pins that they stay that way. #[test] fn the_exactness_shorthand_agrees_with_the_full_reading() { - for ecosystem in ALL { + for ecosystem in Ecosystem::ALL { assert_eq!( ecosystem.bare_version_is_exact(), ecosystem.bare_version() == BareVersion::Exact, diff --git a/crates/dependable/src/cli.rs b/crates/dependable/src/cli.rs index f3223e0..6c3dbbb 100644 --- a/crates/dependable/src/cli.rs +++ b/crates/dependable/src/cli.rs @@ -68,6 +68,14 @@ pub struct CheckArgs { /// any pattern is kept. `*` and `?` do not cross `/`, `**` does. #[arg(long, conflicts_with = "manifest")] pub manifest_glob: Vec, + /// Only use manifests belonging to this ecosystem. Repeatable; a manifest in + /// any of the named ecosystems is kept. + /// + /// It narrows discovery and never widens it: naming an ecosystem that + /// `.dependable.toml` has switched off does not switch it back on, and it + /// registers no fetcher that was not already there. + #[arg(long, value_enum, conflicts_with = "manifest")] + pub ecosystem: Vec, /// Config file path. #[arg(long, default_value = ".dependable.toml")] pub config: PathBuf, @@ -128,6 +136,14 @@ pub struct ListArgs { /// any pattern is kept. `*` and `?` do not cross `/`, `**` does. #[arg(long, conflicts_with = "manifest")] pub manifest_glob: Vec, + /// Only use manifests belonging to this ecosystem. Repeatable; a manifest in + /// any of the named ecosystems is kept. + /// + /// It narrows discovery and never widens it: naming an ecosystem that + /// `.dependable.toml` has switched off does not switch it back on, and it + /// registers no fetcher that was not already there. + #[arg(long, value_enum, conflicts_with = "manifest")] + pub ecosystem: Vec, /// Config file path. `list` reads only the per-ecosystem `enabled` flags from /// it, so that an ecosystem you have switched off is not warned about; it does /// not read registry or network settings. @@ -192,6 +208,14 @@ pub struct FixArgs { /// any pattern is kept. `*` and `?` do not cross `/`, `**` does. #[arg(long, conflicts_with = "manifest")] pub manifest_glob: Vec, + /// Only use manifests belonging to this ecosystem. Repeatable; a manifest in + /// any of the named ecosystems is kept. + /// + /// It narrows discovery and never widens it: naming an ecosystem that + /// `.dependable.toml` has switched off does not switch it back on, and it + /// registers no fetcher that was not already there. + #[arg(long, value_enum, conflicts_with = "manifest")] + pub ecosystem: Vec, #[arg(long, default_value = ".dependable.toml")] pub config: PathBuf, /// Update all, including beyond the declared constraint. @@ -372,3 +396,100 @@ impl From for dependable_fetch::UnstableFilter { } } } + +/// An ecosystem nameable on the command line via `--ecosystem`. +/// +/// A CLI-local mirror of [`dependable_fetch::Ecosystem`]. `ValueEnum` is clap's +/// trait and `Ecosystem` is a foreign type here, so the orphan rule rules out +/// implementing one for the other; deriving `ValueEnum` upstream instead would +/// put clap into `dependable-core`, which is deliberately IO-free and +/// frontend-agnostic. The [`From`] impl below is the only bridge, and the unit +/// test beside it asserts that its image is exactly +/// [`Ecosystem::ALL`](dependable_fetch::Ecosystem::ALL) — the list the defining +/// crate keeps beside the exhaustive matches a new variant breaks. +/// +/// The accepted spellings are the canonical lowercase names and nothing else. +/// Aliases (`kotlin`, `java`, `deno`, `nuget`) are deliberately absent: every +/// accepted string is a permanent compatibility surface, and an alias would +/// outlive the variant it names if an ecosystem later splits — `deno` would go +/// on meaning [`Npm`](Ecosystem::Npm) after a Deno variant existed. Aliases are +/// additive and cheap to add later; they are not removable. +#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] +pub enum EcosystemArg { + Rust, + Go, + Npm, + Python, + Php, + Dart, + // Spelled out because clap's default kebab-casing renders `CSharp` as + // `c-sharp`, which is not a name anybody types. A `///` here would render + // that reasoning beside the value in `--help`, where it is noise. + #[value(name = "csharp")] + CSharp, + Elixir, + Jvm, +} + +impl From for dependable_fetch::Ecosystem { + fn from(value: EcosystemArg) -> Self { + match value { + EcosystemArg::Rust => dependable_fetch::Ecosystem::Rust, + EcosystemArg::Go => dependable_fetch::Ecosystem::Go, + EcosystemArg::Npm => dependable_fetch::Ecosystem::Npm, + EcosystemArg::Python => dependable_fetch::Ecosystem::Python, + EcosystemArg::Php => dependable_fetch::Ecosystem::Php, + EcosystemArg::Dart => dependable_fetch::Ecosystem::Dart, + EcosystemArg::CSharp => dependable_fetch::Ecosystem::CSharp, + EcosystemArg::Elixir => dependable_fetch::Ecosystem::Elixir, + EcosystemArg::Jvm => dependable_fetch::Ecosystem::Jvm, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use dependable_fetch::Ecosystem; + + /// Adding an ecosystem without adding its `--ecosystem` value would leave a + /// supported ecosystem unfilterable, and — worse — leave `--ecosystem` unable + /// to say so. + /// + /// The comparison is against [`Ecosystem::ALL`], not against a second list + /// written here: a list local to this test would be updated by the same hand + /// that forgot the variant, and would agree with itself for ever. `ALL` lives + /// in `dependable-core` beside the exhaustive matches a new variant does not + /// compile past, so it is the one list a new ecosystem cannot be added + /// without meeting. + #[test] + fn every_ecosystem_can_be_named_on_the_command_line() { + let nameable: Vec = EcosystemArg::value_variants() + .iter() + .map(|arg| Ecosystem::from(*arg)) + .collect(); + assert_eq!(nameable, Ecosystem::ALL.to_vec()); + } + + /// The one value whose derived spelling is wrong: clap kebab-cases `CSharp` + /// to `c-sharp`. Asserted on the value clap advertises, not on the variant. + #[test] + fn csharp_is_spelled_the_way_it_is_typed() { + let names: Vec = EcosystemArg::value_variants() + .iter() + .map(|arg| { + arg.to_possible_value() + .expect("no variant is skipped") + .get_name() + .to_owned() + }) + .collect(); + assert_eq!( + names, + [ + "rust", "go", "npm", "python", "php", "dart", "csharp", "elixir", "jvm" + ] + ); + } +} diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index e4c55ee..d6a796d 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -25,7 +25,7 @@ use dependable_tui::TuiOptions; use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; use indicatif::{ProgressBar, ProgressStyle}; -use crate::cli::{CheckArgs, FailOn, FixArgs, ListArgs, TreeArgs, TuiArgs}; +use crate::cli::{CheckArgs, EcosystemArg, FailOn, FixArgs, Format, ListArgs, TreeArgs, TuiArgs}; use crate::config::{Config, load_config}; #[cfg(feature = "report")] use crate::config::{PolicySource, load_policy}; @@ -394,15 +394,17 @@ pub async fn run_check(args: CheckArgs) -> anyhow::Result { #[cfg(not(feature = "report"))] warn_policy_ignored(&args.config); + let ecosystems = requested_ecosystems(&args.ecosystem); let manifests = collect_manifests( args.manifest.as_deref(), args.path.as_deref(), settings.depth, &args.manifest_glob, + &ecosystems, &|ecosystem| cfg.ecosystem_enabled(ecosystem), )?; if manifests.is_empty() { - eprintln!("No supported manifests found."); + report_no_manifests(&ecosystems); return Ok(ExitCode::SUCCESS); } @@ -681,15 +683,28 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { // replaced by defaults that enable all of them. let cfg = load_config(&args.config).with_context(|| format!("reading {}", args.config.display()))?; + let ecosystems = requested_ecosystems(&args.ecosystem); let manifests = collect_manifests( args.manifest.as_deref(), args.path.as_deref(), args.depth, &args.manifest_glob, + &ecosystems, &|ecosystem| cfg.ecosystem_enabled(ecosystem), )?; if manifests.is_empty() { - eprintln!("No supported manifests found."); + report_no_manifests(&ecosystems); + // An empty selection still owes a machine-readable format a document. + // Exiting 0 with byte-empty stdout is what `list --ecosystem csharp + // --format json | jq ...` sees in a per-ecosystem CI matrix, and `jq` + // fails to parse nothing — on precisely the ecosystems exit 0 was chosen + // to keep green. The stderr line above is not machine-readable. + // + // `table` and `text` keep returning early: a human handed an empty table + // wants the stderr line, not a blank one. + if matches!(args.format, Format::Json) { + output::list::render(args.format, &[], &root)?; + } return Ok(ExitCode::SUCCESS); } let mut reports = Vec::new(); @@ -939,15 +954,17 @@ pub async fn run_fix(args: FixArgs) -> anyhow::Result { registry: cfg.rust.registry.clone(), osv_url: cfg.vulnerability.osv_batch_url.clone(), }; + let ecosystems = requested_ecosystems(&args.ecosystem); let manifests = collect_manifests( args.manifest.as_deref(), args.path.as_deref(), settings.depth, &args.manifest_glob, + &ecosystems, &|ecosystem| cfg.ecosystem_enabled(ecosystem), )?; if manifests.is_empty() { - eprintln!("No supported manifests found."); + report_no_manifests(&ecosystems); return Ok(ExitCode::SUCCESS); } @@ -1227,10 +1244,13 @@ pub async fn run_report(args: crate::cli::ReportArgs) -> anyhow::Result Vec { } /// The manifests a command should act on: the one named by `--manifest`, else the -/// depth-limited walk of `path`, narrowed by any `--manifest-glob` patterns. +/// depth-limited walk of `path`, narrowed by any `--ecosystem` values and then by +/// any `--manifest-glob` patterns. /// -/// The globs filter *after* the walk rather than pruning inside it: `path` still -/// roots the scan and `--depth` still bounds it, so the three compose instead of +/// Both filters run *after* the walk rather than pruning inside it: `path` still +/// roots the scan and `--depth` still bounds it, so they compose instead of /// competing, and there stays exactly one walk implementation — in /// `dependable-fetch`, which knows nothing about globs. +/// +/// `ecosystems` empty means unrestricted. When it is not, it does two distinct +/// things, and both are required for the flag to mean what it says: +/// +/// - It narrows the returned set. `dependable_fetch::discover` documents that its +/// `enabled` predicate "gates the notices only — discovery still returns every +/// manifest it recognizes, and narrowing that set stays the caller's job", so +/// composing the request into that predicate alone would suppress warnings and +/// change nothing a command actually reads. +/// - It is composed into that predicate all the same, so `--ecosystem rust` does +/// not also print advice about the Gradle build it just excluded. +/// +/// The filter re-derives each manifest's ecosystem from its path. Discovery only +/// ever returns paths [`ManifestKind::detect`] recognized, so the `None` arm is +/// unreachable in practice; a path that somehow failed to detect is dropped, +/// because an ecosystem nothing can name is not one of the ones asked for. +/// +/// The ecosystem filter runs **before** the glob filter so that the glob's +/// "matched nothing" line counts only the manifests still in play. fn collect_manifests( manifest: Option<&Path>, path: Option<&Path>, depth: usize, globs: &[String], + ecosystems: &[Ecosystem], enabled: &dyn Fn(Ecosystem) -> bool, ) -> anyhow::Result> { if let Some(manifest) = manifest { // `--manifest` names one exact file and bypasses discovery entirely, so - // there is no discovered set for a glob to filter. clap rejects the - // combination rather than letting one of them be silently ignored. + // there is no discovered set for a glob or an ecosystem to filter. clap + // rejects both combinations rather than letting one be silently ignored. return Ok(vec![manifest.to_path_buf()]); } let root = path.map_or_else(|| PathBuf::from("."), Path::to_path_buf); + let requested = |ecosystem: Ecosystem| ecosystems.is_empty() || ecosystems.contains(&ecosystem); // One walk for both answers. Manifests we recognise but cannot read produce // nothing for the walk to return, so this is the only point at which their // absence can be reported at all. - let found = dependable_fetch::discover(&root, depth, enabled); + let found = dependable_fetch::discover(&root, depth, |ecosystem| { + enabled(ecosystem) && requested(ecosystem) + }); for notice in &found.notices { eprintln!("warning: {notice}"); } - let found = found.manifests; + let mut found = found.manifests; + if !ecosystems.is_empty() { + let kept: Vec = found + .iter() + .filter(|manifest| { + ManifestKind::detect(manifest) + .is_some_and(|kind| ecosystems.contains(&kind.ecosystem())) + }) + .cloned() + .collect(); + if kept.is_empty() { + eprintln!("{}", no_ecosystem_match(ecosystems, &found, depth)); + } + found = kept; + } if globs.is_empty() { return Ok(found); } @@ -1361,6 +1419,87 @@ fn collect_manifests( Ok(kept) } +/// The ecosystems `--ecosystem` asked for, as the core type. Empty means +/// unrestricted, which is what an absent flag produces. +/// +/// Deduplicated, in first-named order. clap's `Vec` keeps every repeat, so +/// `--ecosystem rust --ecosystem rust` arrived as two values and +/// [`no_ecosystem_match`] read them back as `no manifest for Rust, Rust`. +/// Selection never cared — it is a `contains` — so this is the diagnostic alone. +/// Dedupe by membership rather than by sorting: [`Ecosystem`] is `Eq` and not +/// `Ord`, and the order the user named them in is the order to say them back. +fn requested_ecosystems(args: &[EcosystemArg]) -> Vec { + let mut requested: Vec = Vec::new(); + for ecosystem in args.iter().copied().map(Ecosystem::from) { + if !requested.contains(&ecosystem) { + requested.push(ecosystem); + } + } + requested +} + +/// Why an `--ecosystem` filter came back empty: what was asked for, how much was +/// searched, and which ecosystems were there instead. +/// +/// This *replaces* the generic "No supported manifests found." rather than joining +/// it — see [`report_no_manifests`]. Naming what was found is the whole point: the +/// two answers a user needs to tell apart are "this repository has no Rust in it" +/// and "the filter removed everything", and the generic line says neither. +fn no_ecosystem_match(requested: &[Ecosystem], searched: &[PathBuf], depth: usize) -> String { + // Discovery returns a sorted list, so first-seen order is deterministic. + // `Ecosystem` is `Eq` but not `Ord`, so dedupe by membership rather than sort. + let mut present: Vec = Vec::new(); + for kind in searched.iter().filter_map(|m| ManifestKind::detect(m)) { + let ecosystem = kind.ecosystem(); + if !present.contains(&ecosystem) { + present.push(ecosystem); + } + } + let count = searched.len(); + let plural = if count == 1 { "" } else { "s" }; + let asked = ecosystem_names(requested); + if present.is_empty() { + format!("no manifest for {asked} (searched {count} manifest{plural} up to --depth {depth})") + } else { + format!( + "no manifest for {asked} (searched {count} manifest{plural} up to --depth {depth}; found {})", + ecosystem_names(&present) + ) + } +} + +/// Ecosystems as a human-readable list, in the order given. +fn ecosystem_names(ecosystems: &[Ecosystem]) -> String { + ecosystems + .iter() + .map(|ecosystem| ecosystem.display_name()) + .collect::>() + .join(", ") +} + +/// The line a command prints when discovery came back with nothing to do. +/// +/// Silent whenever an `--ecosystem` filter was in force, which is what +/// `ecosystems` non-empty tests — not whether that filter is what emptied the +/// set. The two come apart: `--ecosystem rust --manifest-glob 'nope/*'` over a +/// repository that does contain Rust is emptied by the glob, which printed its +/// own line, while [`collect_manifests`]' ecosystem explanation never ran. The +/// generic line is suppressed there too, and deliberately: the glob line is the +/// specific answer in that case, and "No supported manifests found." is a +/// falsehood in a repository full of manifests some filter removed. The +/// predicate is the coarse one because a caller cannot tell the two apart +/// without [`collect_manifests`] reporting back which filter emptied the set, +/// and that return type is deliberately still `Vec`. +/// +/// Either way the exit code is 0 — an empty selection is an answer, not a tool +/// error, and a per-ecosystem CI matrix job must not fail on the ecosystems a +/// repository does not use. +fn report_no_manifests(ecosystems: &[Ecosystem]) { + if ecosystems.is_empty() { + eprintln!("No supported manifests found."); + } +} + /// Compile `--manifest-glob` patterns into a matcher over manifest paths, with /// union semantics: a manifest matching any pattern is kept. /// @@ -1727,16 +1866,27 @@ mod tests { Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/sample-monorepo") } - fn matched(globs: &[&str]) -> Vec { + /// A polyglot fixture: `services/api/Cargo.toml` beside `services/sync/go.mod`, + /// so an ecosystem filter has something of another ecosystem to remove. + fn polyglot() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/sample-polyglot") + } + + /// The manifests `collect_manifests` keeps under both filters, relative to + /// `root` and `/`-separated so an assertion does not depend on the platform. + fn matched_under(root: &Path, globs: &[&str], ecosystems: &[Ecosystem]) -> Vec { let globs: Vec = globs.iter().map(|g| (*g).to_string()).collect(); - let root = monorepo(); - collect_manifests(None, Some(&root), 4, &globs, &|_| true) + collect_manifests(None, Some(root), 4, &globs, ecosystems, &|_| true) .expect("the patterns are valid") .iter() - .map(|m| output::posix(&relative_to(&root, m))) + .map(|m| output::posix(&relative_to(root, m))) .collect() } + fn matched(globs: &[&str]) -> Vec { + matched_under(&monorepo(), globs, &[]) + } + #[test] fn a_star_in_a_manifest_glob_does_not_cross_a_slash() { assert_eq!( @@ -1776,14 +1926,67 @@ mod tests { assert!(matched(&["apps/*/Cargo.toml"]).is_empty()); } + /// A flag repeated is one request, not two. clap's `Vec` keeps every + /// repeat, so `--ecosystem rust --ecosystem rust` reached the empty-selection + /// line as `no manifest for Rust, Rust`. Selection was never affected — it is + /// a `contains` — so this is the diagnostic alone. + #[test] + fn a_repeated_ecosystem_is_named_once() { + use crate::cli::EcosystemArg; + + assert_eq!( + requested_ecosystems(&[EcosystemArg::Rust, EcosystemArg::Rust]), + vec![Ecosystem::Rust] + ); + assert_eq!( + ecosystem_names(&requested_ecosystems(&[ + EcosystemArg::Npm, + EcosystemArg::Rust, + EcosystemArg::Npm, + ])), + "npm, Rust", + "first-named order survives, and nothing is said twice" + ); + // An absent flag is still the unrestricted answer. + assert!(requested_ecosystems(&[]).is_empty()); + } + + /// Both filters at once, which nothing else exercises: `--ecosystem` and + /// `--manifest-glob` intersect rather than override, whichever is narrower. + /// + /// This pins the *set*, and the set alone cannot pin the order the two run + /// in — an intersection is commutative, so both orders return this. What the + /// order changes is the diagnostic each filter prints, which is asserted end + /// to end in `tests/cli_ecosystem.rs`. + #[test] + fn an_ecosystem_and_a_glob_narrow_the_same_set() { + let root = polyglot(); + assert_eq!( + matched_under(&root, &["services/*/*"], &[]), + vec!["services/api/Cargo.toml", "services/sync/go.mod"], + "the glob alone keeps both services" + ); + assert_eq!( + matched_under(&root, &["services/*/*"], &[Ecosystem::Rust]), + vec!["services/api/Cargo.toml"], + "adding the ecosystem removes the Go module the glob had kept" + ); + assert!( + matched_under(&root, &["services/sync/go.mod"], &[Ecosystem::Rust]).is_empty(), + "and a glob naming only an excluded manifest keeps nothing" + ); + } + #[test] fn an_explicit_manifest_is_returned_whatever_the_patterns() { // clap rejects the combination, so this only documents that the glob // never silently filters away a file the user named outright. let named = PathBuf::from("some/other/Cargo.toml"); assert_eq!( - collect_manifests(Some(&named), None, 3, &["nope/*".to_string()], &|_| true) - .expect("the pattern is valid"), + collect_manifests(Some(&named), None, 3, &["nope/*".to_string()], &[], &|_| { + true + }) + .expect("the pattern is valid"), vec![named] ); } @@ -1792,8 +1995,15 @@ mod tests { fn an_unparseable_pattern_is_an_error_not_an_empty_result() { let root = monorepo(); assert!( - collect_manifests(None, Some(&root), 4, &["services/[".to_string()], &|_| true) - .is_err() + collect_manifests( + None, + Some(&root), + 4, + &["services/[".to_string()], + &[], + &|_| true + ) + .is_err() ); } diff --git a/crates/dependable/tests/cli_ecosystem.rs b/crates/dependable/tests/cli_ecosystem.rs new file mode 100644 index 0000000..c6f9b76 --- /dev/null +++ b/crates/dependable/tests/cli_ecosystem.rs @@ -0,0 +1,375 @@ +//! End-to-end: `--ecosystem` narrows which manifests a run reads. +//! +//! Hermetic. Every assertion is made against `list --format json`, which parses +//! from disk and never reaches a registry, or against a `check`/`fix` invocation +//! whose selection is empty or Rust-only over a tree with no lockfile to resolve. +//! The point being pinned is that the flag changes the *inventory* — the failure +//! mode of the flag this replaces was that it parsed, was advertised, and was read +//! by nothing. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +use serde_json::Value; + +const CARGO_TOML: &str = + "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n[dependencies]\nserde = \"1.0.100\"\n"; +const PACKAGE_JSON: &str = "{\n \"name\": \"web\",\n \"version\": \"0.1.0\",\n \"dependencies\": { \"react\": \"^18.0.0\" }\n}\n"; +const CARGO_TOML_NO_DEPS: &str = "[package]\nname = \"solo\"\nversion = \"0.1.0\"\n"; +const GO_MOD: &str = "module example.com/svc\n\ngo 1.21\n\nrequire github.com/google/uuid v1.6.0\n"; +const MIX_EXS: &str = "defmodule Sample.MixProject do\n use Mix.Project\n defp deps do\n [{:phoenix, \"~> 1.7\"}]\n end\nend\n"; +const DENO_JSON: &str = "{\n \"imports\": { \"chalk\": \"npm:chalk@^5.3.0\" }\n}\n"; +const PNPM_WORKSPACE: &str = "packages:\n - 'packages/*'\n\ncatalog:\n lodash: \"4.17.21\"\n"; + +/// A scratch directory of its own per test, under Cargo's per-target temp dir. +fn workdir(name: &str) -> PathBuf { + let dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")) + .join("ecosystem") + .join(name); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).expect("create the scratch directory"); + dir +} + +fn write(dir: &Path, rel: &str, content: &str) { + let path = dir.join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).expect("create the parent directory"); + } + fs::write(path, content).expect("write the manifest"); +} + +/// A polyglot repository: one project per ecosystem, each in its own directory so +/// that nothing about the layout depends on two manifests sharing a path. +fn polyglot(name: &str) -> PathBuf { + let dir = workdir(name); + write(&dir, "rust/Cargo.toml", CARGO_TOML); + write(&dir, "web/package.json", PACKAGE_JSON); + write(&dir, "svc/go.mod", GO_MOD); + dir +} + +/// Colour is pinned off rather than inherited: this repository has had tests go +/// red from an ambient `FORCE_COLOR` in the developer's shell, and `--help` is +/// asserted on as text. +fn run(args: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_dependable")) + .args(args) + .env("NO_COLOR", "1") + .env_remove("FORCE_COLOR") + .env_remove("CLICOLOR_FORCE") + .env_remove("COLORTERM") + .env_remove("DEPENDABLE_FAIL_ON") + .output() + .expect("run dependable") +} + +fn list_json(dir: &Path, extra: &[&str]) -> Value { + let mut args = vec![ + "list", + dir.to_str().expect("utf-8 path"), + "--format", + "json", + ]; + args.extend_from_slice(extra); + let output = run(&args); + assert!( + output.status.success(), + "list failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).expect("valid JSON") +} + +/// The manifest path of every project in the document, `/`-separated as the +/// schema promises, sorted so the assertion does not depend on walk order. +fn manifests(doc: &Value) -> Vec { + let mut paths: Vec = doc["projects"] + .as_array() + .expect("projects array") + .iter() + .map(|p| p["manifest"].as_str().expect("a manifest path").to_owned()) + .collect(); + paths.sort(); + paths +} + +/// The acceptance boundary. The flag this replaces parsed, appeared in `--help` +/// claiming to restrict the run, and was read by nothing — so the one thing worth +/// asserting is that the *inventory* is different, not that the flag is accepted. +#[test] +fn ecosystem_narrows_the_inventory_to_what_was_asked_for() { + let dir = polyglot("narrows"); + + let all = list_json(&dir, &[]); + assert_eq!( + manifests(&all), + ["rust/Cargo.toml", "svc/go.mod", "web/package.json"], + "without the flag every ecosystem is read" + ); + + let rust = list_json(&dir, &["--ecosystem", "rust"]); + assert_eq!(manifests(&rust), ["rust/Cargo.toml"]); + assert_eq!(rust["summary"]["projects"], 1); + assert_eq!( + rust["summary"]["by_ecosystem"], + serde_json::json!({ "Rust": 1 }), + "the summary counts what was read, not what was on disk" + ); +} + +/// An ecosystem is not a filename. `Npm` owns three manifest spellings, and +/// filtering by ecosystem has to cover all of them without the user naming any. +#[test] +fn an_ecosystem_covers_every_manifest_spelling_it_owns() { + let dir = workdir("spellings"); + write(&dir, "rust/Cargo.toml", CARGO_TOML); + write(&dir, "web/package.json", PACKAGE_JSON); + write(&dir, "edge/deno.json", DENO_JSON); + write(&dir, "mono/pnpm-workspace.yaml", PNPM_WORKSPACE); + + let doc = list_json(&dir, &["--ecosystem", "npm"]); + assert_eq!( + manifests(&doc), + [ + "edge/deno.json", + "mono/pnpm-workspace.yaml", + "web/package.json" + ], + "one ecosystem, three spellings, and no Cargo.toml" + ); +} + +/// Two values are a union, not a contradiction — the same reading +/// `--manifest-glob` gives a repeated pattern. +#[test] +fn two_ecosystems_are_a_union_not_a_contradiction() { + let dir = polyglot("union"); + let doc = list_json(&dir, &["--ecosystem", "rust", "--ecosystem", "npm"]); + assert_eq!(manifests(&doc), ["rust/Cargo.toml", "web/package.json"]); +} + +/// An ecosystem nobody has a manifest for is an answer, not a tool error: exit 0, +/// and a line saying what was searched and what was there instead. The generic +/// "No supported manifests found." would be a falsehood here — the repository is +/// full of manifests, the filter removed them — so it must not also be printed. +/// +/// `check` reaches no registry because the selection is empty before any fetcher +/// is constructed, which is also what makes this assertable offline. +#[test] +fn check_narrows_discovery_without_touching_the_network() { + let dir = workdir("empty_selection"); + write(&dir, "mix.exs", MIX_EXS); + + let output = run(&[ + "check", + dir.to_str().expect("utf-8 path"), + "--ecosystem", + "rust", + "--no-vuln", + ]); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + output.status.success(), + "an empty selection is exit 0: {stderr}" + ); + assert!( + stderr.contains("no manifest for Rust"), + "say what was asked for: {stderr}" + ); + assert!( + stderr.contains("Elixir"), + "and what was there instead: {stderr}" + ); + assert!( + !stderr.contains("No supported manifests found."), + "the generic line contradicts the specific one: {stderr}" + ); +} + +/// `--manifest` names one file and skips discovery, so there is no discovered set +/// for an ecosystem to narrow. clap rejects the pair rather than letting one of +/// them be silently ignored — the same contract `--manifest-glob` has. +#[test] +fn ecosystem_and_manifest_are_mutually_exclusive() { + let output = run(&["list", "--manifest", "Cargo.toml", "--ecosystem", "rust"]); + assert!(!output.status.success(), "the combination must be rejected"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("--ecosystem"), "{stderr}"); +} + +/// A flag whose accepted values are not advertised is a guessing game, and the +/// one value clap would otherwise spell wrong (`c-sharp`) is the reason to pin +/// each of them rather than the flag name alone. +/// +/// The check is scoped to clap's own list of accepted values, so it cannot pass +/// on an incidental substring elsewhere in the help — `go` appears inside +/// `cargo`. +#[test] +fn the_flag_is_advertised_with_the_values_it_accepts() { + for command in ["check", "list", "fix"] { + let output = run(&[command, "--help"]); + assert!(output.status.success(), "{command} --help failed"); + let help = String::from_utf8_lossy(&output.stdout); + assert!(help.contains("--ecosystem"), "{command}: {help}"); + + let advertised = help + .lines() + .find(|line| line.trim_start().starts_with("[possible values:")) + .unwrap_or_else(|| panic!("{command} --help must list the accepted values: {help}")); + for value in [ + "rust", "go", "npm", "python", "php", "dart", "csharp", "elixir", "jvm", + ] { + assert!( + advertised.contains(value), + "{command} --help must advertise `{value}`: {advertised}" + ); + } + } +} + +/// Discovery's unread-manifest notices are advice to *enable* something. Telling +/// someone to wire up Gradle during a run they restricted to Rust is noise they +/// asked not to receive, so the request is composed into the notice predicate as +/// well as into the manifest set. +#[test] +fn an_ecosystem_filter_silences_advice_about_ecosystems_it_excluded() { + let dir = workdir("notices"); + write(&dir, "rust/Cargo.toml", CARGO_TOML); + write(&dir, "legacy/build.gradle", "dependencies { }\n"); + + let path = dir.to_str().expect("utf-8 path"); + let default = run(&["list", path, "--format", "json"]); + let default = String::from_utf8_lossy(&default.stderr).into_owned(); + assert!( + default.contains("build.gradle"), + "an unread Gradle build is worth saying so about: {default}" + ); + + let filtered = run(&["list", path, "--format", "json", "--ecosystem", "rust"]); + assert!(filtered.status.success()); + let filtered = String::from_utf8_lossy(&filtered.stderr).into_owned(); + assert!( + !filtered.contains("build.gradle"), + "`--ecosystem rust` is an answer about the JVM, not a question: {filtered}" + ); +} + +/// `fix` writes to the user's files. Without the flag here, `dependable fix` +/// would rewrite manifests the matching `dependable check` deliberately left out +/// — the stated reason `--manifest-glob` is on `fix` at all. +/// +/// Hermetic by construction rather than by mocking: the only Rust manifest in the +/// tree declares no dependencies, so the narrowed run has nothing to look up, and +/// the npm and Go manifests it excluded are the ones that would have gone to a +/// registry. A `fix` that ignored `--ecosystem` would therefore also be the one +/// that reached the network. +#[test] +fn fix_rewrites_only_the_ecosystem_it_was_pointed_at() { + let dir = workdir("fix_scope"); + write(&dir, "rust/Cargo.toml", CARGO_TOML_NO_DEPS); + write(&dir, "web/package.json", PACKAGE_JSON); + write(&dir, "svc/go.mod", GO_MOD); + + let output = run(&[ + "fix", + dir.to_str().expect("utf-8 path"), + "--ecosystem", + "rust", + "--dry-run", + "--no-vuln", + ]); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + + assert!(output.status.success(), "{combined}"); + assert!( + !combined.contains("go.mod"), + "a manifest outside the requested ecosystem must not be considered: {combined}" + ); + assert!( + !combined.contains("package.json"), + "nor an npm one: {combined}" + ); + + // And nothing on disk moved — `--dry-run`, and the excluded files were never + // even read. + assert_eq!( + fs::read_to_string(dir.join("svc/go.mod")).expect("read go.mod"), + GO_MOD + ); + assert_eq!( + fs::read_to_string(dir.join("web/package.json")).expect("read package.json"), + PACKAGE_JSON + ); +} + +/// The ecosystem filter runs **before** the glob filter, so the glob's +/// "matched nothing" line counts only the manifests still in play. +/// +/// The order is invisible in the result: both filters are set intersections, so +/// either order returns the same manifests. It is visible only in what is +/// printed, which is why this is asserted here on stderr rather than as a unit +/// test on the returned set. Swapping the two blocks in `collect_manifests` — +/// an easy move, both are a `filter`/`collect` over `found` — makes both halves +/// of this test fail. +#[test] +fn the_ecosystem_filter_runs_before_the_glob_filter() { + let dir = polyglot("order"); + let path = dir.to_str().expect("utf-8 path"); + + // A glob naming a manifest the ecosystem filter has already removed. Filtered + // in this order, one manifest survives to be globbed and nothing matches; + // globbed first, `svc/go.mod` matches, the ecosystem filter then empties the + // set, and the run reports an ecosystem failure instead of a glob one. + let output = run(&[ + "list", + path, + "--format", + "json", + "--ecosystem", + "rust", + "--manifest-glob", + "svc/go.mod", + ]); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stderr}"); + assert!( + stderr.contains("no manifest matched svc/go.mod (searched 1 manifest up to --depth 3)"), + "the glob must report against the narrowed set, and be the filter that came back empty: {stderr}" + ); + assert!( + !stderr.contains("no manifest for Rust"), + "Rust was found; the glob is what selected nothing: {stderr}" + ); + + // And a glob that matches nothing at all, which is where the wrong order + // prints two contradicting lines: a glob line counting the three manifests on + // disk, and then — `found` now being empty — `no manifest for Rust (searched 0 + // manifests ...)` about a tree that contains Rust. + let output = run(&[ + "list", + path, + "--format", + "json", + "--ecosystem", + "rust", + "--manifest-glob", + "nope/*", + ]); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stderr}"); + assert!( + stderr.contains("no manifest matched nope/* (searched 1 manifest up to --depth 3)"), + "the count is the manifests still in play, not the ones on disk: {stderr}" + ); + assert!( + !stderr.contains("no manifest for Rust"), + "one empty selection is one diagnostic: {stderr}" + ); +} diff --git a/crates/dependable/tests/cli_list.rs b/crates/dependable/tests/cli_list.rs index 7a368f7..f032c64 100644 --- a/crates/dependable/tests/cli_list.rs +++ b/crates/dependable/tests/cli_list.rs @@ -326,3 +326,45 @@ fn a_disabled_ecosystem_is_not_warned_about() { "`[jvm] enabled = false` is an answer, not a question: {disabled}" ); } + +/// An empty selection is exit 0 — and under `--format json` it is still a +/// document. +/// +/// `list --ecosystem csharp --format json | jq '.summary.projects'` is the shape +/// this flag exists for: one shard per ecosystem in a CI matrix. Returning +/// before the document was built made every shard whose ecosystem the repository +/// does not use exit 0 with byte-empty stdout, so `jq` failed to parse — on +/// precisely the ecosystems exit 0 was chosen to keep green. The stderr line +/// saying what was searched is not machine-readable. +/// +/// `table` and `text` are unchanged: a human handed an empty table wants the +/// stderr line, not a blank one. +#[test] +fn an_empty_selection_is_an_empty_document_not_empty_stdout() { + let npm = fixture("sample-npm"); + + let filtered = list_json(&npm, &["--ecosystem", "rust"]); + assert_eq!(filtered["schema"], "dependable.list/v1"); + assert_eq!(filtered["summary"]["projects"], 0); + assert_eq!(filtered["summary"]["dependencies"], 0); + assert_eq!(filtered["summary"]["by_ecosystem"], serde_json::json!({})); + assert!( + filtered["projects"] + .as_array() + .expect("projects array") + .is_empty() + ); + + // The other way a selection empties: the glob path, which prints its own + // diagnostic and reached the same byte-empty stdout. + let globbed = list_json(&npm, &["--manifest-glob", "nope/*"]); + assert_eq!(globbed["summary"]["projects"], 0); + + let path = npm.to_str().expect("utf-8 path"); + for format in ["table", "text"] { + assert!( + run(&["list", path, "--format", format, "--ecosystem", "rust"]).is_empty(), + "{format} says nothing on stdout when nothing was selected" + ); + } +} diff --git a/crates/dependable/tests/fixtures/sample-polyglot/README.md b/crates/dependable/tests/fixtures/sample-polyglot/README.md new file mode 100644 index 0000000..9f55e57 --- /dev/null +++ b/crates/dependable/tests/fixtures/sample-polyglot/README.md @@ -0,0 +1,6 @@ +A polyglot monorepo shape: two sibling services under `services/`, one Rust and +one Go, so that `--ecosystem` and `--manifest-glob` can be exercised against the +same tree — an ecosystem filter needs a manifest of another ecosystem to remove, +and `sample-monorepo` is Rust throughout. + +Parsed as data, never built (the root `Cargo.toml` excludes `tests/fixtures`). diff --git a/crates/dependable/tests/fixtures/sample-polyglot/services/api/Cargo.toml b/crates/dependable/tests/fixtures/sample-polyglot/services/api/Cargo.toml new file mode 100644 index 0000000..5d0c723 --- /dev/null +++ b/crates/dependable/tests/fixtures/sample-polyglot/services/api/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "api" +version = "0.1.0" +edition = "2021" + +[dependencies] +leftpad = "1" diff --git a/crates/dependable/tests/fixtures/sample-polyglot/services/sync/go.mod b/crates/dependable/tests/fixtures/sample-polyglot/services/sync/go.mod new file mode 100644 index 0000000..d28034f --- /dev/null +++ b/crates/dependable/tests/fixtures/sample-polyglot/services/sync/go.mod @@ -0,0 +1,5 @@ +module example.com/sync + +go 1.21 + +require github.com/google/uuid v1.6.0