From c35779658dcb867347818f66cb1ca508b2db9ae9 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 20:25:42 -0400 Subject: [PATCH 1/7] feat(cli): filter discovery by ecosystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--ecosystem` narrows the manifests `check`, `list`, and `fix` read to the ecosystems named, so one command can address one slice of a polyglot repository the way `--manifest-glob` addresses one slice of a monorepo. The flag this replaces parsed, was advertised in `--help` as restricting the run, and was read by nothing. Making the claim true takes two separate effects, and the load-bearing one is easy to miss: `dependable_fetch::discover` documents that its `enabled` predicate gates its unread-manifest notices only — discovery still returns every manifest it recognizes — so composing the request into that predicate alone would silence warnings and change nothing a command actually reads. `collect_manifests` therefore also filters the returned set, before the `--manifest-glob` filter so the glob's "matched nothing" line counts only what is still in play. Accepted values are the canonical ecosystem names and nothing else, so `--ecosystem foo` is now an error where the previous `Option` took it silently. `CSharp` is spelled `csharp`, not clap's derived `c-sharp`. The flag only ever narrows: naming an ecosystem that `.dependable.toml` disables does not switch it back on. Selecting nothing names which ecosystems were searched and which were found instead, and exits 0 — an unused ecosystem must not fail a per-ecosystem CI matrix job. --- README.md | 24 ++ crates/dependable/src/cli.rs | 127 ++++++++++ crates/dependable/src/runner.rs | 149 +++++++++-- crates/dependable/tests/cli_ecosystem.rs | 310 +++++++++++++++++++++++ 4 files changed, 593 insertions(+), 17 deletions(-) create mode 100644 crates/dependable/tests/cli_ecosystem.rs diff --git a/README.md b/README.md index cbfff05..3b5bda3 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,29 @@ 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. + +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/src/cli.rs b/crates/dependable/src/cli.rs index f3223e0..15d2d6d 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,106 @@ 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 pins that every ecosystem crosses it. +/// +/// 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; + + /// Every ecosystem, spelled out. [`Ecosystem`] is `#[non_exhaustive]`, so a + /// manual list is the strongest guard available — the same one + /// `dependable-core`'s own `ALL` uses. + const ALL: [Ecosystem; 9] = [ + Ecosystem::Rust, + Ecosystem::Go, + Ecosystem::Npm, + Ecosystem::Python, + Ecosystem::Php, + Ecosystem::Dart, + Ecosystem::CSharp, + Ecosystem::Elixir, + Ecosystem::Jvm, + ]; + + /// Adding an ecosystem without adding its `--ecosystem` value would leave a + /// supported ecosystem unfilterable, and — worse — leave `--ecosystem` unable + /// to say so. A missing variant fails here rather than at a user's prompt. + #[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, 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 0703f38..117f929 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, ListArgs, TreeArgs, TuiArgs}; use crate::config::{Config, load_config}; #[cfg(feature = "report")] use crate::config::{PolicySource, load_policy}; @@ -370,15 +370,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); } @@ -627,15 +629,17 @@ 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); return Ok(ExitCode::SUCCESS); } let mut reports = Vec::new(); @@ -885,15 +889,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); } @@ -1146,10 +1152,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); } @@ -1280,6 +1327,65 @@ fn collect_manifests( Ok(kept) } +/// The ecosystems `--ecosystem` asked for, as the core type. Empty means +/// unrestricted, which is what an absent flag produces. +fn requested_ecosystems(args: &[EcosystemArg]) -> Vec { + args.iter().copied().map(Ecosystem::from).collect() +} + +/// 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 when `--ecosystem` narrowed the set to nothing: [`collect_manifests`] +/// has already said which ecosystems were asked for and what was there instead, +/// and "No supported manifests found." is a falsehood in a repository full of +/// manifests the filter removed. 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. /// @@ -1604,7 +1710,7 @@ mod tests { fn matched(globs: &[&str]) -> 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, &[], &|_| true) .expect("the patterns are valid") .iter() .map(|m| output::posix(&relative_to(&root, m))) @@ -1656,8 +1762,10 @@ mod tests { // 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] ); } @@ -1666,8 +1774,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..733aeae --- /dev/null +++ b/crates/dependable/tests/cli_ecosystem.rs @@ -0,0 +1,310 @@ +//! 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 + ); +} From 4b510d90332ab1beabb46e88a9b46b6541ac8a02 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:46:45 -0400 Subject: [PATCH 2/7] feat(core): publish the ecosystem list as `Ecosystem::ALL` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list of every variant existed already, but only inside this module's test module, where nothing outside the crate could reach it. A frontend that must cover every ecosystem therefore had to write its own copy, and a copy is not a guard: the hand that forgets a variant is the same hand that would have updated the copy. Promote it to a public associated const, beside the exhaustive matches that a new variant genuinely does not compile past, so the one list a new ecosystem cannot avoid meeting is also the list consumers pin against. The doc comment says plainly that this is a prompt rather than a proof — `#[non_exhaustive]` and the absence of stable variant enumeration leave no way to make the compiler check the list itself. --- crates/dependable-core/src/ecosystem.rs | 50 ++++++++++++++++--------- 1 file changed, 33 insertions(+), 17 deletions(-) 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, From 194a08276f74efdea43365fdc10f787214af106a Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:46:52 -0400 Subject: [PATCH 3/7] fix(cli): pin the nameable ecosystems against `Ecosystem::ALL` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `every_ecosystem_can_be_named_on_the_command_line` compared `EcosystemArg::value_variants()` to a nine-element list written a few lines above it, in the same test module. Neither side referenced the real variant set, so adding an ecosystem changed neither and the test stayed green while `--ecosystem` lost the ability to name it — and `Ecosystem` being `#[non_exhaustive]` means no match in this crate breaks either, `Config::ecosystem_enabled`'s `_ => true` included. Compare against `Ecosystem::ALL` instead. It lives beside the exhaustive matches in the defining crate, which is where a new variant stops compiling, so the author of the variant is already in that file when the list asks to be updated — and once it is, this test goes red. Both doc comments overstated what was enforced ("the unit test beside it pins that every ecosystem crosses it", "A missing variant fails here rather than at a user's prompt"). They now say what the assertion actually rests on, and why a list local to the test would not do. --- crates/dependable/src/cli.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/crates/dependable/src/cli.rs b/crates/dependable/src/cli.rs index 15d2d6d..6c3dbbb 100644 --- a/crates/dependable/src/cli.rs +++ b/crates/dependable/src/cli.rs @@ -404,7 +404,9 @@ impl From for dependable_fetch::UnstableFilter { /// 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 pins that every ecosystem crosses it. +/// 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 @@ -451,31 +453,23 @@ mod tests { use dependable_fetch::Ecosystem; - /// Every ecosystem, spelled out. [`Ecosystem`] is `#[non_exhaustive]`, so a - /// manual list is the strongest guard available — the same one - /// `dependable-core`'s own `ALL` uses. - const ALL: [Ecosystem; 9] = [ - Ecosystem::Rust, - Ecosystem::Go, - Ecosystem::Npm, - Ecosystem::Python, - Ecosystem::Php, - Ecosystem::Dart, - Ecosystem::CSharp, - Ecosystem::Elixir, - Ecosystem::Jvm, - ]; - /// Adding an ecosystem without adding its `--ecosystem` value would leave a /// supported ecosystem unfilterable, and — worse — leave `--ecosystem` unable - /// to say so. A missing variant fails here rather than at a user's prompt. + /// 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, ALL.to_vec()); + assert_eq!(nameable, Ecosystem::ALL.to_vec()); } /// The one value whose derived spelling is wrong: clap kebab-cases `CSharp` From c7b82c401e75512539cde5567ca077e7b4aa5358 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:50:15 -0400 Subject: [PATCH 4/7] test(cli): pin that the ecosystem filter runs before the glob filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collect_manifests` documents the order as load-bearing — "so the glob's 'matched nothing' line counts only the manifests still in play" — and nothing tested it. No test passed both `--ecosystem` and `--manifest-glob`; the `matched` helper passed `&[]` for `ecosystems` on every call. Swapping the two blocks is an easy move: both are a `filter`/`collect` over `found`. Swapped, `list . --ecosystem rust --manifest-glob 'nope/*'` over a Cargo+npm+Go tree prints two contradicting lines — `no manifest matched nope/* (searched 3 manifests ...)` counting manifests the ecosystem filter was about to remove, and then, `found` being empty by that point, `no manifest for Rust (searched 0 manifests ...)` about a tree that contains Rust. The order is invisible in the result, because both filters are set intersections and either order returns the same manifests. So it is pinned where it shows: an end-to-end assertion on the diagnostics. The unit test alongside it covers the other gap — that the two filters compose at all — and says in its own doc comment that it cannot pin the order. `sample-polyglot` is a new fixture rather than an edit to `sample-monorepo`, which is Rust throughout and so has nothing for an ecosystem filter to remove. --- crates/dependable/src/runner.rs | 45 +++++++++++-- crates/dependable/tests/cli_ecosystem.rs | 65 +++++++++++++++++++ .../tests/fixtures/sample-polyglot/README.md | 6 ++ .../sample-polyglot/services/api/Cargo.toml | 7 ++ .../sample-polyglot/services/sync/go.mod | 5 ++ 5 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 crates/dependable/tests/fixtures/sample-polyglot/README.md create mode 100644 crates/dependable/tests/fixtures/sample-polyglot/services/api/Cargo.toml create mode 100644 crates/dependable/tests/fixtures/sample-polyglot/services/sync/go.mod diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index d7ba9c1..b1d326f 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -1833,16 +1833,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!( @@ -1882,6 +1893,32 @@ mod tests { assert!(matched(&["apps/*/Cargo.toml"]).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 diff --git a/crates/dependable/tests/cli_ecosystem.rs b/crates/dependable/tests/cli_ecosystem.rs index 733aeae..c6f9b76 100644 --- a/crates/dependable/tests/cli_ecosystem.rs +++ b/crates/dependable/tests/cli_ecosystem.rs @@ -308,3 +308,68 @@ fn fix_rewrites_only_the_ecosystem_it_was_pointed_at() { 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/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 From 2f53103dc2e776ef65d2a183409249227f8a87e7 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:51:19 -0400 Subject: [PATCH 5/7] fix(cli): give an empty `list --format json` selection a document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_list` reported the empty selection and returned before any JSON was constructed, so stdout was byte-empty. `list . --ecosystem csharp --format json | jq '.summary.projects'` — one shard per ecosystem, which is what this flag is for — then failed to parse on every shard whose ecosystem the repository does not use, which is precisely the set that exiting 0 was chosen to keep green. The stderr line saying what was searched is not machine-readable. Emit the empty `dependable.list/v1` document instead: same schema, zero projects, an empty `by_ecosystem`. Additive for any consumer, and a parseable answer beats no answer. `table` and `text` still return early. A human handed an empty table wants the stderr line, not a blank one. Both ways a selection can empty are covered — the ecosystem filter and the glob — because the byte-empty stdout predated `--ecosystem` and the glob path reached it too. --- README.md | 4 ++- crates/dependable/src/runner.rs | 15 ++++++++++- crates/dependable/tests/cli_list.rs | 42 +++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e7fdec4..ef0cb6a 100644 --- a/README.md +++ b/README.md @@ -285,7 +285,9 @@ in any named ecosystem is kept, it is available on `check`, `list`, and `fix` fo 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. +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 diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index b1d326f..bad9d85 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -25,7 +25,9 @@ use dependable_tui::TuiOptions; use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; use indicatif::{ProgressBar, ProgressStyle}; -use crate::cli::{CheckArgs, EcosystemArg, 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}; @@ -694,6 +696,17 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { )?; if manifests.is_empty() { 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(); 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" + ); + } +} From f9502f473a2a2c3667f7646c65795b7fbdfa18c5 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:52:11 -0400 Subject: [PATCH 6/7] fix(cli): say each requested ecosystem once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clap's `Vec` with `ArgAction::Append` keeps repeats, so `--ecosystem rust --ecosystem rust` reached the empty-selection line as `no manifest for Rust, Rust (...)`. Dedupe by membership — the same `Vec::contains` pattern `no_ecosystem_match` already uses to list what was found instead, `Ecosystem` being `Eq` but not `Ord` — which also keeps the order the user named them in. Selection is untouched: it was already a `contains`, so a repeat never changed which manifests were kept. This is the diagnostic alone. --- crates/dependable/src/runner.rs | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index bad9d85..7a4e6bf 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -25,9 +25,7 @@ use dependable_tui::TuiOptions; use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; use indicatif::{ProgressBar, ProgressStyle}; -use crate::cli::{ - CheckArgs, EcosystemArg, FailOn, FixArgs, Format, 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}; @@ -1423,8 +1421,21 @@ fn collect_manifests( /// 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 { - args.iter().copied().map(Ecosystem::from).collect() + 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 @@ -1906,6 +1917,31 @@ 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. /// From 905259a1472f7a94fff01afb427db7508c32483f Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:52:22 -0400 Subject: [PATCH 7/7] docs(cli): say what suppresses the generic no-manifest line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `report_no_manifests` claimed it is silent because `collect_manifests` "has already said which ecosystems were asked for and what was there instead". The suppression is keyed on `ecosystems.is_empty()` — on an ecosystem filter having been *asked for*, not on that filter being what emptied the set. `--ecosystem rust --manifest-glob 'nope/*'` over a repository that does contain Rust is emptied by the glob; the ecosystem explanation never printed, and the generic line was suppressed on a justification that had not occurred. The behaviour is right — the glob printed its own specific line, and "No supported manifests found." would still be false in a repository full of manifests a filter removed — so only the comment changes. It now describes what the predicate tests and says why it is the coarse one: a caller cannot tell the two cases apart without `collect_manifests` reporting back which filter emptied the set, and that return type stays `Vec` on purpose. --- crates/dependable/src/runner.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 7a4e6bf..d6a796d 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -1479,12 +1479,21 @@ fn ecosystem_names(ecosystems: &[Ecosystem]) -> String { /// The line a command prints when discovery came back with nothing to do. /// -/// Silent when `--ecosystem` narrowed the set to nothing: [`collect_manifests`] -/// has already said which ecosystems were asked for and what was there instead, -/// and "No supported manifests found." is a falsehood in a repository full of -/// manifests the filter removed. 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. +/// 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.");