From 55091f080f074283e892c62627ea11946c20d536 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 22:35:15 -0400 Subject: [PATCH 01/25] feat(core): parse pom.xml literal and same-file property versions A Maven POM is declarative XML, so the dependencies it states can be read without running anything. `` directly under `` yields one `groupId:artifactId` per entry, with the byte span of the `` text so `--fix` can rewrite it where it is written. `${property}` resolves against `` in the same file, following the Gradle catalog's rule for the same reason: a property used by exactly one dependency carries the `` span that governs it, while one several dependencies share carries none, because a single line cannot be rewritten to two versions. `` inheritance, ``, and BOM imports stay out: resolving any of them can require fetching the parent POM from a registry, which would make this a resolution engine rather than a parser. A dependency those rules leave unresolved is reported with no constraint instead of being dropped, so a POM that inherits half its versions is never presented as depending on only the other half. --- crates/dependable-core/src/lib.rs | 7 +- crates/dependable-core/src/manifest.rs | 9 +- crates/dependable-core/src/parsers/mod.rs | 3 + crates/dependable-core/src/parsers/pom_xml.rs | 672 ++++++++++++++++++ crates/dependable-core/src/parsers/project.rs | 74 ++ 5 files changed, 761 insertions(+), 4 deletions(-) create mode 100644 crates/dependable-core/src/parsers/pom_xml.rs diff --git a/crates/dependable-core/src/lib.rs b/crates/dependable-core/src/lib.rs index a31f285..450c96c 100644 --- a/crates/dependable-core/src/lib.rs +++ b/crates/dependable-core/src/lib.rs @@ -37,9 +37,10 @@ pub use parsers::{ AutoTargets, CargoPackageManifest, CargoTarget, CargoTargetKind, CargoTomlParser, CfgDependencyTable, ComposerJsonParser, CsprojParser, DenoJsonParser, DependencySection, GoModParser, GradleCatalogParser, MixExsParser, PackageField, PackageJsonParser, Parser, - PnpmWorkspaceParser, ProjectMeta, ProjectRole, PubspecYamlParser, PyprojectTomlParser, - RequirementsTxtParser, WorkspaceDecl, parse, parse_cargo_config, parse_package_manifest, - parse_package_name, parse_project, parse_workspace, resolve_workspace_inheritance, + PnpmWorkspaceParser, PomXmlParser, ProjectMeta, ProjectRole, PubspecYamlParser, + PyprojectTomlParser, RequirementsTxtParser, WorkspaceDecl, parse, parse_cargo_config, + parse_package_manifest, parse_package_name, parse_project, parse_workspace, + resolve_workspace_inheritance, }; pub use result::{CheckResult, DependencyStatus}; pub use semver::{Evaluation, UnstableFilter, check_version, is_prerelease, to_semver_constraint}; diff --git a/crates/dependable-core/src/manifest.rs b/crates/dependable-core/src/manifest.rs index 3c42ea8..4217475 100644 --- a/crates/dependable-core/src/manifest.rs +++ b/crates/dependable-core/src/manifest.rs @@ -48,6 +48,7 @@ pub enum ManifestKind { MixExs, Csproj, GradleVersionCatalog, + PomXml, } impl ManifestKind { @@ -65,7 +66,7 @@ impl ManifestKind { ManifestKind::PubspecYaml => Ecosystem::Dart, ManifestKind::MixExs => Ecosystem::Elixir, ManifestKind::Csproj => Ecosystem::CSharp, - ManifestKind::GradleVersionCatalog => Ecosystem::Jvm, + ManifestKind::GradleVersionCatalog | ManifestKind::PomXml => Ecosystem::Jvm, } } @@ -179,6 +180,7 @@ impl ManifestKind { "pubspec.yaml" => ManifestKind::PubspecYaml, "mix.exs" => ManifestKind::MixExs, "Directory.Packages.props" => ManifestKind::Csproj, + "pom.xml" => ManifestKind::PomXml, // Gradle reads every `*.versions.toml` under `gradle/` as a catalog; // `libs` is only the conventional name of the default one. _ if name.ends_with(".versions.toml") => ManifestKind::GradleVersionCatalog, @@ -373,6 +375,7 @@ mod tests { "gradle/deps.versions.toml", ManifestKind::GradleVersionCatalog, ), + ("services/api/pom.xml", ManifestKind::PomXml), ]; for (path, expected) in cases { assert_eq!( @@ -435,6 +438,7 @@ mod tests { ManifestKind::MixExs, ManifestKind::Csproj, ManifestKind::GradleVersionCatalog, + ManifestKind::PomXml, ] { assert!(kind.workspace_roots().is_none(), "{kind:?}"); assert!( @@ -484,6 +488,9 @@ mod tests { ); } assert!(ManifestKind::CargoToml.unreadable_manifests().is_empty()); + // A `pom.xml` is data and reads fine; what it cannot resolve is reported + // entry by entry, so there is nothing here to declare unreadable. + assert!(ManifestKind::PomXml.unreadable_manifests().is_empty()); } #[test] diff --git a/crates/dependable-core/src/parsers/mod.rs b/crates/dependable-core/src/parsers/mod.rs index 19b2937..41fd4e4 100644 --- a/crates/dependable-core/src/parsers/mod.rs +++ b/crates/dependable-core/src/parsers/mod.rs @@ -20,6 +20,7 @@ pub mod json_scan; pub mod mix_exs; pub mod package_json; pub mod pnpm_workspace; +pub mod pom_xml; pub mod position; pub mod project; pub mod pubspec_yaml; @@ -42,6 +43,7 @@ pub use gradle_catalog::GradleCatalogParser; pub use mix_exs::MixExsParser; pub use package_json::PackageJsonParser; pub use pnpm_workspace::PnpmWorkspaceParser; +pub use pom_xml::PomXmlParser; pub use project::{ProjectMeta, ProjectRole, parse_project}; pub use pubspec_yaml::PubspecYamlParser; pub use pyproject_toml::PyprojectTomlParser; @@ -68,5 +70,6 @@ pub fn parse(kind: ManifestKind, content: &str) -> Result CsprojParser.parse(content), ManifestKind::MixExs => MixExsParser.parse(content), ManifestKind::GradleVersionCatalog => GradleCatalogParser.parse(content), + ManifestKind::PomXml => PomXmlParser.parse(content), } } diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs new file mode 100644 index 0000000..74479e8 --- /dev/null +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -0,0 +1,672 @@ +//! Parser for a Maven POM (`pom.xml`). +//! +//! The declarative half of a Maven build. A read-only DOM walk (`roxmltree`) over +//! the `` element directly under ``, taking the coordinate +//! from ``/`` and the constraint from ``. The exact +//! byte range of the version *text* is recorded for `--fix` — the one difference +//! from [`csproj`](super::csproj), where a version is an attribute and the span +//! comes from `Attribute::range_value`. +//! +//! # `${property}` +//! +//! A version may name a property instead of stating one: +//! +//! ```xml +//! +//! 4.12.0 +//! +//! +//! +//! com.squareup.okhttp3 +//! okhttp +//! ${okhttp.version} +//! +//! +//! ``` +//! +//! That is the same shape as a Gradle catalog's `version.ref`, and it is resolved +//! the same way and for the same reason: the literal is in **this** file, so the +//! resolution needs no IO and the span of the `` line is a real place +//! to rewrite. A property used by exactly one dependency carries that span; a +//! property several dependencies share carries none, because one line cannot be +//! rewritten to two different versions. Such a dependency is +//! [`PackageSource::Inherited`]: checked and scanned for advisories, never written +//! to. Only a version that is *entirely* one `${…}` is resolved — a composed value +//! like `1.${minor}` has no span that could be replaced with a version. +//! +//! # What is deliberately not resolved +//! +//! `` inheritance, ``, and BOM imports are out of +//! scope: resolving any of them correctly can require fetching the parent POM from +//! a registry, which makes this a resolution engine rather than a parser. Maven's +//! built-in properties (`${project.version}`, `${revision}`, …) are not +//! `` entries and are likewise not resolved. +//! +//! A dependency whose version those rules leave unknown is **reported anyway**, +//! with no constraint and no span — the shape a Cargo member's unresolved +//! `dep.workspace = true` already has, which the CLI renders as `(unresolved)` and +//! never fetches, fixes, or claims a status for. Dropping it instead, as the +//! `csproj` parser drops an MSBuild `$(…)` version, would report a POM that +//! inherits half its versions as depending on only the other half. The +//! [`unreadable_manifests`](crate::manifest::ManifestKind::unreadable_manifests) +//! notice is the wrong surface for this: it is keyed on a file *name*, and a +//! `pom.xml` is perfectly readable — it is individual entries within one that are +//! not. + +use std::collections::HashMap; +use std::ops::Range; + +use super::Parser; +use super::position::{line_starts, offset_to_line_col}; +use crate::error::ParseError; +use crate::item::{DependencyKind, Item, PackageSource}; +use crate::manifest::{ManifestKind, ParsedManifest}; + +/// How far a `${a}` → `${b}` → literal chain is followed before giving up, which +/// also bounds a property that (illegally) refers to itself. +const MAX_PROPERTY_HOPS: usize = 8; + +/// Parses `pom.xml`. +pub struct PomXmlParser; + +/// A version literal and where it is written, before it is known whether the +/// dependency that uses it may rewrite it. +struct Located { + value: String, + span: Option>, +} + +/// Where one dependency's version comes from. +enum Source { + /// Stated on the dependency itself. + Literal(Located), + /// Deferred to a `` entry, named here by the property the chain + /// ends at — the one whose line a rewrite would have to touch. + Property(String), + /// Not knowable from this file alone: absent, a built-in property, a composed + /// value, or a property this POM does not declare. + Unknown, +} + +/// One ``, read but not yet resolved. +struct Declared { + name: String, + version: Source, + source: PackageSource, + kind: DependencyKind, +} + +impl Parser for PomXmlParser { + fn parse(&self, content: &str) -> Result { + let doc = roxmltree::Document::parse(content) + .map_err(|e| ParseError::Structural(e.to_string()))?; + let starts = line_starts(content); + let project = doc.root_element(); + + let properties = read_properties(project); + let declared = read_dependencies(project, &properties); + + // A property used by exactly one dependency is that dependency's own line + // to fix; one shared by several belongs to none of them. + let mut uses: HashMap<&str, usize> = HashMap::new(); + for entry in &declared { + if let Source::Property(name) = &entry.version { + *uses.entry(name.as_str()).or_default() += 1; + } + } + + let items = declared + .iter() + .map(|entry| match &entry.version { + Source::Literal(version) => item(entry, version, &starts), + Source::Property(name) => { + let located = &properties[name.as_str()]; + if uses.get(name.as_str()).copied() == Some(1) { + item(entry, located, &starts) + } else { + inherited(entry, &located.value) + } + } + Source::Unknown => unresolved(entry), + }) + .collect(); + + Ok(ParsedManifest { + kind: ManifestKind::PomXml, + items, + alternate_registries: Vec::new(), + }) + } +} + +/// Read `` into property name → literal, skipping any entry that only +/// points at another unknown. +fn read_properties<'a>(project: roxmltree::Node<'a, 'a>) -> HashMap { + let mut out = HashMap::new(); + let Some(table) = child(project, "properties") else { + return out; + }; + for entry in table.children().filter(roxmltree::Node::is_element) { + if let Some(located) = text_of(entry) { + out.insert(entry.tag_name().name().to_owned(), located); + } + } + out +} + +/// Read the `` directly under ``, in source order. +/// +/// Only that one: `` states versions for dependencies +/// declared elsewhere, `` describes the build rather than the +/// artifact, and `` applies conditionally. None of the three is a +/// dependency of this project as written. +fn read_dependencies<'a>( + project: roxmltree::Node<'a, 'a>, + properties: &HashMap, +) -> Vec { + let mut out = Vec::new(); + let Some(list) = child(project, "dependencies") else { + return out; + }; + for node in list.children().filter(roxmltree::Node::is_element) { + if node.tag_name().name() != "dependency" { + continue; + } + // Without both halves there is no coordinate to look up, and nothing that + // could be reported under a name. A `${…}` group is interpolated first, + // since `${project.groupId}` aside, a group is often a property. + let (Some(group), Some(artifact)) = ( + interpolated(node, "groupId", properties), + interpolated(node, "artifactId", properties), + ) else { + continue; + }; + let scope = child(node, "scope") + .and_then(text_of) + .map(|located| located.value) + .unwrap_or_default(); + out.push(Declared { + name: format!("{group}:{artifact}"), + version: version_source(node, properties), + // A `system` dependency is a jar at a path on this machine, not + // something a registry has ever heard of. + source: match scope.as_str() { + "system" => PackageSource::Local, + _ => PackageSource::Registry, + }, + kind: dependency_kind(node, &scope), + }); + } + out +} + +/// Which section a `` belongs to, read from `` and +/// `` rather than guessed. +/// +/// `test` is the only scope that names a section [`DependencyKind`] has: `provided` +/// and `runtime` both describe a runtime dependency whose *provider* differs, which +/// is not the distinction this records. +fn dependency_kind(node: roxmltree::Node<'_, '_>, scope: &str) -> DependencyKind { + if scope == "test" { + return DependencyKind::Dev; + } + let optional = child(node, "optional") + .and_then(text_of) + .is_some_and(|located| located.value == "true"); + if optional { + DependencyKind::Optional + } else { + DependencyKind::Normal + } +} + +/// Where a ``'s version comes from, as far as this file can say. +fn version_source(node: roxmltree::Node<'_, '_>, properties: &HashMap) -> Source { + // No `` at all: supplied by `` or a ``, + // neither of which is read here. + let Some(located) = child(node, "version").and_then(text_of) else { + return Source::Unknown; + }; + let Some(reference) = interpolation(&located.value) else { + // A composed value (`1.${minor}`) states no version this file can rewrite. + return if located.value.contains('$') { + Source::Unknown + } else { + Source::Literal(located) + }; + }; + match terminal(reference, properties) { + Some(name) => Source::Property(name.to_owned()), + None => Source::Unknown, + } +} + +/// The name of the property a `${…}` chain ends at — the one that states a literal. +/// +/// `None` when the chain leaves this file, states nothing, or does not terminate. +fn terminal<'a>(start: &str, properties: &'a HashMap) -> Option<&'a str> { + let mut name = start; + for _ in 0..MAX_PROPERTY_HOPS { + let (key, located) = properties.get_key_value(name)?; + match interpolation(&located.value) { + Some(next) => name = next, + None if located.value.contains('$') => return None, + None => return Some(key.as_str()), + } + } + None +} + +/// The inner name of a value that is *entirely* one `${…}` reference. +fn interpolation(value: &str) -> Option<&str> { + let inner = value.strip_prefix("${")?.strip_suffix('}')?; + if inner.is_empty() || inner.contains(['$', '{', '}']) { + return None; + } + Some(inner) +} + +/// A child element's text, with `${…}` resolved against `properties`. +fn interpolated( + node: roxmltree::Node<'_, '_>, + tag: &str, + properties: &HashMap, +) -> Option { + let located = child(node, tag).and_then(text_of)?; + match interpolation(&located.value) { + Some(reference) => Some(properties[terminal(reference, properties)?].value.clone()), + None if located.value.contains('$') => None, + None => Some(located.value), + } +} + +/// The first direct child element named `tag`. +fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option> { + node.children() + .find(|child| child.is_element() && child.tag_name().name() == tag) +} + +/// An element's text content and the byte span of the text itself, trimmed of the +/// whitespace a pretty-printed POM puts around it. +/// +/// The span is dropped where the source text and the unescaped text cannot be the +/// same bytes — a character reference, or text split across several nodes — because +/// an offset into one is not an offset into the other. An empty element yields +/// nothing: it states no value, which is not the same as stating one this parser +/// resolved. +fn text_of(node: roxmltree::Node<'_, '_>) -> Option { + let mut texts = node.children().filter(roxmltree::Node::is_text); + let text = texts.next()?; + let raw = text.text()?; + let value = raw.trim(); + if value.is_empty() { + return None; + } + let range = text.range(); + let faithful = texts.next().is_none() && range.len() == raw.len(); + let start = range.start + (raw.len() - raw.trim_start().len()); + Some(Located { + value: value.to_owned(), + span: faithful.then(|| start..start + value.len()), + }) +} + +/// An item pointing at the version literal that governs it, wherever in this file +/// that literal is written. +fn item(entry: &Declared, version: &Located, starts: &[usize]) -> Item { + let Some(span) = &version.span else { + return inherited(entry, &version.value); + }; + let (line, col_start) = offset_to_line_col(starts, span.start); + Item { + name: entry.name.clone(), + version_constraint: version.value.clone(), + source: entry.source, + version_line: line, + version_col_start: col_start, + version_col_end: col_start + span.len(), + registry: None, + locked_version: None, + kind: entry.kind, + } +} + +/// An item whose version is known but whose line is not this dependency's to +/// rewrite — a `` entry several dependencies share. +fn inherited(entry: &Declared, version: &str) -> Item { + Item { + name: entry.name.clone(), + version_constraint: version.to_owned(), + source: PackageSource::Inherited, + version_line: 0, + version_col_start: 0, + version_col_end: 0, + registry: None, + locked_version: None, + kind: entry.kind, + } +} + +/// An item this file states no version for: it is reported, and nothing is claimed +/// about it. +fn unresolved(entry: &Declared) -> Item { + inherited(entry, "") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(content: &str) -> ParsedManifest { + PomXmlParser.parse(content).unwrap() + } + + fn sliced<'a>(content: &'a str, item: &Item) -> &'a str { + let line = content.lines().nth(item.version_line).unwrap(); + &line[item.version_col_start..item.version_col_end] + } + + fn find<'a>(manifest: &'a ParsedManifest, name: &str) -> &'a Item { + manifest + .items + .iter() + .find(|item| item.name == name) + .unwrap_or_else(|| panic!("no item {name}")) + } + + fn pom(body: &str) -> String { + format!( + "\n\ + {body}\n" + ) + } + + #[test] + fn parses_literal_versions_with_positions() { + let content = pom(" \n\ + \x20 \n\ + \x20 com.google.guava\n\ + \x20 guava\n\ + \x20 32.1.3-jre\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + assert_eq!(m.kind, ManifestKind::PomXml); + let guava = find(&m, "com.google.guava:guava"); + assert_eq!(guava.version_constraint, "32.1.3-jre"); + assert_eq!(sliced(&content, guava), "32.1.3-jre"); + assert_eq!(guava.source, PackageSource::Registry); + assert!(guava.is_rewritable()); + } + + /// A property used once carries the `` line that governs it, so + /// `--fix` rewrites where the version is actually written. + #[test] + fn a_property_used_once_carries_the_line_that_states_it() { + let content = pom(" \n\ + \x20 4.12.0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 com.squareup.okhttp3\n\ + \x20 okhttp\n\ + \x20 ${okhttp.version}\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let okhttp = find(&m, "com.squareup.okhttp3:okhttp"); + assert_eq!(okhttp.version_constraint, "4.12.0"); + assert_eq!(sliced(&content, okhttp), "4.12.0"); + assert!(okhttp.is_rewritable()); + } + + /// One line cannot be rewritten to two different versions, so a shared property + /// is resolved and checked but never written to. + #[test] + fn a_shared_property_is_resolved_but_never_rewritten() { + let content = pom(" \n\ + \x20 2.17.0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 com.fasterxml.jackson.core\n\ + \x20 jackson-core\n\ + \x20 ${jackson.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 com.fasterxml.jackson.core\n\ + \x20 jackson-databind\n\ + \x20 ${jackson.version}\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + for artifact in ["jackson-core", "jackson-databind"] { + let item = find(&m, &format!("com.fasterxml.jackson.core:{artifact}")); + assert_eq!(item.version_constraint, "2.17.0", "{artifact}"); + assert_eq!(item.source, PackageSource::Inherited, "{artifact}"); + assert!(item.is_checkable(), "{artifact}"); + assert!(!item.is_rewritable(), "{artifact}"); + } + } + + /// The required behaviour: a version this file cannot resolve is reported with + /// no constraint rather than dropped, so the dependency list is never quietly + /// short. + #[test] + fn an_unresolvable_version_is_reported_without_a_constraint() { + let content = pom(" \n\ + \x20 org.springframework.boot\n\ + \x20 spring-boot-starter-parent\n\ + \x20 3.2.5\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 org.springframework.boot\n\ + \x20 spring-boot-starter-web\n\ + \x20 \n\ + \x20 \n\ + \x20 org.example\n\ + \x20 sibling\n\ + \x20 ${project.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 org.example\n\ + \x20 composed\n\ + \x20 1.${minor}\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + // The `` is not a dependency, and its version is not read. + let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!( + names, + vec![ + "org.springframework.boot:spring-boot-starter-web", + "org.example:sibling", + "org.example:composed", + ] + ); + for item in &m.items { + assert!(item.version_constraint.is_empty(), "{item:?}"); + assert_eq!(item.source, PackageSource::Inherited, "{item:?}"); + assert!(!item.is_checkable(), "{item:?}"); + assert!(!item.has_position(), "{item:?}"); + } + } + + /// Versions stated for dependencies declared elsewhere, for the build, or under + /// a condition are not this project's dependencies. + #[test] + fn only_the_projects_own_dependencies_are_read() { + let content = pom(" \n\ + \x20 \n\ + \x20 \n\ + \x20 managed\n\ + \x20 managed\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 plugin\n\ + \x20 plugin\n\ + \x20 2.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 real\n\ + \x20 real\n\ + \x20 3.0.0\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!(names, vec!["real:real"]); + } + + /// `` and `` are stated in the manifest, so they are read + /// rather than guessed at from a file name. + #[test] + fn scope_and_optional_name_the_section() { + let content = pom(" \n\ + \x20 \n\ + \x20 org.junit.jupiter\n\ + \x20 junit-jupiter\n\ + \x20 5.10.2\n\ + \x20 test\n\ + \x20 \n\ + \x20 \n\ + \x20 org.example\n\ + \x20 extra\n\ + \x20 1.0.0\n\ + \x20 true\n\ + \x20 \n\ + \x20 \n\ + \x20 org.example\n\ + \x20 vendored\n\ + \x20 1.0.0\n\ + \x20 system\n\ + \x20 /opt/vendored.jar\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + assert_eq!( + find(&m, "org.junit.jupiter:junit-jupiter").kind, + DependencyKind::Dev + ); + assert_eq!(find(&m, "org.example:extra").kind, DependencyKind::Optional); + + let vendored = find(&m, "org.example:vendored"); + assert_eq!(vendored.source, PackageSource::Local); + assert!(!vendored.is_checkable(), "a system jar has no registry"); + } + + /// A property may name another; the span that governs is the one that finally + /// states a literal. + #[test] + fn a_property_chain_resolves_to_the_line_that_states_a_literal() { + let content = pom(" \n\ + \x20 real.version\n\ + \x20 9.9.9\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 ${alias}\n\ + \x20 \n\ + \x20 \n"); + // `` states a literal of its own, so it is the terminal property. + let m = parse(&content); + assert_eq!(find(&m, "g:a").version_constraint, "real.version"); + + let chained = content.replace( + "real.version", + "${real.version}", + ); + let m = parse(&chained); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, "9.9.9"); + assert_eq!(sliced(&chained, item), "9.9.9"); + } + + /// A cycle must terminate rather than spin, and states no version. + #[test] + fn a_property_cycle_resolves_to_nothing() { + let content = pom(" \n\ + \x20 ${b}\n\ + \x20 ${a}\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 ${a}\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + assert_eq!(find(&m, "g:a").version_constraint, ""); + } + + /// A group stated as a property still names a package; one that is not + /// resolvable names nothing that could be looked up. + #[test] + fn a_coordinate_may_be_stated_by_property() { + let content = pom(" \n\ + \x20 org.example\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 ${group}\n\ + \x20 named\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 ${project.groupId}\n\ + \x20 unnameable\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!(names, vec!["org.example:named"]); + } + + /// Whitespace around a version is formatting, not part of it, and the span has + /// to exclude it or `--fix` would rewrite the indentation too. + #[test] + fn a_span_excludes_the_whitespace_around_the_text() { + let content = pom(" \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 \n\ + \x20 1.2.3\n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, "1.2.3"); + assert_eq!(sliced(&content, item), "1.2.3"); + } + + #[test] + fn malformed_xml_is_a_structural_error() { + assert!(PomXmlParser.parse("").is_err()); + } + + #[test] + fn a_pom_without_dependencies_yields_none() { + let m = parse(&pom(" solo\n")); + assert!(m.items.is_empty()); + } +} diff --git a/crates/dependable-core/src/parsers/project.rs b/crates/dependable-core/src/parsers/project.rs index 9b9a35a..14d6ef0 100644 --- a/crates/dependable-core/src/parsers/project.rs +++ b/crates/dependable-core/src/parsers/project.rs @@ -74,6 +74,7 @@ pub fn parse_project(kind: ManifestKind, content: &str) -> ProjectMeta { ManifestKind::GoMod => go_module(content), ManifestKind::PubspecYaml => pubspec(content), ManifestKind::MixExs => mix(content), + ManifestKind::PomXml => pom(content), // A `pnpm-workspace.yaml` exists to hold catalogs; `Directory.Packages.props` // exists to hold central versions. A Gradle version catalog is the same shape // again — the project it serves is described by a build script. None names a @@ -215,6 +216,41 @@ fn mix(content: &str) -> ProjectMeta { } } +/// `pom.xml`: the ``, ``, and `` directly under +/// ``. +/// +/// The coordinate is the identity, and it is spelled `groupId:artifactId` — the same +/// way the POM parser names the dependencies it reads, so a project and a dependency +/// on it are the same string. A POM that states no `` of its own inherits it +/// from its ``, which is out of reach here (see +/// [`pom_xml`](super::pom_xml)); the bare `artifactId` is reported rather than a +/// coordinate that would be half guessed. A `` that is a property +/// (`${revision}`) is likewise not a literal this file states. +fn pom(content: &str) -> ProjectMeta { + let Ok(doc) = roxmltree::Document::parse(content) else { + return unnamed(); + }; + let project = doc.root_element(); + let field = |tag: &str| { + project + .children() + .find(|child| child.is_element() && child.tag_name().name() == tag) + .and_then(|child| child.text()) + .map(str::trim) + .filter(|text| !text.is_empty() && !text.contains('$')) + .map(str::to_owned) + }; + let name = field("artifactId").map(|artifact| match field("groupId") { + Some(group) => format!("{group}:{artifact}"), + None => artifact, + }); + ProjectMeta { + role: role_for(name.as_deref()), + name, + version: field("version").map(PackageField::Literal), + } +} + /// A manifest that declares dependency versions for others but no package of its own. fn workspace_meta() -> ProjectMeta { ProjectMeta { @@ -392,6 +428,44 @@ mod tests { ); } + /// A POM names itself by coordinate — the same string a dependency on it would + /// use. Anything it leaves to its `` is left unstated rather than guessed. + #[test] + fn a_pom_is_named_by_its_coordinate() { + let meta = parse_project( + ManifestKind::PomXml, + "\n org.example\n \ + demo\n 1.4.0\n\n", + ); + assert_eq!(meta.name.as_deref(), Some("org.example:demo")); + assert_eq!(meta.literal_version(), Some("1.4.0")); + assert_eq!(meta.role, ProjectRole::Package); + + // Group and version both inherited from a parent, and a `` of its own + // whose fields must not be mistaken for the project's. + let inheriting = parse_project( + ManifestKind::PomXml, + "\n \n org.parent\n \ + parent\n 9.9.9\n \ + \n child\n\n", + ); + assert_eq!(inheriting.name.as_deref(), Some("child")); + assert_eq!(inheriting.literal_version(), None); + + // `${revision}` is CI-friendly versioning, not a version. + let templated = parse_project( + ManifestKind::PomXml, + "\n demo\n \ + ${revision}\n\n", + ); + assert_eq!(templated.literal_version(), None); + + assert_eq!( + parse_project(ManifestKind::PomXml, " Date: Mon, 31 Aug 2026 22:38:02 -0400 Subject: [PATCH 02/25] feat(jvm): report a POM whose versions cannot be resolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notice surface a Gradle build script uses is a directory scan keyed on a file name, and it does not fit here: a `pom.xml` is perfectly readable, and it is individual entries within one that are not. Rather than add a second mechanism, an unresolvable dependency is reported through the vocabulary that already describes exactly this state — `PackageSource::Inherited` with no constraint, the shape a Cargo member's unresolved `dep.workspace = true` already has. It is never fetched, positioned, or fixed, and `list` already renders it as `(unresolved)`. `sample-maven/pom.xml` covers all of it in one file: a literal version, a property used once, a property two dependencies share, a version supplied by the ``, and a `${revision}` that is Maven's rather than the file's, beside a `` entry and a plugin dependency that must not leak into the artifact's list. The CLI test asserts the unresolvable entry is present with a null constraint, which is what "reported rather than skipped" has to mean at the surface a user sees. --- README.md | 22 ++- crates/dependable/tests/fixture_maven.rs | 178 ++++++++++++++++++ .../tests/fixtures/sample-maven/pom.xml | 101 ++++++++++ 3 files changed, 294 insertions(+), 7 deletions(-) create mode 100644 crates/dependable/tests/fixture_maven.rs create mode 100644 crates/dependable/tests/fixtures/sample-maven/pom.xml diff --git a/README.md b/README.md index 8c03cb2..bf899ed 100644 --- a/README.md +++ b/README.md @@ -38,13 +38,21 @@ Or download a prebuilt binary for your platform from the | Dart / Flutter | `pubspec.yaml` | pub.dev | `pubspec.lock` | 🧪 Experimental | | C# / .NET | `*.csproj`, `Directory.Packages.props` | NuGet | — | 🧪 Experimental | | Elixir | `mix.exs` | Hex | `mix.lock` | 🧪 Experimental | -| Kotlin / Java | `gradle/libs.versions.toml` | Maven Central | — | 🧪 Experimental | - -Kotlin / Java coverage is the **declarative** half of a Gradle build: the version -catalog. A build script (`build.gradle`, `build.gradle.kts`) is a program, and reading -one means running your build — so a build script found without a catalog beside it is -reported as unread rather than silently skipped, and a handful of catalog entries never -gets presented as a whole dependency list. +| Kotlin / Java | `gradle/libs.versions.toml`, `pom.xml` | Maven Central | — | 🧪 Experimental | + +Kotlin / Java coverage is the **declarative** half of a JVM build. For Gradle that is +the version catalog: a build script (`build.gradle`, `build.gradle.kts`) is a program, +and reading one means running your build — so a build script found without a catalog +beside it is reported as unread rather than silently skipped, and a handful of catalog +entries never gets presented as a whole dependency list. + +A Maven `pom.xml` is data throughout, so its `` are read directly, with +`${property}` resolved against the `` of the same file. What a POM defers +to its ``, to ``, or to an imported BOM is **not** +resolved — doing so correctly can mean fetching the parent POM from a registry, which +is a resolution engine rather than a parser. Those dependencies are still listed, with +no version and nothing claimed about them, so a POM that inherits some of its versions +is never presented as depending on only the rest. ### Lockfiles diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs new file mode 100644 index 0000000..ac10545 --- /dev/null +++ b/crates/dependable/tests/fixture_maven.rs @@ -0,0 +1,178 @@ +//! Offline parse of the Maven fixture: a POM's coordinates, `${property}` +//! resolution, version-span round-tripping, and what a version this file cannot +//! resolve is reported as. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use dependable_fetch::core::{DependencyKind, Item, PackageSource, parse, parse_project}; +use dependable_fetch::{Ecosystem, ManifestKind}; +use serde_json::Value; + +fn fixture(rel: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures") + .join(rel) +} + +fn slice<'a>(content: &'a str, item: &Item) -> &'a str { + let line = content.lines().nth(item.version_line).unwrap(); + &line[item.version_col_start..item.version_col_end] +} + +fn find<'a>(items: &'a [Item], name: &str) -> &'a Item { + items + .iter() + .find(|item| item.name == name) + .unwrap_or_else(|| panic!("no item {name}")) +} + +#[test] +fn parses_a_maven_pom() { + let path = fixture("sample-maven/pom.xml"); + let kind = ManifestKind::detect(&path).expect("recognised by name"); + assert_eq!(kind, ManifestKind::PomXml); + assert_eq!(kind.ecosystem(), Ecosystem::Jvm); + assert_eq!(kind.ecosystem().osv_name(), "Maven"); + + let manifest = std::fs::read_to_string(&path).unwrap(); + let parsed = parse(kind, &manifest).unwrap(); + + // Only `` under ``: the ``, the + // `` entry, and the plugin's own dependency are not + // dependencies of this artifact. + let names: Vec<&str> = parsed.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!( + names, + vec![ + "com.google.guava:guava", + "com.squareup.okhttp3:okhttp", + "com.fasterxml.jackson.core:jackson-core", + "com.fasterxml.jackson.core:jackson-databind", + "org.springframework.boot:spring-boot-starter-web", + "com.example:sample-shared", + "org.junit.jupiter:junit-jupiter", + ] + ); + + // A version stated on the dependency is rewritable where it is written. + let guava = find(&parsed.items, "com.google.guava:guava"); + assert_eq!(guava.version_constraint, "32.1.3-jre"); + assert_eq!(slice(&manifest, guava), "32.1.3-jre"); + assert_eq!(guava.source, PackageSource::Registry); + assert!(guava.is_rewritable()); + + // A property used once points at the `` line that governs it, so + // `--fix` rewrites the version where Maven actually reads it. + let okhttp = find(&parsed.items, "com.squareup.okhttp3:okhttp"); + assert_eq!(okhttp.version_constraint, "4.12.0"); + assert_eq!(slice(&manifest, okhttp), "4.12.0"); + assert!(okhttp.is_rewritable()); + assert!( + manifest + .lines() + .nth(okhttp.version_line) + .unwrap() + .contains("okhttp.version"), + "the span belongs to the entry, not to the " + ); + + // A property two dependencies share is resolved but never rewritten: one line + // cannot be rewritten to two different versions. + for artifact in ["jackson-core", "jackson-databind"] { + let item = find( + &parsed.items, + &format!("com.fasterxml.jackson.core:{artifact}"), + ); + assert_eq!(item.version_constraint, "2.17.0", "{artifact}"); + assert_eq!(item.source, PackageSource::Inherited, "{artifact}"); + assert!(item.is_checkable(), "{artifact}"); + assert!(!item.is_rewritable(), "{artifact}"); + } + + // `` is stated in the manifest, so the section is read rather than guessed. + let junit = find(&parsed.items, "org.junit.jupiter:junit-jupiter"); + assert_eq!(junit.kind, DependencyKind::Dev); + assert_eq!(junit.version_constraint, "5.10.2"); +} + +/// The required behaviour. A version supplied by a `` and one written as a +/// Maven built-in are both out of a parser's reach, and both are **reported** with no +/// constraint rather than dropped. Dropping them, the way the `csproj` parser drops an +/// MSBuild `$(…)` version, would present a POM that inherits some of its versions as +/// depending on only the rest — a short list that looks complete. +#[test] +fn a_version_this_file_cannot_resolve_is_reported_rather_than_dropped() { + let path = fixture("sample-maven/pom.xml"); + let manifest = std::fs::read_to_string(&path).unwrap(); + let parsed = parse(ManifestKind::PomXml, &manifest).unwrap(); + + for name in [ + "org.springframework.boot:spring-boot-starter-web", + "com.example:sample-shared", + ] { + let item = find(&parsed.items, name); + assert!(item.version_constraint.is_empty(), "{name}"); + assert_eq!(item.source, PackageSource::Inherited, "{name}"); + // Nothing is claimed about it: it is not fetched, not positioned, not fixed. + assert!(!item.is_checkable(), "{name}"); + assert!(!item.has_position(), "{name}"); + assert!(!item.is_rewritable(), "{name}"); + } +} + +/// A POM names itself by coordinate, and the ``'s coordinate is not it. +#[test] +fn a_pom_reports_its_own_coordinate() { + let manifest = std::fs::read_to_string(fixture("sample-maven/pom.xml")).unwrap(); + let meta = parse_project(ManifestKind::PomXml, &manifest); + assert_eq!(meta.name.as_deref(), Some("com.example:sample-maven")); + assert_eq!(meta.literal_version(), Some("1.4.0")); +} + +/// The other half of the requirement, at the surface a user sees: `list` is offline, +/// so it reports exactly what the parser produced. An unresolvable dependency is +/// present with a null constraint and an `inherited` source — visible, and not +/// mistaken for one that was checked. +#[test] +fn the_cli_lists_an_unresolvable_dependency_instead_of_omitting_it() { + let output = Command::new(env!("CARGO_BIN_EXE_dependable")) + .args([ + "list", + fixture("sample-maven").to_str().unwrap(), + "--format", + "json", + ]) + .output() + .expect("run dependable"); + assert!( + output.status.success(), + "list failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let doc: Value = serde_json::from_slice(&output.stdout).expect("valid JSON"); + + let project = doc["projects"] + .as_array() + .expect("projects array") + .iter() + .find(|p| p["name"] == "com.example:sample-maven") + .unwrap_or_else(|| panic!("no Maven project in {}", doc["projects"])); + assert_eq!(project["ecosystem"], "JVM"); + + let dependencies = project["dependencies"].as_array().expect("dependencies"); + let dependency = |name: &str| { + dependencies + .iter() + .find(|d| d["name"] == name) + .unwrap_or_else(|| panic!("no dependency {name}")) + }; + + let unresolved = dependency("org.springframework.boot:spring-boot-starter-web"); + assert!(unresolved["constraint"].is_null(), "{unresolved}"); + assert_eq!(unresolved["source"], "inherited"); + + let guava = dependency("com.google.guava:guava"); + assert_eq!(guava["constraint"], "32.1.3-jre"); + assert_eq!(guava["source"], "registry"); +} diff --git a/crates/dependable/tests/fixtures/sample-maven/pom.xml b/crates/dependable/tests/fixtures/sample-maven/pom.xml new file mode 100644 index 0000000..c2b5a3d --- /dev/null +++ b/crates/dependable/tests/fixtures/sample-maven/pom.xml @@ -0,0 +1,101 @@ + + + 4.0.0 + + + + org.springframework.boot + spring-boot-starter-parent + 3.2.5 + + + com.example + sample-maven + 1.4.0 + + + 4.12.0 + 2.17.0 + + + + + + + org.apache.commons + commons-text + 1.11.0 + + + + + + + + com.google.guava + guava + 32.1.3-jre + + + + + com.squareup.okhttp3 + okhttp + ${okhttp.version} + + + + + com.fasterxml.jackson.core + jackson-core + ${jackson.version} + + + com.fasterxml.jackson.core + jackson-databind + ${jackson.version} + + + + + org.springframework.boot + spring-boot-starter-web + + + + + com.example + sample-shared + ${revision} + + + + org.junit.jupiter + junit-jupiter + 5.10.2 + test + + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + org.junit.platform + junit-platform-launcher + 1.10.2 + + + + + + From d0bccfafda5337eace00f88e97e3af6d1721132c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 22:38:36 -0400 Subject: [PATCH 03/25] docs(core): say what a value is kept as --- crates/dependable-core/src/parsers/pom_xml.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 74479e8..4b96b41 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -139,8 +139,10 @@ impl Parser for PomXmlParser { } } -/// Read `` into property name → literal, skipping any entry that only -/// points at another unknown. +/// Read `` into property name → stated value. +/// +/// The value is kept exactly as written, `${…}` and all: whether it is a literal or +/// another reference is [`terminal`]'s question, not this one's. fn read_properties<'a>(project: roxmltree::Node<'a, 'a>) -> HashMap { let mut out = HashMap::new(); let Some(table) = child(project, "properties") else { From 3a29eeab8a2faca018ca9dbd8a563ac1955e2ce7 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 22:39:21 -0400 Subject: [PATCH 04/25] test(core): an exclusion's coordinate is not the dependency's own --- crates/dependable-core/src/parsers/pom_xml.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 4b96b41..a577777 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -661,6 +661,30 @@ mod tests { assert_eq!(sliced(&content, item), "1.2.3"); } + /// An `` block names packages that must **not** be pulled in. Its + /// coordinates are nested, and reading only direct children is what keeps them + /// from being mistaken for the dependency's own. + #[test] + fn an_exclusion_is_not_a_dependency() { + let content = pom(" \n\ + \x20 \n\ + \x20 org.example\n\ + \x20 app\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 commons-logging\n\ + \x20 commons-logging\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!(names, vec!["org.example:app"]); + assert_eq!(sliced(&content, &m.items[0]), "1.0.0"); + } + #[test] fn malformed_xml_is_a_structural_error() { assert!(PomXmlParser.parse("").is_err()); From 725ef4165c289b39b7a85943707c0c214d203341 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 23:59:09 -0400 Subject: [PATCH 05/25] feat(core): report a dependency whose currency could not be determined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DependencyStatus` had no word for "nothing was learned about this". A dependency that names no version this tool can resolve fell into `Local`, which states something quite different and quite false: that there is no registry behind the package at all. That is what `local` means for a Cargo `path = "../x"` and for a Maven `system` jar, and applying it to `spring-boot-starter-web` — an artifact on Maven Central whose version this run simply did not read — is a claim, not a shrug. `Undetermined` is that word. It is deliberately named for the fact rather than for the ecosystem that surfaced it, because two situations produce the same fact: a manifest that defers its version somewhere this parser does not follow, and an ecosystem that publishes no registry to compare a version against at all. The second has no producer yet. `PackageSource::Inherited` and the two `Item` predicates that gate on it still described a Cargo `workspace = true` and nothing else, though three parsers now emit the variant. They say what the variant means instead. --- crates/dependable-core/src/item.rs | 47 ++++++++++++++++------ crates/dependable-core/src/result.rs | 59 ++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/crates/dependable-core/src/item.rs b/crates/dependable-core/src/item.rs index 89dafc6..5bcf9ef 100644 --- a/crates/dependable-core/src/item.rs +++ b/crates/dependable-core/src/item.rs @@ -40,9 +40,13 @@ impl Item { /// sources are skipped. /// /// An [`Inherited`](PackageSource::Inherited) item is checkable only once - /// [`resolve_workspace_inheritance`](crate::resolve_workspace_inheritance) has - /// supplied the workspace root's constraint. Unresolved, the manifest states no - /// version at all, and there is nothing to ask a registry for. + /// something has supplied the constraint declared elsewhere — the workspace root + /// via [`resolve_workspace_inheritance`](crate::resolve_workspace_inheritance) for + /// Cargo, the `[versions]` table for a Gradle catalog, the `` table + /// for a POM. Without one the manifest states no version at all, and there is + /// nothing to ask a registry for; a check reports such an item as + /// [`Undetermined`](crate::result::DependencyStatus::Undetermined) rather than + /// claiming it has no registry. #[must_use] pub fn is_checkable(&self) -> bool { match self.source { @@ -60,7 +64,9 @@ impl Item { /// parser that declines to record a span also gives the item a source nothing would /// fetch, so [`is_checkable`](Self::is_checkable) covers all of them but one: a /// resolved [`Inherited`](PackageSource::Inherited) item is worth checking and still - /// has no home here, because its version string is in the workspace root. + /// has no home here, because the version string it was resolved from belongs to + /// another entry — a workspace root's table, a catalog `[versions]` alias, a shared + /// POM `` value. #[must_use] pub fn has_position(&self) -> bool { self.is_checkable() && self.source != PackageSource::Inherited @@ -153,14 +159,33 @@ pub enum PackageSource { Local, /// A git dependency — skipped for version checks. Git, - /// A Cargo `dep.workspace = true`: the manifest opts into a version declared in - /// the workspace root's `[workspace.dependencies]` and states none of its own. + /// The dependency's version is declared somewhere other than this entry, so + /// there is no version string here to check against or to rewrite. /// - /// Reading `workspace = true` needs no filesystem, so the IO-free parser records it - /// — which is what keeps it distinct from a [`Local`](Self::Local) `path` entry that - /// happens to share a name with a root declaration. Resolving it against the root - /// does need IO, and is [`resolve_workspace_inheritance`](crate::resolve_workspace_inheritance) - /// applied by the caller that has the root in hand. + /// Three parsers emit it, for the same reason and with the same consequences: + /// + /// - Cargo's `dep.workspace = true` — the version is in the workspace root's + /// `[workspace.dependencies]`. Reading `workspace = true` needs no + /// filesystem, so the IO-free parser records the fact; *resolving* it does + /// need IO, and is + /// [`resolve_workspace_inheritance`](crate::resolve_workspace_inheritance) + /// applied by the caller that has the root in hand. + /// - A Gradle version catalog entry whose `version.ref` names a `[versions]` + /// alias several entries share. + /// - A Maven POM entry whose version comes from a `` value several + /// dependencies share, or from a `` / `` / + /// undeclared property this file does not state. + /// + /// What keeps it distinct from [`Local`](Self::Local) is that the package is a + /// real registry package — an entry that merely shares a name with a root + /// `path` declaration is `Local`, and a POM `system` jar is + /// `Local`, because neither has a registry at all. + /// + /// The constraint tells the two halves apart. Filled in, the version was found + /// elsewhere and the item is checkable — never rewritable, since the string it + /// would rewrite is not this dependency's own. Empty, no version was found at + /// all, and a check reports + /// [`DependencyStatus::Undetermined`](crate::result::DependencyStatus::Undetermined). Inherited, } diff --git a/crates/dependable-core/src/result.rs b/crates/dependable-core/src/result.rs index 732032a..b607e11 100644 --- a/crates/dependable-core/src/result.rs +++ b/crates/dependable-core/src/result.rs @@ -114,14 +114,48 @@ impl CheckResult { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum DependencyStatus { + /// The best available version is already the one in use. UpToDate, + /// A newer patch release exists within the declared constraint. PatchAvailable, + /// A newer release exists within the declared constraint. UpdateAvailable, + /// A newer release exists outside the declared constraint. Outdated, + /// A known advisory affects the version in use. Vulnerable, + /// The registry was asked and the request failed; the text is what it said. Error(String), + /// There is no registry behind this dependency: a `path` entry, a Maven + /// `system` jar. Nothing was looked up because there is + /// nowhere to look. Local, + /// A git dependency, tracked by revision rather than by version. Git, + /// Whether this dependency is current **could not be determined**, and no + /// claim is made either way. + /// + /// Distinct from all three of its neighbours, and the distinction is the + /// point: + /// + /// - [`Local`](Self::Local) says *there is no registry for this*. Applied to + /// a package that is on one, it is a false statement. + /// - [`Error`](Self::Error) says *the registry was asked and it failed*. + /// Nothing was asked here. + /// - [`UpToDate`](Self::UpToDate) says *this is current*, which is precisely + /// what was not established. + /// + /// Two situations produce it. The manifest names a real package but states no + /// version this tool can resolve — a Maven POM deferring to ``, + /// ``, or a property it does not declare, or a Cargo + /// member inheriting a name its workspace root never declares. Or the + /// ecosystem publishes no registry to compare a version against at all, so + /// currency is not merely unread but unknowable. + /// + /// A run is expected to say *why* alongside it: the check that produces one + /// emits a manifest-level warning naming the dependencies involved, because a + /// status word on its own does not tell a reader what to fix. + Undetermined, } impl DependencyStatus { @@ -137,6 +171,7 @@ impl DependencyStatus { DependencyStatus::Error(_) => "error", DependencyStatus::Local => "local", DependencyStatus::Git => "git", + DependencyStatus::Undetermined => "undetermined", } } @@ -152,6 +187,7 @@ impl DependencyStatus { DependencyStatus::Error(_) => "ERROR", DependencyStatus::Local => "LOCAL", DependencyStatus::Git => "GIT", + DependencyStatus::Undetermined => "UNDETERMINED", } } } @@ -798,6 +834,29 @@ mod tests { assert_eq!(Advisory::unrated_count(&[]), 0); } + /// The tokens are what a CI consumer matches on, so they are pinned here. + /// `UNDETERMINED` in particular must never collapse into `LOCAL`: one says + /// there is no registry for this package, the other says nothing was read + /// about a package that has one. + #[test] + fn status_labels_and_tokens_are_stable_and_distinct() { + let cases = [ + (DependencyStatus::UpToDate, "up to date", "OK"), + (DependencyStatus::Local, "local", "LOCAL"), + (DependencyStatus::Git, "git", "GIT"), + ( + DependencyStatus::Undetermined, + "undetermined", + "UNDETERMINED", + ), + ]; + for (status, label, token) in &cases { + assert_eq!(status.label(), *label); + assert_eq!(status.token(), *token); + } + assert_ne!(DependencyStatus::Undetermined, DependencyStatus::Local); + } + #[test] fn a_fresh_result_carries_no_advisories() { let bare = CheckResult::new(item("serde"), DependencyStatus::Local); From 949b4f88ff26eb9bdca8b8eb8c0723a70f622afd Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 23:59:28 -0400 Subject: [PATCH 06/25] fix(core): follow the number of property hops the constant documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MAX_PROPERTY_HOPS = 8` is a count of hops, but the loop spent one iteration on arriving at the first property, which is not a hop. An eight-hop chain that terminates perfectly well therefore returned nothing, and the dependency reading it was reported with no version. It fails safe, so nothing was ever mis-stated — but the constant and the code disagreed about what the number meant, and the doc comment now says which one it is: the longest chain that resolves is `MAX_PROPERTY_HOPS + 1` properties. --- crates/dependable-core/src/parsers/pom_xml.rs | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index a577777..36b3f60 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -62,8 +62,13 @@ use crate::error::ParseError; use crate::item::{DependencyKind, Item, PackageSource}; use crate::manifest::{ManifestKind, ParsedManifest}; -/// How far a `${a}` → `${b}` → literal chain is followed before giving up, which -/// also bounds a property that (illegally) refers to itself. +/// How many `${a}` → `${b}` **hops** a property chain may take before it is given +/// up on, which also bounds a property that (illegally) refers to itself. +/// +/// A hop is a step from one property to the next, so the longest chain that still +/// resolves is `MAX_PROPERTY_HOPS + 1` properties long: eight hops from `${p0}` +/// reach `p8`, and `p8` is read. [`terminal`] loops one more time than this number +/// for exactly that reason — the first read is not a hop. const MAX_PROPERTY_HOPS: usize = 8; /// Parses `pom.xml`. @@ -248,7 +253,9 @@ fn version_source(node: roxmltree::Node<'_, '_>, properties: &HashMap(start: &str, properties: &'a HashMap) -> Option<&'a str> { let mut name = start; - for _ in 0..MAX_PROPERTY_HOPS { + // Inclusive: `MAX_PROPERTY_HOPS` hops means one more property read than hops + // taken, since arriving at the first property costs no hop. + for _ in 0..=MAX_PROPERTY_HOPS { let (key, located) = properties.get_key_value(name)?; match interpolation(&located.value) { Some(next) => name = next, @@ -690,6 +697,34 @@ mod tests { assert!(PomXmlParser.parse("").is_err()); } + /// Eight hops is what the constant says, so eight hops has to resolve. + #[test] + fn a_chain_resolves_up_to_the_documented_number_of_hops() { + let chain = |hops: usize| { + let mut properties = String::from(" \n"); + for hop in 0..hops { + properties.push_str(&format!(" ${{p{}}}\n", hop + 1)); + } + properties.push_str(&format!(" 9.9.9\n \n")); + let content = pom(&format!( + "{properties} \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 ${{p0}}\n\ + \x20 \n\ + \x20 \n" + )); + parse(&content).items[0].version_constraint.clone() + }; + assert_eq!(chain(MAX_PROPERTY_HOPS), "9.9.9", "the documented limit"); + assert_eq!( + chain(MAX_PROPERTY_HOPS + 1), + "", + "one hop past it states nothing, rather than spinning" + ); + } + #[test] fn a_pom_without_dependencies_yields_none() { let m = parse(&pom(" solo\n")); From b0b08256eda0ace43e5276beaf03f6962213f5e8 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Mon, 31 Aug 2026 23:59:46 -0400 Subject: [PATCH 07/25] fix(core): read a pom version split by a comment whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `text_of` took the first text node's value and dropped only the *span* when there were several, so `1.0.0` yielded the constraint `1.0` — a version the file never declares — and reported it as checkable. dependable then asked Maven Central about the artifact and evaluated it against a constraint of its own invention. `--fix` could not corrupt the file, because the span was already dropped, so this was a wrong claim rather than a wrong write. It is still a wrong claim. Every text node is now concatenated, which is the value Maven itself reads. The sibling spellings were already correct and stay so: `1.0.0` and a `CDATA` section both yield `1.0.0` with the span dropped, since the source bytes are not the value's bytes and an offset into one is not an offset into the other. A test pins all three together. --- crates/dependable-core/src/parsers/pom_xml.rs | 77 ++++++++++++++++--- 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 36b3f60..d3207c1 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -298,25 +298,35 @@ fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option1.0.0\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let item = find(&m, "g:a"); + assert_eq!( + item.version_constraint, "1.0.0", + "every text node is the value Maven reads" + ); + assert!( + !item.is_rewritable(), + "the source bytes are not the value's bytes, so there is nothing to rewrite" + ); + } + + /// The sibling cases: an escaped character and a `CDATA` section both state the + /// whole version, and neither offers bytes a rewrite could replace. + #[test] + fn an_escaped_or_wrapped_version_is_read_whole_and_never_rewritten() { + for spelling in ["1.0.0", ""] { + let content = pom(&format!( + " \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 {spelling}\n\ + \x20 \n\ + \x20 \n" + )); + let m = parse(&content); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, "1.0.0", "{spelling}"); + assert!(!item.is_rewritable(), "{spelling}"); + } + } + /// Eight hops is what the constant says, so eight hops has to resolve. #[test] fn a_chain_resolves_up_to_the_documented_number_of_hops() { From 2ed3a0f4164dfe65722b31249126ae84b6d0f1fe Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:00:02 -0400 Subject: [PATCH 08/25] =?UTF-8?q?fix(core):=20count=20every=20${=E2=80=A6}?= =?UTF-8?q?=20reference=20when=20deciding=20who=20owns=20a=20properties=20?= =?UTF-8?q?line?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sole ownership of a `` line was decided by counting only the references made from the one top-level `` list. A property that list reads once but a `` block, a `` entry, or a plugin's own `` also reads was still marked rewritable, and the rewrite reached all of them. Demonstrated with `32.1.3-jre` read by the top-level `guava` and by a profile-only `guava-gwt`: `fix --all` printed "Updated 1 dependency", rewrote the line, and silently moved `guava-gwt` — a different artifact, never fetched, never validated, never named in the fix record. Sole ownership is a fact about the line, not about ``, so the count now walks the whole document. A `` value that is *entirely* one `${…}` is a link in a chain rather than a reader of it and is skipped, or no chained property could ever be rewritten; a composed value is a reader and is counted. This is the `count_version_refs` rule from `gradle_catalog`, which repairs the identical defect for a `[versions]` alias a `[plugins]` entry shares. --- crates/dependable-core/src/parsers/pom_xml.rs | 204 +++++++++++++++++- 1 file changed, 196 insertions(+), 8 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index d3207c1..cea3652 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -111,14 +111,11 @@ impl Parser for PomXmlParser { let properties = read_properties(project); let declared = read_dependencies(project, &properties); - // A property used by exactly one dependency is that dependency's own line - // to fix; one shared by several belongs to none of them. - let mut uses: HashMap<&str, usize> = HashMap::new(); - for entry in &declared { - if let Source::Property(name) = &entry.version { - *uses.entry(name.as_str()).or_default() += 1; - } - } + // A property used by exactly one thing in this document is that + // dependency's own line to fix; one anything else also reads belongs to + // none of them — see `count_property_refs`. + let mut uses: HashMap = HashMap::new(); + count_property_refs(project, &properties, &mut uses); let items = declared .iter() @@ -289,6 +286,71 @@ fn interpolated( } } +/// Count every `${…}` reference in the document, by the property its chain ends at. +/// +/// The whole document, because sole ownership of a `` line is a fact +/// about that line and not about ``. A POM that states +/// `32.1.3-jre` and reads it from both the top-level +/// `guava` and a ``-only `guava-gwt` has one line and two readers; +/// counting only the top-level list makes `guava` the sole reader, so `--fix` +/// rewrites the `` line and silently moves `guava-gwt` with it — a +/// different artifact, never fetched, never validated, never named in the fix +/// record. `` and a ``'s `` are the same +/// story. This is the `count_version_refs` rule of +/// [`gradle_catalog`](super::gradle_catalog), applied to the same defect. +/// +/// A `` value that is *entirely* one `${…}` is a link in a chain +/// rather than a reader of it, and is skipped: the reference is counted against +/// the property the chain ends at when whoever started the chain is counted. +/// A composed value (`${core.version}-jre`) **is** a reader, and is counted. +fn count_property_refs( + project: roxmltree::Node<'_, '_>, + properties: &HashMap, + uses: &mut HashMap, +) { + let table = child(project, "properties"); + for node in project.descendants().filter(roxmltree::Node::is_text) { + let Some(text) = node.text() else { continue }; + // A top-level `` entry's own value: a pure `${…}` is a chain + // link, not a use of the property it names. + let chain_link = table + .is_some_and(|table| node.parent().and_then(|entry| entry.parent()) == Some(table)) + && interpolation(text.trim()).is_some(); + if chain_link { + continue; + } + for reference in references(text) { + if let Some(name) = terminal(reference, properties) { + *uses.entry(name.to_owned()).or_default() += 1; + } + } + } +} + +/// Every `${…}` reference in a string, in order, by the name each names. +/// +/// A version may compose one (`1.${minor}`) and a plugin configuration may hold +/// several, so this scans rather than matching the whole value the way +/// [`interpolation`] does. +fn references(value: &str) -> impl Iterator { + let mut rest = value; + std::iter::from_fn(move || { + loop { + let start = rest.find("${")?; + let after = &rest[start + 2..]; + let Some(end) = after.find('}') else { + rest = ""; + return None; + }; + let inner = &after[..end]; + rest = &after[end + 1..]; + if !inner.is_empty() && !inner.contains(['$', '{']) { + return Some(inner); + } + } + }) +} + /// The first direct child element named `tag`. fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option> { node.children() @@ -752,6 +814,132 @@ mod tests { } } + /// A property the top-level list reads once but a `` block reads too + /// has two readers, not one. Rewriting its line would move an artifact that was + /// never fetched, never validated, and never named in the fix record. + #[test] + fn a_property_a_profile_also_reads_is_not_one_dependencys_to_rewrite() { + let content = pom(" \n\ + \x20 32.1.3-jre\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 com.google.guava\n\ + \x20 guava\n\ + \x20 ${lib.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 gwt\n\ + \x20 \n\ + \x20 \n\ + \x20 com.google.guava\n\ + \x20 guava-gwt\n\ + \x20 ${lib.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let guava = find(&m, "com.google.guava:guava"); + assert_eq!(guava.version_constraint, "32.1.3-jre"); + assert!( + guava.is_checkable(), + "the version is known and worth checking" + ); + assert!( + !guava.is_rewritable(), + "the profile reads the same line, and would move with it" + ); + } + + /// The same rule for the other two readers a POM has, so no single reader is + /// privileged: `` and a build plugin's own version. + #[test] + fn a_property_read_elsewhere_in_the_document_is_never_rewritable() { + let elsewhere = [ + " \n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 managed\n\ + \x20 ${lib.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 \n", + " \n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 plug\n\ + \x20 ${lib.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 \n", + ]; + for other in elsewhere { + let content = pom(&format!( + " \n\ + \x20 1.2.3\n\ + \x20 \n\ + {other}\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 ${{lib.version}}\n\ + \x20 \n\ + \x20 \n" + )); + let m = parse(&content); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, "1.2.3", "{other}"); + assert!(!item.is_rewritable(), "{other}"); + } + } + + /// A composed `` value reads the property it names, so the line it + /// names is shared; a value that is *only* a reference is a link in a chain and + /// is not itself a reader, or no chained property could ever be rewritten. + #[test] + fn a_chain_link_is_not_a_reader_but_a_composed_value_is() { + let chained = pom(" \n\ + \x20 ${real.version}\n\ + \x20 9.9.9\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 ${alias}\n\ + \x20 \n\ + \x20 \n"); + assert!( + find(&parse(&chained), "g:a").is_rewritable(), + "one dependency, one chain, one line to rewrite" + ); + + // A second property composing the same line is a reader of it, so the line + // is no longer any one dependency's to rewrite. + let composed = chained + .replace( + "${alias}", + "${real.version}", + ) + .replace( + "${real.version}", + "${real.version}-jre", + ); + let m = parse(&composed); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, "9.9.9"); + assert!( + !item.is_rewritable(), + "`` composes the same line into a second value" + ); + } + /// Eight hops is what the constant says, so eight hops has to resolve. #[test] fn a_chain_resolves_up_to_the_documented_number_of_hops() { From cd03d51c2a9dc4ac50c21b386a317b7941768470 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:00:38 -0400 Subject: [PATCH 09/25] feat(core): say what a parser saw and declined to read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A POM that declares every one of its dependencies inside `` listed as `(0 dependencies)`. Excluding conditional dependencies is defensible — a profile applies on a JDK version or an activated property, so its dependencies are not this project's as written, and reading them would state as fact something that holds only under a condition this file does not evaluate. A list that reads as complete and is not is the failure the POM parser exists to prevent, and being silent is what made this one that. `ParsedManifest` gains `notices`: what the parser saw and deliberately did not interpret, in the parser's own words. Not an error and not a warning about any dependency in `items` — the words go on stderr, where `list` and `check` both already put manifest-level notes, so the same sentence reaches a reader whichever `--format` they chose and no machine-readable document changes shape. The POM parser is the only one with anything to declare so far. Every other parser leaves it empty. --- crates/dependable-core/src/manifest.rs | 12 +++ .../dependable-core/src/parsers/cargo_toml.rs | 1 + .../src/parsers/composer_json.rs | 1 + crates/dependable-core/src/parsers/csproj.rs | 1 + .../dependable-core/src/parsers/deno_json.rs | 1 + crates/dependable-core/src/parsers/go_mod.rs | 1 + .../src/parsers/gradle_catalog.rs | 1 + crates/dependable-core/src/parsers/mix_exs.rs | 1 + .../src/parsers/package_json.rs | 1 + .../src/parsers/pnpm_workspace.rs | 1 + crates/dependable-core/src/parsers/pom_xml.rs | 88 ++++++++++++++++++- .../src/parsers/pubspec_yaml.rs | 1 + .../src/parsers/pyproject_toml.rs | 1 + .../src/parsers/requirements_txt.rs | 1 + crates/dependable/src/runner.rs | 8 ++ 15 files changed, 117 insertions(+), 3 deletions(-) diff --git a/crates/dependable-core/src/manifest.rs b/crates/dependable-core/src/manifest.rs index 8e32003..6ef3d9f 100644 --- a/crates/dependable-core/src/manifest.rs +++ b/crates/dependable-core/src/manifest.rs @@ -16,6 +16,18 @@ pub struct ParsedManifest { pub items: Vec, /// Alternate registry declarations (Rust `[registries.*]`). pub alternate_registries: Vec, + /// What the parser saw and deliberately did not read, in the parser's own + /// words, ready to print. + /// + /// Not errors and not warnings *about* the dependencies in + /// [`items`](Self::items): each one names a construct this parser declines to + /// interpret, so that a list which reads as complete and is not says so. + /// A Maven `` block holding dependencies is the motivating case — + /// excluding conditional dependencies is defensible, printing + /// `(0 dependencies)` for a POM that declares twelve of them is not. + /// + /// Empty for every parser that has nothing to declare, which is most of them. + pub notices: Vec, } /// A declared alternate registry (Rust only). diff --git a/crates/dependable-core/src/parsers/cargo_toml.rs b/crates/dependable-core/src/parsers/cargo_toml.rs index 338c9e6..bbdd540 100644 --- a/crates/dependable-core/src/parsers/cargo_toml.rs +++ b/crates/dependable-core/src/parsers/cargo_toml.rs @@ -70,6 +70,7 @@ impl Parser for CargoTomlParser { kind: ManifestKind::CargoToml, items, alternate_registries, + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/composer_json.rs b/crates/dependable-core/src/parsers/composer_json.rs index a747435..288160b 100644 --- a/crates/dependable-core/src/parsers/composer_json.rs +++ b/crates/dependable-core/src/parsers/composer_json.rs @@ -47,6 +47,7 @@ impl Parser for ComposerJsonParser { kind: ManifestKind::ComposerJson, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/csproj.rs b/crates/dependable-core/src/parsers/csproj.rs index 711b669..c8ddfd6 100644 --- a/crates/dependable-core/src/parsers/csproj.rs +++ b/crates/dependable-core/src/parsers/csproj.rs @@ -67,6 +67,7 @@ impl Parser for CsprojParser { kind: ManifestKind::Csproj, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/deno_json.rs b/crates/dependable-core/src/parsers/deno_json.rs index 71ab1df..93898ef 100644 --- a/crates/dependable-core/src/parsers/deno_json.rs +++ b/crates/dependable-core/src/parsers/deno_json.rs @@ -29,6 +29,7 @@ impl Parser for DenoJsonParser { kind: ManifestKind::DenoJson, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/go_mod.rs b/crates/dependable-core/src/parsers/go_mod.rs index dddebb6..9932387 100644 --- a/crates/dependable-core/src/parsers/go_mod.rs +++ b/crates/dependable-core/src/parsers/go_mod.rs @@ -52,6 +52,7 @@ impl Parser for GoModParser { kind: ManifestKind::GoMod, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/gradle_catalog.rs b/crates/dependable-core/src/parsers/gradle_catalog.rs index c4d930c..3d600d6 100644 --- a/crates/dependable-core/src/parsers/gradle_catalog.rs +++ b/crates/dependable-core/src/parsers/gradle_catalog.rs @@ -121,6 +121,7 @@ impl Parser for GradleCatalogParser { kind: ManifestKind::GradleVersionCatalog, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/mix_exs.rs b/crates/dependable-core/src/parsers/mix_exs.rs index 45bb1f4..8e46124 100644 --- a/crates/dependable-core/src/parsers/mix_exs.rs +++ b/crates/dependable-core/src/parsers/mix_exs.rs @@ -56,6 +56,7 @@ impl Parser for MixExsParser { kind: ManifestKind::MixExs, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/package_json.rs b/crates/dependable-core/src/parsers/package_json.rs index b6dc778..51cb23b 100644 --- a/crates/dependable-core/src/parsers/package_json.rs +++ b/crates/dependable-core/src/parsers/package_json.rs @@ -36,6 +36,7 @@ impl Parser for PackageJsonParser { kind: ManifestKind::PackageJson, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/pnpm_workspace.rs b/crates/dependable-core/src/parsers/pnpm_workspace.rs index c233200..9d6ec63 100644 --- a/crates/dependable-core/src/parsers/pnpm_workspace.rs +++ b/crates/dependable-core/src/parsers/pnpm_workspace.rs @@ -64,6 +64,7 @@ impl Parser for PnpmWorkspaceParser { kind: ManifestKind::PnpmWorkspaceYaml, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index cea3652..07219ac 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -137,6 +137,7 @@ impl Parser for PomXmlParser { kind: ManifestKind::PomXml, items, alternate_registries: Vec::new(), + notices: profile_notice(project).into_iter().collect(), }) } } @@ -351,6 +352,41 @@ fn references(value: &str) -> impl Iterator { }) } +/// Say that a `` block was seen and its dependencies were not read. +/// +/// A profile applies conditionally — on a JDK version, an activated property, an +/// operating system — so its dependencies are not this project's as written, and +/// parsing them would state as fact something that holds only under a condition +/// this file does not evaluate. Staying out is the decision; staying *silent* +/// about it is not, because a POM that declares every one of its dependencies +/// inside a profile then lists as `(0 dependencies)`, which reads as complete and +/// is not. +fn profile_notice(project: roxmltree::Node<'_, '_>) -> Option { + let profiles = child(project, "profiles")?; + let count = profiles + .descendants() + .filter(|node| { + node.is_element() + && node.tag_name().name() == "dependency" + && node + .parent() + .is_some_and(|list| list.tag_name().name() == "dependencies") + }) + .count(); + if count == 0 { + return None; + } + Some(format!( + "{count} {} declared inside {} not listed: a profile applies conditionally, so its dependencies are not this project's as written", + if count == 1 { + "dependency" + } else { + "dependencies" + }, + if count == 1 { "is" } else { "are" }, + )) +} + /// The first direct child element named `tag`. fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option> { node.children() @@ -769,6 +805,13 @@ mod tests { assert!(PomXmlParser.parse("").is_err()); } + #[test] + fn a_pom_without_dependencies_yields_none() { + let m = parse(&pom(" solo\n")); + assert!(m.items.is_empty()); + assert!(m.notices.is_empty()); + } + /// A comment splits the version into two text nodes. Reading only the first /// states `1.0` — a version this file never declares, which would then be /// fetched and evaluated as if it were the real constraint. @@ -968,9 +1011,48 @@ mod tests { ); } + /// Not parsing conditional dependencies is the decision; not *saying so* would + /// leave a POM that declares all of them in a profile listing as empty. #[test] - fn a_pom_without_dependencies_yields_none() { - let m = parse(&pom(" solo\n")); - assert!(m.items.is_empty()); + fn a_profiles_block_holding_dependencies_is_announced() { + let content = pom(" \n\ + \x20 \n\ + \x20 native\n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 g\n\ + \x20 b\n\ + \x20 2.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + assert!(m.items.is_empty(), "a profile dependency is conditional"); + assert_eq!(m.notices.len(), 1, "{:?}", m.notices); + assert!( + m.notices[0].contains("2 dependencies declared inside "), + "{:?}", + m.notices + ); + } + + /// A profile that declares no dependency of its own has nothing to announce. + #[test] + fn a_profile_without_dependencies_says_nothing() { + let content = pom(" \n\ + \x20 \n\ + \x20 release\n\ + \x20 \n\ + \x20 true\n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + assert!(parse(&content).notices.is_empty()); } } diff --git a/crates/dependable-core/src/parsers/pubspec_yaml.rs b/crates/dependable-core/src/parsers/pubspec_yaml.rs index 6de88f2..fde581e 100644 --- a/crates/dependable-core/src/parsers/pubspec_yaml.rs +++ b/crates/dependable-core/src/parsers/pubspec_yaml.rs @@ -70,6 +70,7 @@ impl Parser for PubspecYamlParser { kind: ManifestKind::PubspecYaml, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/pyproject_toml.rs b/crates/dependable-core/src/parsers/pyproject_toml.rs index 27e2826..8de269b 100644 --- a/crates/dependable-core/src/parsers/pyproject_toml.rs +++ b/crates/dependable-core/src/parsers/pyproject_toml.rs @@ -89,6 +89,7 @@ impl Parser for PyprojectTomlParser { kind: ManifestKind::PyprojectToml, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable-core/src/parsers/requirements_txt.rs b/crates/dependable-core/src/parsers/requirements_txt.rs index b1c5cd5..31c0d76 100644 --- a/crates/dependable-core/src/parsers/requirements_txt.rs +++ b/crates/dependable-core/src/parsers/requirements_txt.rs @@ -26,6 +26,7 @@ impl Parser for RequirementsTxtParser { kind: ManifestKind::RequirementsTxt, items, alternate_registries: Vec::new(), + notices: Vec::new(), }) } } diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 5e25916..620671d 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -626,6 +626,14 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { // versions of one crate, and `pick_locked` chooses among them *by the declared // constraint*. Resolving second would hand it an empty constraint and pick the // highest — reporting `syn 2.0` locked against a member that inherits `syn = "1"`. + // What the parser saw and declined to read. On stderr rather than in the + // listing, so the same words reach a reader whichever `--format` they + // chose, and no machine-readable document changes shape — the same place + // `check` puts a manifest-level warning. + for notice in &parsed.notices { + eprintln!("warning: {} — {notice}", manifest.display()); + } + let inherited = workspace_source(manifest, kind, &content) .map(|(_, declarations)| { resolve_workspace_inheritance(&mut parsed.items, &declarations) From 0521ead41452202628a197a44a5a284b6cfd53bc Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:01:29 -0400 Subject: [PATCH 10/25] fix(fetch): report an unread version as undetermined, not as local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `evaluate_item` sent every non-checkable, non-git item to `DependencyStatus::Local`, so an `Inherited` entry that never found a version landed there through the wildcard arm. On a ``-inheriting POM — the dominant real-world shape, along with same-file `` — that produced a whole table of `local` rows: org.springframework.boot:spring-boot-starter-web — — local org.springframework.boot:spring-boot-starter-test — — local Totals: 2 skipped `local` is what this tool prints for a Cargo `path = "../x"` and a Maven `system` jar, and it means there is no registry behind the package. `spring-boot-starter-web` is on Maven Central; only its version went unread. `list` calls the same entry `(unresolved)`, so the two commands contradicted each other, and `"status": "LOCAL"` was the wrong token for a CI consumer to read. It now reports `Undetermined`, which claims nothing. An unresolved Cargo `workspace = true` reaches the same arm and is the same mistake, so it is corrected with it. A check also said none of this out loud: stderr was empty, and a reader looking at a column of dashes had to infer that nothing had been read. A manifest-level warning now names the dependencies and where their versions come from. It stays separate from `undeclared_inheritance` on purpose: a Cargo member inheriting a name its root never declared is a manifest Cargo refuses to build, while a POM deferring to its `` is ordinary, valid, and extremely common — the same status, two different things to tell a reader. Parser notices are forwarded into the same warnings, so a `` block `check` did not read is announced exactly as `list` announces it. --- crates/dependable-fetch/src/check.rs | 52 ++++++++++++++++++++++++ crates/dependable-fetch/tests/checker.rs | 9 +++- crates/dependable/tests/cli_workspace.rs | 6 ++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 2483a42..c39d42b 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -565,6 +565,7 @@ impl Checker { // `PackageSource::Inherited`, which is what keeps `--fix` off a span that means // nothing in this file. let mut warnings = Vec::new(); + warnings.extend(std::mem::take(&mut parsed.notices)); if let Some((root, declarations)) = &workspace { // The resolved names are the caller's business; the annotated items are ours. let _ = resolve_workspace_inheritance(&mut parsed.items, declarations); @@ -582,6 +583,10 @@ impl Checker { apply_lockfile(&mut parsed.items, &data); } + if let Some(warning) = deferred_versions(&parsed.items, kind) { + warnings.push(warning); + } + // Build the fetch task list, routing each checkable item to a fetcher: // JSR-sourced items (Deno `jsr:` deps) to the JSR fetcher, items naming a // resolved alternate Rust registry to that registry, and everything else @@ -789,6 +794,47 @@ fn undeclared_inheritance(items: &[Item], root: &Path) -> Vec { .collect() } +/// Say, once per manifest, that some of its entries state no version this file can +/// resolve — and therefore that nothing was checked for them. +/// +/// Without it the run is silent: those entries report as +/// [`DependencyStatus::Undetermined`] in a table a reader may not be reading, and +/// stderr says nothing at all. `undeclared_inheritance` above is the Cargo +/// equivalent and stays separate, because a Cargo member inheriting a name its root +/// never declared is a *broken* manifest, while a POM deferring to its `` is +/// an ordinary, valid, extremely common one — the same status, two different things +/// to tell the reader. +fn deferred_versions(items: &[Item], kind: ManifestKind) -> Option { + let source = match kind { + ManifestKind::PomXml => { + "a ``, ``, or a property this file does not declare" + } + _ => return None, + }; + let mut names: Vec<&str> = items + .iter() + .filter(|item| { + item.source == PackageSource::Inherited && item.version_constraint.is_empty() + }) + .map(|item| item.name.as_str()) + .collect(); + names.sort_unstable(); + names.dedup(); + if names.is_empty() { + return None; + } + let (subject, verb, object) = if names.len() == 1 { + ("dependency", "takes its version", "it") + } else { + ("dependencies", "take their version", "them") + }; + Some(format!( + "{} {subject} {verb} from {source}, so no version was read for {object} and nothing was checked: {}", + names.len(), + names.join(", ") + )) +} + /// Evaluate one parsed item against the fetched version lists, applying the /// configured pre-release filter before classification. fn evaluate_item( @@ -800,6 +846,12 @@ fn evaluate_item( if !item.is_checkable() { let status = match item.source { PackageSource::Git => DependencyStatus::Git, + // An entry that defers its version elsewhere and found nothing there is + // a real package on a real registry whose version this run never read. + // `Local` would say the opposite — that there is no registry for it — + // which of `spring-boot-starter-web` is simply false, and is the wrong + // token for a CI consumer to read. + PackageSource::Inherited => DependencyStatus::Undetermined, _ => DependencyStatus::Local, }; return CheckResult::new(item.clone(), status); diff --git a/crates/dependable-fetch/tests/checker.rs b/crates/dependable-fetch/tests/checker.rs index 07e9041..be18ec4 100644 --- a/crates/dependable-fetch/tests/checker.rs +++ b/crates/dependable-fetch/tests/checker.rs @@ -1045,7 +1045,12 @@ async fn a_member_is_checked_against_the_workspace_roots_constraint() { .iter() .find(|r| r.item.name == "serde") .expect("serde is declared"); - assert_eq!(serde.status, DependencyStatus::Local); + assert_eq!( + serde.status, + DependencyStatus::Undetermined, + "no root was found, so no version was read — not `Local`, which would say \ + serde has no registry" + ); assert!(serde.item.version_constraint.is_empty()); assert!(detached.workspace_root.is_none()); } @@ -1147,6 +1152,6 @@ async fn an_inherited_name_the_root_never_declared_is_reported() { "both declarations are still reported" ); for result in &check.results { - assert_eq!(result.status, DependencyStatus::Local); + assert_eq!(result.status, DependencyStatus::Undetermined); } } diff --git a/crates/dependable/tests/cli_workspace.rs b/crates/dependable/tests/cli_workspace.rs index 406497e..947a95f 100644 --- a/crates/dependable/tests/cli_workspace.rs +++ b/crates/dependable/tests/cli_workspace.rs @@ -202,7 +202,11 @@ fn a_constraint_the_root_never_declared_is_attributed_to_nobody() { let doc = check_json(&dir, &["check", "--manifest", member.to_str().unwrap()]); let tokio = result(&doc, "crates/app/Cargo.toml", "tokio"); - assert_eq!(tokio["status"], "LOCAL", "nothing to check: {tokio}"); + assert_eq!( + tokio["status"], "UNDETERMINED", + "the root declares no version, so nothing is known — `tokio` is on crates.io, \ + and calling it LOCAL would say it is not: {tokio}" + ); assert!( tokio["inherited_from"].is_null(), "the root declares no tokio: {tokio}" From dfcde31829dfd828739d5ef161609e078ed0faa8 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:01:51 -0400 Subject: [PATCH 11/25] fix(cli): count and gate an undetermined dependency apart from a skipped one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status now reaches every surface a `check` speaks through: the table (a colour of its own, since a gap in the report is not the same as a row there was nothing to say about), the totals line, `--format json`'s per-result token and its `summary.undetermined` counter, and `--format text`. The totals count it apart from `skipped`. A `path` dependency was passed over deliberately and there is nothing a stricter run could ever learn about it; an undetermined one is a version this run failed to read, and folding the two together is how a POM that yielded nothing came to print "2 skipped". --fail-on --------- `--fail-on any` no longer exits 0 over a manifest whose versions were never read; `--fail-on vulnerable` and `--fail-on outdated` still do. The two narrow gates ask a specific question — is anything vulnerable, is anything behind — and an unread version answers neither. Failing a build that asked about vulnerabilities because a POM defers to its `` would make the flag mean something other than what it says. `--fail-on any` asks the general one: is every dependency checked and current. A dependency whose version was never read is not current, it is unestablished, and exiting 0 asserts something the run never determined — which is exactly how a parent-inheriting POM used to go green while dependable had read nothing at all. An unreadable version is admittedly not a vulnerability, so the failure never travels alone: `check` names the dependencies on stderr, and a job that fails says what to fix. --- crates/dependable/src/cli.rs | 6 ++++++ crates/dependable/src/output/json.rs | 5 +++++ crates/dependable/src/output/mod.rs | 6 ++++++ crates/dependable/src/output/table.rs | 9 +++++++++ crates/dependable/src/runner.rs | 25 +++++++++++++++++++++++++ 5 files changed, 51 insertions(+) diff --git a/crates/dependable/src/cli.rs b/crates/dependable/src/cli.rs index ae3c7ba..d80c007 100644 --- a/crates/dependable/src/cli.rs +++ b/crates/dependable/src/cli.rs @@ -300,9 +300,15 @@ pub enum TreeFormat { #[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum FailOn { + /// Never fail: report and exit 0. None, + /// Fail on a dependency behind its latest release, or vulnerable. Outdated, + /// Fail only on a known advisory. Vulnerable, + /// Fail unless every dependency was checked and is current. A dependency + /// whose version could not be read counts against this: nothing was + /// established about it, so exiting 0 would claim more than the run knows. Any, } diff --git a/crates/dependable/src/output/json.rs b/crates/dependable/src/output/json.rs index eaf10d4..47f3696 100644 --- a/crates/dependable/src/output/json.rs +++ b/crates/dependable/src/output/json.rs @@ -27,6 +27,10 @@ struct SummaryDto { outdated: usize, vulnerable: usize, error: usize, + /// Declarations whose currency could not be established — a POM deferring to + /// its ``, an unresolved workspace inheritance. Additive, and + /// deliberately not folded into `error`: nothing failed, nothing was asked. + undetermined: usize, } #[derive(Serialize)] @@ -94,6 +98,7 @@ pub fn render(reports: &[ManifestReport]) -> anyhow::Result<()> { outdated: summary.outdated, vulnerable: summary.vulnerable, error: summary.error, + undetermined: summary.undetermined, }, results, }; diff --git a/crates/dependable/src/output/mod.rs b/crates/dependable/src/output/mod.rs index 5f20b25..e556afb 100644 --- a/crates/dependable/src/output/mod.rs +++ b/crates/dependable/src/output/mod.rs @@ -55,6 +55,11 @@ pub struct Summary { pub error: usize, pub local: usize, pub git: usize, + /// [`DependencyStatus::Undetermined`] count: declarations whose currency this + /// run could not establish. Kept apart from [`local`](Self::local) and + /// [`git`](Self::git), which are deliberately skipped and therefore clean, + /// because these were not skipped on purpose — nothing was learned about them. + pub undetermined: usize, } impl Summary { @@ -78,6 +83,7 @@ impl Summary { DependencyStatus::Error(_) => s.error += 1, DependencyStatus::Local => s.local += 1, DependencyStatus::Git => s.git += 1, + DependencyStatus::Undetermined => s.undetermined += 1, _ => {} } } diff --git a/crates/dependable/src/output/table.rs b/crates/dependable/src/output/table.rs index d23b753..23d8da3 100644 --- a/crates/dependable/src/output/table.rs +++ b/crates/dependable/src/output/table.rs @@ -109,6 +109,10 @@ fn status_cell(result: &CheckResult) -> String { DependencyStatus::Outdated | DependencyStatus::Error(_) => Style::new().red(), DependencyStatus::Vulnerable => Style::new().red().bold(), DependencyStatus::Local | DependencyStatus::Git => Style::new().dimmed(), + // Not dimmed with the deliberately-skipped rows beside it: this one is a + // gap in the report rather than a row there was nothing to say about, and + // `--fail-on any` fails the build over it. + DependencyStatus::Undetermined => Style::new().yellow(), _ => Style::new(), }; format!( @@ -148,6 +152,11 @@ fn print_totals(summary: &Summary) { if skipped > 0 { parts.push(format!("{skipped} skipped")); } + // Counted separately from `skipped`: a path dependency was passed over on + // purpose, an undetermined one is a version this run failed to read. + if summary.undetermined > 0 { + parts.push(format!("{} undetermined", summary.undetermined)); + } if parts.is_empty() { parts.push("nothing to check".to_string()); } diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 620671d..fce5e22 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -1319,6 +1319,29 @@ fn expand_env(content: &str) -> String { out } +/// The exit code for a finished run under the configured gate. +/// +/// # Where [`DependencyStatus::Undetermined`] sits +/// +/// It is **clean** under `--fail-on vulnerable` and `--fail-on outdated`, and +/// **not clean** under `--fail-on any`. +/// +/// Those two named gates ask a specific question — is anything vulnerable, is +/// anything behind — and an unread version answers neither. Failing a build that +/// asked about vulnerabilities because a POM defers to its `` would make +/// the flag mean something other than what it says. +/// +/// `--fail-on any` asks the general one: is every dependency checked and current. +/// A dependency whose version was never read is not current — it is unestablished, +/// and exiting `0` asserts something this run never determined. That is precisely +/// how a parent-inheriting POM used to pass green while dependable had read +/// nothing at all. It is grouped with the failures rather than with +/// [`DependencyStatus::Local`] and [`DependencyStatus::Git`], which are clean +/// because they were skipped *on purpose*: there is no registry behind them, so +/// there is nothing a stricter run could ever learn. +/// +/// The status never travels alone: `check` emits a manifest-level warning on +/// stderr naming the dependencies involved, so a failing job says what to fix. fn exit_code(reports: &[ManifestReport], fail_on: FailOn) -> ExitCode { let triggered = reports .iter() @@ -1332,6 +1355,8 @@ fn exit_code(reports: &[ManifestReport], fail_on: FailOn) -> ExitCode { | DependencyStatus::UpdateAvailable | DependencyStatus::Vulnerable ), + // `Undetermined` is absent from this clean list on purpose — see the + // doc comment above. FailOn::Any => !matches!( result.status, DependencyStatus::UpToDate | DependencyStatus::Local | DependencyStatus::Git From fbec08a8dd23a197fafdb2a5803a0efcc12716e9 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:02:10 -0400 Subject: [PATCH 12/25] fix(cli): agree with source about whether a dependency is inherited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `list --format json` emitted `"source": "inherited"` beside `"inherited": false` on the same object. The boolean was filled in only from Cargo workspace resolution, so every entry the Gradle-catalog and POM parsers mark inherited arrived contradicting the field next to it, and a consumer reading both got two answers. Both fields describe one fact — this dependency's version is declared somewhere other than its own entry — so the boolean is now read from the source. The workspace-root list still contributes, because a root declaring a crate by `path` replaces `source` outright and the fact that it was inherited would otherwise be lost. No schema change: the key, its type, and its meaning for a Cargo manifest are all unchanged. --- crates/dependable/src/output/list.rs | 12 ++++++++++-- crates/dependable/tests/cli_workspace.rs | 10 +++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/dependable/src/output/list.rs b/crates/dependable/src/output/list.rs index 04fe9e9..49d8878 100644 --- a/crates/dependable/src/output/list.rs +++ b/crates/dependable/src/output/list.rs @@ -175,7 +175,8 @@ fn json(reports: &[ProjectReport], root: &Path) -> anyhow::Result<()> { source: source_token(item.source), locked: item.locked_version.as_deref(), registry: item.registry.as_deref(), - inherited: report.inherited.contains(&item.name), + inherited: item.source == PackageSource::Inherited + || report.inherited.contains(&item.name), features: report.features.get(&item.name).map(Vec::as_slice), license: report.licenses.get(&item.name).map(String::as_str), }) @@ -235,7 +236,14 @@ struct DependencyDto<'a> { source: &'static str, locked: Option<&'a str>, registry: Option<&'a str>, - /// Whether the constraint came from the workspace root rather than this manifest. + /// Whether this dependency's version is declared somewhere other than its own + /// entry: a Cargo `workspace = true` resolved against the root, a Gradle + /// `[versions]` alias, a shared Maven `` value. + /// + /// True for every entry whose `source` is `inherited`, so the two fields can no + /// longer contradict each other on the same object. Also true where the root's + /// declaration supplied a `path` or `git` source, which replaces `source` + /// outright and would otherwise lose the fact that it was inherited at all. inherited: bool, #[serde(skip_serializing_if = "Option::is_none")] features: Option<&'a [String]>, diff --git a/crates/dependable/tests/cli_workspace.rs b/crates/dependable/tests/cli_workspace.rs index 947a95f..5495633 100644 --- a/crates/dependable/tests/cli_workspace.rs +++ b/crates/dependable/tests/cli_workspace.rs @@ -182,7 +182,15 @@ fn a_relative_path_never_adopts_the_current_directorys_workspace() { dependency["constraint"], "9.9.9", "took the constraint from a workspace that is not an ancestor: {dependency}" ); - assert_eq!(dependency["inherited"], false, "{dependency}"); + assert!( + dependency["constraint"].is_null(), + "no constraint was adopted, so none is reported: {dependency}" + ); + // `inherited` says the entry defers its version elsewhere, which this one does + // — it says nothing about whether anything was found there. It agrees with + // `source` by construction, so the two can never contradict each other. + assert_eq!(dependency["source"], "inherited", "{dependency}"); + assert_eq!(dependency["inherited"], true, "{dependency}"); } /// `workspace_root` names the manifest that *governs* this one, whether or not anything From c1b510f05a10dffe8a1bda54dec24d3f45138a20 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:02:32 -0400 Subject: [PATCH 13/25] fix(report): keep an unread version out of the up-to-date denominator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Summary::checkable` subtracted only the path and git dependencies, so an undetermined one sat below the line as if the run had checked it and found it behind. On a parent-inheriting POM that silently depressed the up-to-date percentage toward nothing. It is now subtracted too, and counted in a field of its own beside `local` and `git`. SARIF emits no finding for it, which is the same decision `Error` gets and for the same reason: nothing was learned, so there is nothing to report about the code — and there is no line to pin a finding to either, since a dependency that defers its version elsewhere records no span. The CLI's manifest-level warning is where that belongs. The HTML badge and the TUI's detail pane both give it a colour of its own rather than the muted one the deliberately-skipped statuses share. The TUI *tree* still shows a green `ok` for a non-Cargo package, for an unrelated reason that predates this branch and spans csproj, mix, and Gradle as well: `direct_graph` zeroes the version. Filed as #96 and deliberately not touched here. --- .../src/html/templates/styles.css | 3 ++ crates/dependable-report/src/sarif.rs | 6 ++- crates/dependable-report/src/summary.rs | 45 ++++++++++++++++--- .../dependable-report/tests/golden/empty.html | 3 ++ .../dependable-report/tests/golden/full.html | 3 ++ .../tests/golden/single_ecosystem.html | 3 ++ crates/dependable-tui/src/ui/detail.rs | 4 ++ 7 files changed, 59 insertions(+), 8 deletions(-) diff --git a/crates/dependable-report/src/html/templates/styles.css b/crates/dependable-report/src/html/templates/styles.css index 304b63e..5271e76 100644 --- a/crates/dependable-report/src/html/templates/styles.css +++ b/crates/dependable-report/src/html/templates/styles.css @@ -88,6 +88,9 @@ code, .ver, .vector { font-family: ui-monospace, SFMono-Regular, "SF Mono", Menl .st-OK { border-color: var(--ok); color: var(--ok); } .st-ERROR { border-color: var(--bad); color: var(--bad); } .sev-UNRATED, .sev-NONE, .st-LOCAL, .st-GIT { color: var(--muted); } +/* Not muted with the skipped statuses: nothing is known about this row, which is + a gap in the report rather than a row there was nothing to say about. */ +.st-UNDETERMINED { border-color: var(--warn); color: var(--warn); } .withdrawn { color: var(--muted); text-transform: lowercase; } .score { color: var(--muted); font-size: .8rem; } diff --git a/crates/dependable-report/src/sarif.rs b/crates/dependable-report/src/sarif.rs index 65e8de8..caf3eb3 100644 --- a/crates/dependable-report/src/sarif.rs +++ b/crates/dependable-report/src/sarif.rs @@ -184,7 +184,11 @@ fn findings(report: &Report) -> Vec { // Security tab drowning in notes. // `UpToDate`, `Local` and `Git` have nothing to report, and // `Error` is a tool failure rather than a finding about the code - // — the CLI already puts it on stderr. + // — the CLI already puts it on stderr. `Undetermined` is the same + // shape as `Error`: nothing was learned, so there is no finding to + // pin to a line — and it has no line to pin one to, since a + // dependency deferring its version elsewhere records no span. The + // CLI's manifest-level warning is where that belongs. _ => {} } } diff --git a/crates/dependable-report/src/summary.rs b/crates/dependable-report/src/summary.rs index e635b05..18d1df6 100644 --- a/crates/dependable-report/src/summary.rs +++ b/crates/dependable-report/src/summary.rs @@ -23,9 +23,13 @@ pub struct Summary { pub manifests: usize, /// Every declared dependency across every manifest. pub total: usize, - /// Dependencies that could actually be checked against a registry: - /// [`Self::total`] minus the path and git dependencies. The only honest - /// denominator for an "up to date" percentage. + /// Dependencies whose currency this run actually established: + /// [`Self::total`] minus the path and git dependencies, and minus the ones + /// whose version could not be read at all. The only honest denominator for an + /// "up to date" percentage — an + /// [`Undetermined`](DependencyStatus::Undetermined) dependency is neither up to + /// date nor behind, so counting it below the line would quietly depress the + /// percentage on every parent-inheriting POM. pub checkable: usize, /// [`DependencyStatus::UpToDate`] count. pub up_to_date: usize, @@ -43,6 +47,10 @@ pub struct Summary { pub local: usize, /// [`DependencyStatus::Git`] count. pub git: usize, + /// [`DependencyStatus::Undetermined`] count: dependencies this run could not + /// establish anything about. Excluded from [`Self::checkable`], and kept apart + /// from [`Self::local`] and [`Self::git`], which were skipped deliberately. + pub undetermined: usize, /// Distinct `(dependency, advisory ID)` pairs — one per row a vulnerability /// table would print. The same advisory affecting three packages counts three /// times here and once in [`Self::distinct_advisories`]. @@ -209,6 +217,7 @@ impl Report { DependencyStatus::Error(_) => summary.error += 1, DependencyStatus::Local => summary.local += 1, DependencyStatus::Git => summary.git += 1, + DependencyStatus::Undetermined => summary.undetermined += 1, // `DependencyStatus` is `#[non_exhaustive]`; an unrecognized // status still counts toward the total and nothing else. _ => {} @@ -239,7 +248,7 @@ impl Report { } } - summary.checkable = summary.total - summary.local - summary.git; + summary.checkable = summary.total - summary.local - summary.git - summary.undetermined; summary.distinct_advisories = seen_advisories.len(); summary.withdrawn_advisories = withdrawn.len(); // The same key the HTML pie chart sorts its slices by, so the chart and @@ -300,13 +309,14 @@ mod tests { ("broken", DependencyStatus::Error("502".into())), ("mine", DependencyStatus::Local), ("forked", DependencyStatus::Git), + ("unread", DependencyStatus::Undetermined), ]), )]); let summary = report.summary(); assert_eq!(summary.manifests, 1); - assert_eq!(summary.total, 8); + assert_eq!(summary.total, 9); assert_eq!(summary.up_to_date, 1); assert_eq!(summary.patch_available, 1); assert_eq!(summary.update_available, 1); @@ -315,11 +325,32 @@ mod tests { assert_eq!(summary.error, 1); assert_eq!(summary.local, 1); assert_eq!(summary.git, 1); - // Path and git dependencies have no registry verdict, so they are not - // part of the denominator. + assert_eq!(summary.undetermined, 1); + // Path and git dependencies have no registry verdict, and an undetermined + // one produced none, so none of the three is part of the denominator — an + // unread version is neither up to date nor behind, and counting it below + // the line would depress the percentage on every parent-inheriting POM. assert_eq!(summary.checkable, 6); } + /// A run that read nothing is not a run that is 100% up to date. + #[test] + fn an_undetermined_dependency_is_outside_the_up_to_date_denominator() { + let report = report(vec![ManifestResults::new( + PathBuf::from("pom.xml"), + Ecosystem::Jvm, + results(&[ + ("serde", DependencyStatus::UpToDate), + ("tokio", DependencyStatus::Undetermined), + ]), + )]); + + let summary = report.summary(); + + assert_eq!(summary.checkable, 1); + assert_eq!(summary.up_to_date_percent(), Some(100.0)); + } + #[test] fn up_to_date_percent_is_none_when_nothing_is_checkable() { let report = report(vec![ManifestResults::new( diff --git a/crates/dependable-report/tests/golden/empty.html b/crates/dependable-report/tests/golden/empty.html index 1dbc845..8fcab68 100644 --- a/crates/dependable-report/tests/golden/empty.html +++ b/crates/dependable-report/tests/golden/empty.html @@ -87,6 +87,9 @@ .st-OK { border-color: var(--ok); color: var(--ok); } .st-ERROR { border-color: var(--bad); color: var(--bad); } .sev-UNRATED, .sev-NONE, .st-LOCAL, .st-GIT { color: var(--muted); } +/* Not muted with the skipped statuses: nothing is known about this row, which is + a gap in the report rather than a row there was nothing to say about. */ +.st-UNDETERMINED { border-color: var(--warn); color: var(--warn); } .withdrawn { color: var(--muted); text-transform: lowercase; } .score { color: var(--muted); font-size: .8rem; } diff --git a/crates/dependable-report/tests/golden/full.html b/crates/dependable-report/tests/golden/full.html index 696aa0c..9539e6b 100644 --- a/crates/dependable-report/tests/golden/full.html +++ b/crates/dependable-report/tests/golden/full.html @@ -87,6 +87,9 @@ .st-OK { border-color: var(--ok); color: var(--ok); } .st-ERROR { border-color: var(--bad); color: var(--bad); } .sev-UNRATED, .sev-NONE, .st-LOCAL, .st-GIT { color: var(--muted); } +/* Not muted with the skipped statuses: nothing is known about this row, which is + a gap in the report rather than a row there was nothing to say about. */ +.st-UNDETERMINED { border-color: var(--warn); color: var(--warn); } .withdrawn { color: var(--muted); text-transform: lowercase; } .score { color: var(--muted); font-size: .8rem; } diff --git a/crates/dependable-report/tests/golden/single_ecosystem.html b/crates/dependable-report/tests/golden/single_ecosystem.html index 642aaad..5f1f23f 100644 --- a/crates/dependable-report/tests/golden/single_ecosystem.html +++ b/crates/dependable-report/tests/golden/single_ecosystem.html @@ -87,6 +87,9 @@ .st-OK { border-color: var(--ok); color: var(--ok); } .st-ERROR { border-color: var(--bad); color: var(--bad); } .sev-UNRATED, .sev-NONE, .st-LOCAL, .st-GIT { color: var(--muted); } +/* Not muted with the skipped statuses: nothing is known about this row, which is + a gap in the report rather than a row there was nothing to say about. */ +.st-UNDETERMINED { border-color: var(--warn); color: var(--warn); } .withdrawn { color: var(--muted); text-transform: lowercase; } .score { color: var(--muted); font-size: .8rem; } diff --git a/crates/dependable-tui/src/ui/detail.rs b/crates/dependable-tui/src/ui/detail.rs index 33b07db..40fcc77 100644 --- a/crates/dependable-tui/src/ui/detail.rs +++ b/crates/dependable-tui/src/ui/detail.rs @@ -416,6 +416,10 @@ fn status_style(status: &DependencyStatus) -> Style { } DependencyStatus::Outdated => theme::fg(Token::Critical), DependencyStatus::Vulnerable => theme::bold(Token::Critical), + // Nothing was established about this dependency, which is not the same as + // there being nothing to establish — it reads as a gap, not as a skip. + DependencyStatus::Undetermined => theme::fg(Token::Warn), + // `Local`, `Git`, `Error`, and any status a later release adds. _ => theme::fg(Token::Muted), } } From eee2821d953a856f9cc48f073da63b25025aad2f Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 00:03:55 -0400 Subject: [PATCH 14/25] test(jvm): check a pom that takes its versions from its parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing Maven fixture exercised `list --format json` only, which is why `check` calling every unresolvable entry `local` went unseen: `list` renders the parser's output directly, and the mislabel happened a stage later. These drive the binary end to end against a ``-only POM and a same-file `` one, hermetically — the JVM registry points at a port nothing listens on, so the connection is refused at once and nothing here depends on what a registry would have said. What is under test is decided before any request is made. Covered: the status in `--format json`, in the table, and in `--format text`; the `summary.undetermined` counter; that `--fail-on any` fails while `--fail-on outdated` and `--fail-on vulnerable` stay green; that the warning naming the dependencies reaches stderr; that a `system` jar is still `LOCAL` and still clean, so the distinction between "no registry" and "nothing read" is pinned from both sides; that a profiles-only POM says why it lists nothing; and that `source` and `inherited` agree on every object. --- crates/dependable/tests/fixture_maven.rs | 298 ++++++++++++++++++++++- 1 file changed, 297 insertions(+), 1 deletion(-) diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs index ac10545..137efb0 100644 --- a/crates/dependable/tests/fixture_maven.rs +++ b/crates/dependable/tests/fixture_maven.rs @@ -1,9 +1,16 @@ //! Offline parse of the Maven fixture: a POM's coordinates, `${property}` //! resolution, version-span round-tripping, and what a version this file cannot //! resolve is reported as. +//! +//! The `check`-level tests are hermetic the same way `cli_workspace` is: the JVM +//! registry points at `http://127.0.0.1:1`, where the connection is refused at +//! once. Nothing here depends on what a registry would have said — what is under +//! test is how a dependency this file states **no version for** is classified, +//! which is decided before any request is made. +use std::fs; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Output}; use dependable_fetch::core::{DependencyKind, Item, PackageSource, parse, parse_project}; use dependable_fetch::{Ecosystem, ManifestKind}; @@ -175,4 +182,293 @@ fn the_cli_lists_an_unresolvable_dependency_instead_of_omitting_it() { let guava = dependency("com.google.guava:guava"); assert_eq!(guava["constraint"], "32.1.3-jre"); assert_eq!(guava["source"], "registry"); + assert_eq!(guava["inherited"], false, "its version is its own: {guava}"); + + // `source` and `inherited` describe the same fact, so they can never disagree + // on one object. They used to: the boolean was filled in only by Cargo + // workspace resolution, so every non-Cargo `"source": "inherited"` arrived + // beside `"inherited": false`, and a consumer reading both got a + // contradiction. + for name in [ + "com.fasterxml.jackson.core:jackson-core", + "com.fasterxml.jackson.core:jackson-databind", + "org.springframework.boot:spring-boot-starter-web", + ] { + let entry = dependency(name); + assert_eq!(entry["source"], "inherited", "{entry}"); + assert_eq!(entry["inherited"], true, "{entry}"); + } +} + +/// A registry nothing is listening on: `connect` fails at once. +const OFFLINE: &str = "[jvm]\nregistry = \"http://127.0.0.1:1\"\n"; + +/// A temp directory holding one `pom.xml`, walled off from this repository. +fn pom_dir(name: &str, body: &str) -> PathBuf { + let dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(name); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + // A repository boundary, so the discovery walk stays inside the temp dir. + fs::create_dir_all(dir.join(".git")).unwrap(); + fs::write(dir.join("dependable.toml"), OFFLINE).unwrap(); + fs::write( + dir.join("pom.xml"), + format!( + "\n \ + com.example\n \ + app\n \ + 1.0.0\n{body}\n" + ), + ) + .unwrap(); + dir +} + +/// Run the CLI in `dir`. `--config` and the hermetic flags belong to the +/// subcommand, not to the binary, and `list` takes no config at all — it reads no +/// registry, so it needs none. +fn run(dir: &Path, args: &[&str]) -> Output { + let config = dir.join("dependable.toml"); + let mut all: Vec = args.iter().map(|a| (*a).to_string()).collect(); + if args[0] != "list" { + all.push("--config".to_string()); + all.push(config.to_string_lossy().into_owned()); + } + if args[0] == "check" { + all.push("--no-vuln".to_string()); + all.push("--no-cache".to_string()); + } + Command::new(env!("CARGO_BIN_EXE_dependable")) + .current_dir(dir) + .args(all) + .output() + .expect("run dependable") +} + +fn check_json(dir: &Path, extra: &[&str]) -> Value { + let mut args: Vec<&str> = vec!["check", ".", "--format", "json"]; + args.extend_from_slice(extra); + let output = run(dir, &args); + serde_json::from_slice(&output.stdout).unwrap_or_else(|e| { + panic!( + "invalid JSON ({e}): {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) + }) +} + +fn status_of<'a>(doc: &'a Value, name: &str) -> &'a Value { + doc["results"] + .as_array() + .expect("results array") + .iter() + .find(|r| r["name"] == name) + .unwrap_or_else(|| panic!("no result {name}: {}", doc["results"])) +} + +/// The dominant real-world POM: a `` supplies every version, so this file +/// states none. +const PARENT_ONLY: &str = " \n \ + org.springframework.boot\n \ + spring-boot-starter-parent\n \ + 3.2.5\n \ + \n \ + \n \ + \n \ + org.springframework.boot\n \ + spring-boot-starter-web\n \ + \n \ + \n \ + org.springframework.boot\n \ + spring-boot-starter-test\n \ + test\n \ + \n \ + \n"; + +/// `check` must not call a dependency it merely failed to read a **local** one. +/// +/// `local` is what this tool prints for a Cargo `path = "../x"` and a Maven +/// `system` jar, and it means one thing: there is no registry +/// behind this. Said of `spring-boot-starter-web` — which is on Maven Central — +/// it is a plain false statement, and `LOCAL` is the wrong token for a CI job to +/// read. It also contradicted `list`, which calls the same entry `(unresolved)`. +#[test] +fn a_version_supplied_by_a_parent_is_undetermined_and_never_called_local() { + let dir = pom_dir("maven_parent_only", PARENT_ONLY); + let doc = check_json(&dir, &[]); + + for artifact in ["spring-boot-starter-web", "spring-boot-starter-test"] { + let name = format!("org.springframework.boot:{artifact}"); + let result = status_of(&doc, &name); + assert_eq!( + result["status"], "UNDETERMINED", + "the artifact is on Maven Central; only its version went unread: {result}" + ); + assert_ne!(result["status"], "LOCAL", "{result}"); + } + assert_eq!(doc["summary"]["undetermined"], 2, "{}", doc["summary"]); + assert_eq!( + doc["summary"]["error"], 0, + "nothing was asked, so nothing failed" + ); +} + +/// The same fact in the table and in `--format text`, since those are what a +/// person and a shell pipeline actually read. +#[test] +fn the_table_and_the_text_format_both_say_undetermined() { + let dir = pom_dir("maven_parent_only_surfaces", PARENT_ONLY); + + let table = run(&dir, &["check", "."]); + let stdout = String::from_utf8_lossy(&table.stdout); + assert!( + stdout.contains("undetermined"), + "the table calls it undetermined: {stdout}" + ); + assert!(!stdout.contains("local"), "and never local: {stdout}"); + assert!( + stdout.contains("2 undetermined"), + "the totals count it apart from the deliberately skipped: {stdout}" + ); + + let text = run(&dir, &["check", ".", "--format", "text"]); + let stdout = String::from_utf8_lossy(&text.stdout); + assert_eq!( + stdout.matches("UNDETERMINED").count(), + 2, + "one stable token per unread dependency: {stdout}" + ); +} + +/// A CI job that reads nothing must not go green. `--fail-on any` asks whether +/// every dependency is checked and current; an unread version answers neither. +/// The two narrower gates keep meaning what they say. +#[test] +fn fail_on_any_does_not_pass_a_pom_whose_versions_were_never_read() { + let dir = pom_dir("maven_parent_only_gate", PARENT_ONLY); + for (gate, expected) in [("any", false), ("outdated", true), ("vulnerable", true)] { + let output = run(&dir, &["check", ".", "--fail-on", gate]); + assert_eq!( + output.status.success(), + expected, + "--fail-on {gate}: {}", + String::from_utf8_lossy(&output.stdout) + ); + } +} + +/// Nothing anywhere used to say that a POM's versions had not been read: stderr +/// was empty, and a reader looking at a table of dashes had to infer it. +#[test] +fn a_pom_whose_versions_were_not_read_says_so_on_stderr() { + let dir = pom_dir("maven_parent_only_warning", PARENT_ONLY); + let output = run(&dir, &["check", ".", "--format", "json"]); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!(stderr.contains("2 dependencies"), "{stderr}"); + assert!(stderr.contains(""), "{stderr}"); + assert!(stderr.contains(""), "{stderr}"); + assert!( + stderr.contains("org.springframework.boot:spring-boot-starter-web"), + "the warning names them, so the reader knows what to fix: {stderr}" + ); +} + +/// The same-file `` case, which needs no `` at all +/// and is just as common. +#[test] +fn a_version_supplied_by_the_files_own_dependency_management_is_undetermined() { + let dir = pom_dir( + "maven_managed_only", + " \n \ + \n \ + \n \ + com.google.guava\n \ + guava\n \ + 32.1.3-jre\n \ + \n \ + \n \ + \n \ + \n \ + \n \ + com.google.guava\n \ + guava\n \ + \n \ + \n", + ); + let doc = check_json(&dir, &[]); + let guava = status_of(&doc, "com.google.guava:guava"); + assert_eq!(guava["status"], "UNDETERMINED", "{guava}"); +} + +/// A POM that says nothing about its versions is not the same as one whose +/// dependencies genuinely have no registry. A `system` jar is +/// still `LOCAL`, and must stay that way, or the new status means nothing. +#[test] +fn a_system_scoped_jar_is_still_local() { + let dir = pom_dir( + "maven_system_scope", + " \n \ + \n \ + org.example\n \ + vendored\n \ + 1.0.0\n \ + system\n \ + /opt/vendored.jar\n \ + \n \ + \n", + ); + let doc = check_json(&dir, &[]); + let vendored = status_of(&doc, "org.example:vendored"); + assert_eq!( + vendored["status"], "LOCAL", + "there is no registry: {vendored}" + ); + assert_eq!(doc["summary"]["undetermined"], 0, "{}", doc["summary"]); + + // And `--fail-on any` stays green over it: it was skipped on purpose. + let output = run(&dir, &["check", ".", "--fail-on", "any"]); + assert!(output.status.success()); +} + +/// A POM whose dependencies all live in `` used to list as +/// `(0 dependencies)` — a list that reads as complete and is not. Excluding +/// conditional dependencies stays the decision; being silent about it does not. +#[test] +fn a_pom_whose_dependencies_are_all_in_profiles_says_so() { + let dir = pom_dir( + "maven_profiles_only", + " \n \ + \n \ + native\n \ + \n \ + \n \ + com.google.guava\n \ + guava\n \ + 32.1.3-jre\n \ + \n \ + \n \ + \n \ + \n", + ); + + let listed = run(&dir, &["list", "."]); + let stdout = String::from_utf8_lossy(&listed.stdout); + let stderr = String::from_utf8_lossy(&listed.stderr); + assert!( + stdout.contains("(0 dependencies)"), + "a profile dependency is still not listed: {stdout}" + ); + assert!( + stderr.contains("1 dependency declared inside "), + "but the reader is told why the list is empty: {stderr}" + ); + + let checked = run(&dir, &["check", "."]); + assert!( + String::from_utf8_lossy(&checked.stderr).contains(""), + "and `check` says it too: {}", + String::from_utf8_lossy(&checked.stderr) + ); } From 412f7b4288e519c2a993aca9f8b7fadf13eadbe4 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 02:20:49 -0400 Subject: [PATCH 15/25] fix(jvm): report a dependency whose group this POM cannot resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `${project.groupId}` is the standard way a multi-module build names a sibling module, and it names a Maven built-in that is not a `` entry — so the coordinate could not be resolved and the whole dependency was dropped. A POM declaring three dependencies, two of them siblings, listed as depending on one, with nothing on stderr to say otherwise. A missing `` element went the same way. That is the silent omission this parser already refuses for a `` a `` supplies, reached through the other half of the coordinate — and the asymmetry was inside one function: an unresolvable version was reported with an empty constraint, an unresolvable group deleted the entry even though the artifact was named. Each half is now read as text first and resolved second: the resolved value where this file states one, the literal as written where it does not. An entry either half of which is unresolved reports no constraint either, since a coordinate this file cannot state is one nothing can be fetched for. It lands in `Undetermined`, beside the parent-deferred ones. Only a `` naming neither half is skipped, having stated nothing to report. --- crates/dependable-core/src/parsers/pom_xml.rs | 149 +++++++++++++++--- crates/dependable/tests/fixture_maven.rs | 45 ++++++ 2 files changed, 172 insertions(+), 22 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 07219ac..f0d64c2 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -42,8 +42,13 @@ //! built-in properties (`${project.version}`, `${revision}`, …) are not //! `` entries and are likewise not resolved. //! -//! A dependency whose version those rules leave unknown is **reported anyway**, -//! with no constraint and no span — the shape a Cargo member's unresolved +//! The same holds for a coordinate: `${project.groupId}` — the +//! standard idiom for a sibling module in a multi-module build — names a built-in +//! this file does not state, and the dependency is reported under that literal +//! rather than dropped. +//! +//! A dependency whose version or coordinate those rules leave unknown is +//! **reported anyway**, with no constraint and no span — the shape a Cargo member's unresolved //! `dep.workspace = true` already has, which the CLI renders as `(unresolved)` and //! never fetches, fixes, or claims a status for. Dropping it instead, as the //! `csproj` parser drops an MSBuild `$(…)` version, would report a POM that @@ -93,6 +98,14 @@ enum Source { Unknown, } +/// One half of a `groupId:artifactId` coordinate: the text to report it under, and +/// whether that text is the resolved value or the literal this file could not +/// resolve. +struct Half { + text: String, + resolved: bool, +} + /// One ``, read but not yet resolved. struct Declared { name: String, @@ -177,22 +190,36 @@ fn read_dependencies<'a>( if node.tag_name().name() != "dependency" { continue; } - // Without both halves there is no coordinate to look up, and nothing that - // could be reported under a name. A `${…}` group is interpolated first, - // since `${project.groupId}` aside, a group is often a property. - let (Some(group), Some(artifact)) = ( - interpolated(node, "groupId", properties), - interpolated(node, "artifactId", properties), - ) else { + // A `${…}` half is interpolated, since `${project.groupId}` aside, a group is + // often a property. A half this file cannot resolve is reported under the + // literal it states rather than dropped — see [`coordinate`]. + let group = coordinate(node, "groupId", properties); + let artifact = coordinate(node, "artifactId", properties); + // A `` naming neither half states nothing at all: there is no + // text to report it under, so there is nothing to report. + if group.is_none() && artifact.is_none() { continue; - }; + } + let known = group.as_ref().is_some_and(|half| half.resolved) + && artifact.as_ref().is_some_and(|half| half.resolved); + let name = format!( + "{}:{}", + group.map(|half| half.text).unwrap_or_default(), + artifact.map(|half| half.text).unwrap_or_default(), + ); let scope = child(node, "scope") .and_then(text_of) .map(|located| located.value) .unwrap_or_default(); out.push(Declared { - name: format!("{group}:{artifact}"), - version: version_source(node, properties), + name, + // A coordinate this file cannot state is a coordinate nothing can be + // fetched for, whatever version sits beside it. + version: if known { + version_source(node, properties) + } else { + Source::Unknown + }, // A `system` dependency is a jar at a path on this machine, not // something a registry has ever heard of. source: match scope.as_str() { @@ -273,18 +300,38 @@ fn interpolation(value: &str) -> Option<&str> { Some(inner) } -/// A child element's text, with `${…}` resolved against `properties`. -fn interpolated( +/// One half of a coordinate, with `${…}` resolved against `properties` where it can +/// be — and reported as written where it cannot. +/// +/// `None` only when the element is absent or states nothing. An *unresolvable* half +/// (`${project.groupId}`, the standard idiom for a sibling module, or a property +/// declared in a ``) still yields the literal text, because dropping the +/// dependency would report a POM as depending on fewer things than it declares — +/// the silent omission this parser exists to avoid, and an asymmetry with +/// ``, which is reported unresolved rather than deleted. +fn coordinate( node: roxmltree::Node<'_, '_>, tag: &str, properties: &HashMap, -) -> Option { +) -> Option { let located = child(node, tag).and_then(text_of)?; - match interpolation(&located.value) { - Some(reference) => Some(properties[terminal(reference, properties)?].value.clone()), + let resolved = match interpolation(&located.value) { + Some(reference) => terminal(reference, properties) + .map(|name| properties[name].value.clone()) + .map(|text| Half { + text, + resolved: true, + }), None if located.value.contains('$') => None, - None => Some(located.value), - } + None => Some(Half { + text: located.value.clone(), + resolved: true, + }), + }; + Some(resolved.unwrap_or(Half { + text: located.value, + resolved: false, + })) } /// Count every `${…}` reference in the document, by the property its chain ends at. @@ -733,8 +780,10 @@ mod tests { assert_eq!(find(&m, "g:a").version_constraint, ""); } - /// A group stated as a property still names a package; one that is not - /// resolvable names nothing that could be looked up. + /// A group stated as a property still names a package. One that is not + /// resolvable names nothing that could be looked up — but the dependency is + /// still declared, so it is reported under the literal it states rather than + /// deleted from the manifest's own list. #[test] fn a_coordinate_may_be_stated_by_property() { let content = pom(" \n\ @@ -754,7 +803,63 @@ mod tests { \x20 \n"); let m = parse(&content); let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); - assert_eq!(names, vec!["org.example:named"]); + assert_eq!( + names, + vec!["org.example:named", "${project.groupId}:unnameable"] + ); + } + + /// `${project.groupId}` is *the* idiom for a sibling module in a multi-module + /// build, and a POM using it for two of its three dependencies must not list as + /// depending on one. Reported under the literal, with no constraint — the shape + /// a parent-deferred version already has, which the CLI renders `(unresolved)` + /// and never fetches — rather than dropped in silence. + #[test] + fn a_group_this_file_cannot_resolve_is_reported_not_dropped() { + let content = pom(" \n\ + \x20 \n\ + \x20 ${project.groupId}\n\ + \x20 app-core\n\ + \x20 ${project.version}\n\ + \x20 \n\ + \x20 \n\ + \x20 org.slf4j\n\ + \x20 slf4j-api\n\ + \x20 2.0.13\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + assert_eq!(m.items.len(), 2, "both dependencies are declared"); + let sibling = find(&m, "${project.groupId}:app-core"); + assert_eq!(sibling.version_constraint, ""); + assert_eq!(sibling.source, PackageSource::Inherited); + assert!( + !sibling.is_checkable(), + "no coordinate, so nothing to fetch" + ); + assert!(!sibling.is_rewritable()); + assert_eq!(find(&m, "org.slf4j:slf4j-api").version_constraint, "2.0.13"); + } + + /// A `` this file never states is the same omission by a shorter + /// route: the artifact is named, so the entry is reported under what there is. + /// A `` naming neither half states nothing at all and is skipped. + #[test] + fn a_missing_coordinate_half_is_still_reported_under_the_other() { + let content = pom(" \n\ + \x20 \n\ + \x20 orphan\n\ + \x20 1.0.0\n\ + \x20 \n\ + \x20 \n\ + \x20 2.0.0\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); + assert_eq!(names, vec![":orphan"]); + assert_eq!(m.items[0].version_constraint, ""); + assert!(!m.items[0].is_checkable()); } /// Whitespace around a version is formatting, not part of it, and the span has diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs index 137efb0..4e3c10d 100644 --- a/crates/dependable/tests/fixture_maven.rs +++ b/crates/dependable/tests/fixture_maven.rs @@ -472,3 +472,48 @@ fn a_pom_whose_dependencies_are_all_in_profiles_says_so() { String::from_utf8_lossy(&checked.stderr) ); } + +/// `${project.groupId}` is the standard way a multi-module build names a sibling +/// module, and this file cannot resolve it — Maven's built-ins are not +/// `` entries. Dropping the entry made `list` report a POM declaring +/// three dependencies as having one, with nothing on stderr: the same silent +/// omission a parent-supplied `` used to cause, reached through the +/// coordinate instead of the version. +#[test] +fn a_group_this_file_cannot_resolve_is_still_listed() { + let dir = pom_dir( + "maven_unresolvable_group", + " \n \ + \n \ + ${project.groupId}\n \ + app-core\n \ + ${project.version}\n \ + \n \ + \n \ + ${project.groupId}\n \ + app-web\n \ + ${project.version}\n \ + \n \ + \n \ + org.slf4j\n \ + slf4j-api\n \ + 2.0.13\n \ + \n \ + \n", + ); + + let listed = run(&dir, &["list", "."]); + let stdout = String::from_utf8_lossy(&listed.stdout); + assert!(stdout.contains("3 dependencies"), "{stdout}"); + for artifact in ["app-core", "app-web", "slf4j-api"] { + assert!(stdout.contains(artifact), "{artifact} is missing: {stdout}"); + } + + // And `check` classifies the two as undetermined rather than omitting them. + let doc = check_json(&dir, &[]); + assert_eq!(doc["summary"]["total"], 3, "{}", doc["summary"]); + for name in ["${project.groupId}:app-core", "${project.groupId}:app-web"] { + assert_eq!(status_of(&doc, name)["status"], "UNDETERMINED", "{name}"); + } + assert_eq!(doc["summary"]["undetermined"], 2, "{}", doc["summary"]); +} From f6c72d6780ec1faaaf77d8e6a4715a751c0f0e67 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 02:21:23 -0400 Subject: [PATCH 16/25] fix(jvm): keep a system-scoped jar local when its version is reconstructed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comment, a CDATA section, or a character reference inside `` costs the entry its byte-faithful span, and the span-less path rebuilt the item with `PackageSource::Inherited` hardcoded — discarding the `Local` that `system` had already established. `Inherited` is checkable, so the run went and asked Maven Central about a jar sitting at a path on this machine, and reported `ERROR` for it: a false statement of exactly the kind this branch set out to remove, pointed the other way. Two dependencies identical but for a comment inside the version, both `system`, disagreed about their own status and about whether `--fail-on any` passed. The entry's own source now survives: only a registry entry becomes `Inherited`, which is the one case the variant describes. --- crates/dependable-core/src/parsers/pom_xml.rs | 32 ++++++++++++- crates/dependable/tests/fixture_maven.rs | 45 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index f0d64c2..7e66d56 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -497,11 +497,20 @@ fn item(entry: &Declared, version: &Located, starts: &[usize]) -> Item { /// An item whose version is known but whose line is not this dependency's to /// rewrite — a `` entry several dependencies share. +/// +/// The entry's own source survives when it says something +/// [`Inherited`](PackageSource::Inherited) would contradict: a `system` +/// jar is [`Local`](PackageSource::Local) — there is no registry that has heard of +/// it — whether or not its version happened to need reconstructing. Only a registry +/// entry becomes `Inherited`, which is the case the variant describes. fn inherited(entry: &Declared, version: &str) -> Item { Item { name: entry.name.clone(), version_constraint: version.to_owned(), - source: PackageSource::Inherited, + source: match entry.source { + PackageSource::Registry => PackageSource::Inherited, + other => other, + }, version_line: 0, version_col_start: 0, version_col_end: 0, @@ -862,6 +871,27 @@ mod tests { assert!(!m.items[0].is_checkable()); } + /// A `system` jar is a file on this machine whatever shape its version takes. + /// Reconstructing the version — here around a comment — must not relabel the + /// entry as one a registry could answer for: `Inherited` is checkable, and the + /// run would go and ask crates of a jar no registry has ever published. + #[test] + fn a_system_scoped_jar_stays_local_when_its_version_is_reconstructed() { + let content = pom(" \n\ + \x20 \n\ + \x20 g\n\ + \x20 cmtsys\n\ + \x20 1.0.0\n\ + \x20 system\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let item = find(&m, "g:cmtsys"); + assert_eq!(item.version_constraint, "1.0.0"); + assert_eq!(item.source, PackageSource::Local); + assert!(!item.is_checkable(), "a system jar has no registry to ask"); + } + /// Whitespace around a version is formatting, not part of it, and the span has /// to exclude it or `--fix` would rewrite the indentation too. #[test] diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs index 4e3c10d..20525e5 100644 --- a/crates/dependable/tests/fixture_maven.rs +++ b/crates/dependable/tests/fixture_maven.rs @@ -517,3 +517,48 @@ fn a_group_this_file_cannot_resolve_is_still_listed() { } assert_eq!(doc["summary"]["undetermined"], 2, "{}", doc["summary"]); } + +/// The `system` scope survives a version this parser had to reconstruct. +/// +/// A comment inside `` costs the entry its byte-faithful span, and the +/// span-less path used to overwrite the source with `Inherited` — which is +/// checkable, so a jar with no registry at all was fetched from one and reported +/// `ERROR`, failing `--fail-on any`. Same POM, same scope, two answers. +#[test] +fn a_system_scoped_jar_stays_local_when_its_version_is_reconstructed() { + let dir = pom_dir( + "maven_system_scope_reconstructed", + " \n \ + \n \ + org.example\n \ + plainsys\n \ + 1.0.0\n \ + system\n \ + /opt/plain.jar\n \ + \n \ + \n \ + org.example\n \ + cmtsys\n \ + 1.0.0\n \ + system\n \ + /opt/cmt.jar\n \ + \n \ + \n", + ); + + let doc = check_json(&dir, &[]); + for artifact in ["plainsys", "cmtsys"] { + let result = status_of(&doc, &format!("org.example:{artifact}")); + assert_eq!( + result["status"], "LOCAL", + "a system jar has no registry however its version is spelled: {result}" + ); + } + + let output = run(&dir, &["check", ".", "--fail-on", "any"]); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); +} From 43e7c1279fe00c5fa9ad0febfa980339cdb62908 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 02:21:30 -0400 Subject: [PATCH 17/25] fix(jvm): refuse a version spliced back together across lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concatenating every text node is what reads `1.0.0` as the `1.0.0` Maven reads, and only the ends of the join were trimmed. Broken over lines the same construct keeps the pretty-printer's indentation between its halves: 1.0 .0 which joined to `1.0\n \n .0` — a value nobody wrote, that Maven would reject, and that `is_checkable` nonetheless waved through to the fetch layer as a real constraint. Stitching the halves together would state a version the file does not, so a joined value with whitespace inside it now states nothing at all: the dependency is reported unresolved, which is what it is. The single-line form is untouched and still reads whole. --- crates/dependable-core/src/parsers/pom_xml.rs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 7e66d56..78ded79 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -451,6 +451,11 @@ fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option) -> Option { let mut texts = node.children().filter(roxmltree::Node::is_text); let first = texts.next()?; @@ -465,6 +470,15 @@ fn text_of(node: roxmltree::Node<'_, '_>) -> Option { if value.is_empty() { return None; } + // Concatenating across a comment splices whatever a pretty-printer put around + // it into the middle of the value: a `` broken over three lines joins + // to `1.0\n \n .0`, which is no version anyone wrote and which + // Maven itself would reject. Trimming the ends does not reach it, and stitching + // the halves together would state a version the file does not. Refusing leaves + // the dependency unresolved, which is what it is. + if !single && value.chars().any(char::is_whitespace) { + return None; + } let range = first.range(); let faithful = single && range.len() == raw.len(); let start = range.start + (raw.len() - raw.trim_start().len()); @@ -892,6 +906,29 @@ mod tests { assert!(!item.is_checkable(), "a system jar has no registry to ask"); } + /// The same trick spread over lines splices the pretty-printer's indentation + /// into the middle of the version. `1.0\n \n .0` is no version + /// anyone wrote; stating it as the constraint would send it to the registry as + /// if it were. + #[test] + fn a_version_split_across_lines_states_no_version() { + let content = pom(" \n\ + \x20 \n\ + \x20 g\n\ + \x20 a\n\ + \x20 \n\ + \x20 1.0\n\ + \x20 \n\ + \x20 .0\n\ + \x20 \n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let item = find(&m, "g:a"); + assert_eq!(item.version_constraint, ""); + assert!(!item.is_checkable()); + } + /// Whitespace around a version is formatting, not part of it, and the span has /// to exclude it or `--fix` would rewrite the indentation too. #[test] From a84ac1cd75ec8a7f1a10036aae89ac6ee00eb5d3 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 02:22:22 -0400 Subject: [PATCH 18/25] fix(fetch): say why a detached package's inherited version was never read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reporting an unread version as `Undetermined` rather than `Local` changed what `--fail-on any` does to a Cargo package that declares `serde = { workspace = true }` with no workspace root above it: `Local` is on that flag's clean list, `Undetermined` is not, so an existing user's job flipped green to red. With nothing on stderr, because `undeclared_inheritance` only runs once a root has been found and `deferred_versions` speaks only for POMs. The status is right — the crate is on crates.io and this run read no version for it — but it broke the promise `result.rs` makes about itself: that a run says why alongside one. So the walk running out is now reported the way finding the wrong root already was, with every such entry named once on stderr. Cargo only, since `workspace = true` is the only spelling that promises a root there might be none of; a POM deferring to its `` has none to be missing, and stays `deferred_versions`' story to tell. --- crates/dependable-fetch/src/check.rs | 34 ++++++++++++++++ crates/dependable/tests/cli_workspace.rs | 52 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 44d4c76..0191caa 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -570,6 +570,8 @@ impl Checker { // The resolved names are the caller's business; the annotated items are ours. let _ = resolve_workspace_inheritance(&mut parsed.items, declarations); warnings.extend(undeclared_inheritance(&parsed.items, root)); + } else { + warnings.extend(detached_inheritance(&parsed.items, kind)); } // Apply the lockfile to annotate locked versions, dispatching on the file @@ -794,6 +796,38 @@ fn undeclared_inheritance(items: &[Item], root: &Path) -> Vec { .collect() } +/// Name every entry that says it inherits when no governing root was found at all. +/// +/// The sibling of [`undeclared_inheritance`], for the case where the walk upwards ran +/// out before it found a root rather than finding one that declares the wrong things. +/// A detached member — a `Cargo.toml` with `serde = { workspace = true }` and nothing +/// above it — reports every such entry as [`DependencyStatus::Undetermined`], which +/// `--fail-on any` treats as a failure; without this the run exits non-zero with +/// nothing on stderr to say why. +/// +/// Only Cargo, because `workspace = true` is the only spelling that promises a root: +/// a POM deferring to its `` has no root to be missing and is +/// [`deferred_versions`]' story to tell. +fn detached_inheritance(items: &[Item], kind: ManifestKind) -> Vec { + if kind != ManifestKind::CargoToml { + return Vec::new(); + } + let mut seen = HashSet::new(); + items + .iter() + .filter(|item| { + item.source == PackageSource::Inherited && item.version_constraint.is_empty() + }) + .filter(|item| seen.insert(item.name.as_str())) + .map(|item| { + format!( + "`{}` is declared `workspace = true`, but no workspace root was found above this manifest, so no version was read for it and nothing was checked", + item.name, + ) + }) + .collect() +} + /// Say, once per manifest, that some of its entries state no version this file can /// resolve — and therefore that nothing was checked for them. /// diff --git a/crates/dependable/tests/cli_workspace.rs b/crates/dependable/tests/cli_workspace.rs index 5495633..76650f2 100644 --- a/crates/dependable/tests/cli_workspace.rs +++ b/crates/dependable/tests/cli_workspace.rs @@ -220,3 +220,55 @@ fn a_constraint_the_root_never_declared_is_attributed_to_nobody() { "the root declares no tokio: {tokio}" ); } + +/// A package that inherits from a workspace root there is none of. +/// +/// The status is `UNDETERMINED`, which `--fail-on any` fails the run for — so the +/// exit code alone tells a reader nothing, and stderr saying nothing at all leaves a +/// CI job red with no explanation anywhere. Reporting an unread version as +/// undetermined rather than local is the right call; doing it silently is not. +#[test] +fn a_package_inheriting_from_no_workspace_at_all_says_so() { + let dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("workspace_detached_member"); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(dir.join(".git")).unwrap(); + // A package, not a workspace: nothing above it declares `serde`, and the `.git` + // marker stops the walk before it reaches this repository's own root. + fs::write( + dir.join("Cargo.toml"), + "[package]\nname = \"detached\"\nversion = \"0.1.0\"\n\n[dependencies]\nserde.workspace = true\n", + ) + .unwrap(); + fs::write(dir.join("dependable.toml"), CONFIG).unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_dependable")) + .args([ + "check", + dir.to_str().unwrap(), + "--config", + dir.join("dependable.toml").to_str().unwrap(), + "--fail-on", + "any", + "--format", + "json", + "--no-vuln", + "--no-cache", + ]) + .output() + .expect("run dependable"); + + let doc: Value = serde_json::from_slice(&output.stdout).expect("valid JSON"); + let serde = result(&doc, "Cargo.toml", "serde"); + assert_eq!(serde["status"], "UNDETERMINED", "{serde}"); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + output.status.code(), + Some(1), + "`--fail-on any` fails on an undetermined dependency" + ); + assert!( + stderr.contains("serde") && stderr.contains("no workspace root was found"), + "an undetermined dependency that fails the run has to say why: {stderr:?}" + ); +} From 50922c492295d9346db1392f2983e368a4bb3d1d Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 02:22:22 -0400 Subject: [PATCH 19/25] fix(report): count an undetermined dependency in the HTML status table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `§1`'s status table listed eight statuses and `Undetermined` was not among them, so an executive summary reading "6 dependencies" sat over rows summing to 5 — and a POM whose only dependency defers to its `` reported one dependency over a table accounting for none of it. `Summary` has carried the count since the status was added; `SummaryView` never passed it to the template, and an undeclared template variable renders as the empty string rather than failing, so the row would have been blank even once added. §3 and the ecosystem table were right all along — it is the summary above them that did not add up. No golden held an `Undetermined` result, which is why three byte-for-byte fixtures did not notice. The full report now carries one, and the test asserts the rendered row carries its count as well as its heading, plus that the rows account for every dependency the summary totals. --- crates/dependable-report/src/html/model.rs | 2 + .../src/html/templates/summary.html | 1 + .../dependable-report/tests/golden/empty.html | 1 + .../dependable-report/tests/golden/full.html | 43 ++++++++++++------- .../tests/golden/single_ecosystem.html | 1 + crates/dependable-report/tests/html_golden.rs | 40 ++++++++++++++++- 6 files changed, 71 insertions(+), 17 deletions(-) diff --git a/crates/dependable-report/src/html/model.rs b/crates/dependable-report/src/html/model.rs index c503d61..e840fa3 100644 --- a/crates/dependable-report/src/html/model.rs +++ b/crates/dependable-report/src/html/model.rs @@ -191,6 +191,7 @@ pub(crate) struct SummaryView { pub error: usize, pub local: usize, pub git: usize, + pub undetermined: usize, pub advisory_instances: usize, pub distinct_advisories: usize, pub withdrawn_advisories: usize, @@ -381,6 +382,7 @@ fn summary_view(summary: &Summary) -> SummaryView { error: summary.error, local: summary.local, git: summary.git, + undetermined: summary.undetermined, advisory_instances: summary.advisory_instances, distinct_advisories: summary.distinct_advisories, withdrawn_advisories: summary.withdrawn_advisories, diff --git a/crates/dependable-report/src/html/templates/summary.html b/crates/dependable-report/src/html/templates/summary.html index e104067..69c6970 100644 --- a/crates/dependable-report/src/html/templates/summary.html +++ b/crates/dependable-report/src/html/templates/summary.html @@ -39,6 +39,7 @@

1. Executive summary

Errored{{ summary.error }} Path (local){{ summary.local }} Git{{ summary.git }} +Undetermined{{ summary.undetermined }}

diff --git a/crates/dependable-report/tests/golden/empty.html b/crates/dependable-report/tests/golden/empty.html index 8fcab68..ef7ac3a 100644 --- a/crates/dependable-report/tests/golden/empty.html +++ b/crates/dependable-report/tests/golden/empty.html @@ -185,6 +185,7 @@

1. Executive summary

Errored0 Path (local)0 Git0 +Undetermined0

diff --git a/crates/dependable-report/tests/golden/full.html b/crates/dependable-report/tests/golden/full.html index 9539e6b..2b542d3 100644 --- a/crates/dependable-report/tests/golden/full.html +++ b/crates/dependable-report/tests/golden/full.html @@ -164,7 +164,7 @@

1. Executive summary

    -
  • 7dependencies
    • +
  • 8dependencies
  • 2manifests
  • 16.7%of 6 checkable up to date
  • 2vulnerable
    • @@ -191,6 +191,7 @@

    1. Executive summary

    Errored1 Path (local)1 Git0 +Undetermined1

    @@ -284,7 +285,7 @@

    2. Vulnerability detail

    3. Dependency status

    -Cargo.toml — Rust (5 dependencies) +Cargo.toml — Rust (6 dependencies) @@ -349,6 +350,16 @@

    3. Dependency status

    502 Bad Gateway from the index
    + + + + + + + +
    helper—— +undetermined +
    @@ -453,26 +464,26 @@

    5. Ecosystem breakdown

    Dependencies by ecosystem -A pie chart of 7 dependencies split across 2 ecosystem(s). The same figures are in the table beneath it. - - +A pie chart of 8 dependencies split across 2 ecosystem(s). The same figures are in the table beneath it. + +
    • Rust -5 (71.4%) - -up to date: 1 -outdated: 1 -vulnerable: 1 -not checkable: 2 +6 (75.0%) + +up to date: 1 +outdated: 1 +vulnerable: 1 +not checkable: 3
    • npm -2 (28.6%) +2 (25.0%) outdated: 1 vulnerable: 1 @@ -496,17 +507,17 @@

      5. Ecosystem breakdown

      Rust -5 -71.4% +6 +75.0% 1 1 1 -2 +3 npm 2 -28.6% +25.0% 0 1 1 diff --git a/crates/dependable-report/tests/golden/single_ecosystem.html b/crates/dependable-report/tests/golden/single_ecosystem.html index 5f1f23f..e103c63 100644 --- a/crates/dependable-report/tests/golden/single_ecosystem.html +++ b/crates/dependable-report/tests/golden/single_ecosystem.html @@ -185,6 +185,7 @@

      1. Executive summary

      Errored0 Path (local)0 Git0 +Undetermined0

      diff --git a/crates/dependable-report/tests/html_golden.rs b/crates/dependable-report/tests/html_golden.rs index 32a35ce..0d068b5 100644 --- a/crates/dependable-report/tests/html_golden.rs +++ b/crates/dependable-report/tests/html_golden.rs @@ -84,7 +84,8 @@ fn full_report() -> Report { serde = \"1.0.228\"\n\ regex = \"1.5\"\n\ dependable-core = { path = \"../dependable-core\" }\n\ - brokenpkg = \"2\"\n", + brokenpkg = \"2\"\n\ + helper = { workspace = true }\n", ); let mut rust_results: Vec = Vec::new(); @@ -159,6 +160,13 @@ fn full_report() -> Report { rust[4].clone(), DependencyStatus::Error("502 Bad Gateway from the index".to_owned()), )); + // An entry whose version was never read: on a registry, but nothing was fetched + // for it. It is in `summary.total`, so a status table that omits it accounts for + // fewer dependencies than the run reports. + rust_results.push(CheckResult::new( + rust[5].clone(), + DependencyStatus::Undetermined, + )); let npm = items( ManifestKind::PackageJson, @@ -215,6 +223,36 @@ fn full_report_matches_the_golden() { // Sanity checks the golden alone would not make obvious. assert!(html.contains(" 0, + "the fixture exercises the new row" + ); + // The count, not just the label: an undeclared template variable renders as the + // empty string rather than failing, so a row that reads nothing still "contains" + // its own heading. + assert!( + html.contains(&format!( + "Undetermined{}", + summary.undetermined + )), + "{html}" + ); assert_golden("full", &html); } From 8ddd1b1e68117df2915cc650bda013a9ccb97f25 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:02 -0400 Subject: [PATCH 20/25] docs(cli): keep the inheritance ordering rationale with the code it explains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paragraph explaining why workspace inheritance must be resolved before the lockfile — `pick_locked` chooses among several locked versions of one crate *by the declared constraint*, so resolving second hands it an empty one — had a notice loop inserted between it and the `workspace_source` / `resolve_workspace_inheritance` call it describes. Move the loop above it, each comment beside the statement it is about, so moving one later cannot carry the other's rationale away. --- crates/dependable/src/runner.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 6b66186..736e92c 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -622,14 +622,6 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { } }; - // A member writing `dep.workspace = true` states no version of its own; the - // constraint lives in the workspace root. Same resolution `check` and `fix` get, - // so an inventory and a check never disagree about what a member depends on. - // - // Before the lockfile, and in that order for a reason: a lockfile can hold several - // versions of one crate, and `pick_locked` chooses among them *by the declared - // constraint*. Resolving second would hand it an empty constraint and pick the - // highest — reporting `syn 2.0` locked against a member that inherits `syn = "1"`. // What the parser saw and declined to read. On stderr rather than in the // listing, so the same words reach a reader whichever `--format` they // chose, and no machine-readable document changes shape — the same place @@ -638,6 +630,14 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { eprintln!("warning: {} — {notice}", manifest.display()); } + // A member writing `dep.workspace = true` states no version of its own; the + // constraint lives in the workspace root. Same resolution `check` and `fix` get, + // so an inventory and a check never disagree about what a member depends on. + // + // Before the lockfile, and in that order for a reason: a lockfile can hold several + // versions of one crate, and `pick_locked` chooses among them *by the declared + // constraint*. Resolving second would hand it an empty constraint and pick the + // highest — reporting `syn 2.0` locked against a member that inherits `syn = "1"`. let inherited = workspace_source(manifest, kind, &content) .map(|(_, declarations)| { resolve_workspace_inheritance(&mut parsed.items, &declarations) From 13575bf7544d61f4f276b3d43462b69779da3d2f Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:08 -0400 Subject: [PATCH 21/25] fix(cli): decline to rewrite a Maven single-version interval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[1.0]` is Maven's only way to state a *hard* requirement — the one form that defeats nearest-wins mediation. Every existing decline missed it: no comma, no space or pipe after the empty operator prefix, no `@`, it starts with `[` rather than a letter, and `is_wildcard` splits it into `["[1", "0]"]`, neither of which is `x`/`X` nor starts with `*`/`+`. `--fix` therefore rewrote it to a bare `1.0`, silently reverting it to a soft requirement any transitive declaration may outvote, and recorded the change as "from [1.0] to 1.0", which does not read as a semantics change. Decline any constraint whose remainder opens with `[` or `(`, alongside the dist-tag and wildcard declines it belongs with. --- crates/dependable/src/fix.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/dependable/src/fix.rs b/crates/dependable/src/fix.rs index ddc0c5f..05ad50a 100644 --- a/crates/dependable/src/fix.rs +++ b/crates/dependable/src/fix.rs @@ -120,7 +120,7 @@ fn plan_fixes(content: &str, results: &[CheckResult], all: bool) -> (String, Vec /// (npm/pubspec `>=1.0.0 <2.0.0`), a `||` alternation (`^1 || ^2`), a dist-tag /// (`latest`), a wildcard (`*`, `1.x`, `1.*`), or anything carrying an `@` /// (a Composer stability flag such as `@dev` or `^1.0@beta`, an npm alias such -/// as `npm:pkg@1.0.0`). +/// as `npm:pkg@1.0.0`), or a Maven interval (`[1.0]`, `(,2.0)`). fn rewrite_constraint(original: &str, new_version: &str) -> Option { let trimmed = original.trim(); if trimmed.contains(',') { @@ -153,6 +153,16 @@ fn rewrite_constraint(original: &str, new_version: &str) -> Option { if rest.starts_with(|c: char| c.is_ascii_alphabetic()) { return None; } + // A Maven interval — `[1.0]`, `[1.0,2.0)`, `(,1.0]` — states its bounds in + // brackets rather than with an operator, and `[1.0]` in particular is Maven's + // one way to say "exactly this, and defeat nearest-wins mediation". Rewriting it + // to a bare `1.0` reverts it to a *soft* requirement any transitive declaration + // may outvote, and the fix record reads "from [1.0] to 1.0", which does not read + // as the semantics change it is. A range with a comma is already declined above; + // the single-version form has no comma to catch it. + if rest.starts_with(['[', '(']) { + return None; + } // A wildcard (`*`, `1.x`, `1.*`, Gradle's `1.+`) is a range the author chose, // not a version — substituting a concrete release narrows it to a pin (#87). // The decline is deliberately blanket rather than per-ecosystem: substituting @@ -282,6 +292,25 @@ mod tests { assert_eq!(rewrite_constraint("*", "2.0.0"), None); } + /// A Maven interval states its bounds in brackets, and `[1.0]` is Maven's only + /// way to say "exactly 1.0, and do not let nearest-wins mediation substitute + /// anything else". Rewriting it to a bare `1.0` turns a hard requirement into a + /// soft one that any transitive declaration may outvote, and the fix record reads + /// "from [1.0] to 1.0" — a semantics change that does not look like one. Every + /// other guard misses it: no comma, no space or `|` after the empty operator + /// prefix, no `@`, it starts with `[` rather than a letter, and `is_wildcard` + /// splits it into `["[1", "0]"]`, neither of which is `x`/`X` nor starts with + /// `*`/`+`. + #[test] + fn rewrite_never_softens_a_maven_interval() { + assert_eq!(rewrite_constraint("[1.0]", "2.0.0"), None); + assert_eq!(rewrite_constraint("[1.0,2.0)", "2.0.0"), None); + assert_eq!(rewrite_constraint("(,1.0]", "2.0.0"), None); + assert_eq!(rewrite_constraint("[1.0,)", "2.0.0"), None); + // Whitespace before the bracket is still a bracket. + assert_eq!(rewrite_constraint(" [1.0] ", "2.0.0"), None); + } + /// A wildcard segment is not always the whole dot-segment. Composer allows a /// stability flag after the constraint, so `"symfony/symfony": "2.8.*@dev"` /// splits into `["2", "8", "*@dev"]` — no segment *equals* a wildcard, and a From 94f7cd17e6bcee72ef216be12f60657b5b0a6f81 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:13 -0400 Subject: [PATCH 22/25] fix(report): withhold the up-to-date share when a version went unread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An undetermined dependency is rightly outside the denominator: a version this run never read is no evidence that anything is current. But that alone let a report over one up-to-date and one undetermined dependency render "100% up to date" — a stronger claim than a run that read half the manifest can make, and the one figure a reader takes away. Return `None` while anything is undetermined, so the renderer prints its n/a marker instead. The status counts beside it still say exactly what was and was not read. --- crates/dependable-report/src/summary.rs | 24 ++++++++++++++++--- .../dependable-report/tests/golden/full.html | 2 +- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/dependable-report/src/summary.rs b/crates/dependable-report/src/summary.rs index 18d1df6..acd86b0 100644 --- a/crates/dependable-report/src/summary.rs +++ b/crates/dependable-report/src/summary.rs @@ -165,9 +165,17 @@ impl Summary { /// /// `None` when nothing is checkable, so a renderer can print "n/a" rather /// than dividing by zero and emitting `NaN%`. + /// + /// `None` too when anything went [`Undetermined`](DependencyStatus::Undetermined). + /// Such a dependency is rightly outside the denominator — an unread version is no + /// evidence of currency — but a headline "100% up to date" over a run that read + /// half the manifest is a stronger claim than the run can make, and it is the one + /// figure a reader takes away. A run that could not read everything has no single + /// number for how current it is; the status counts beside it still say exactly + /// what was and was not read. #[must_use] pub fn up_to_date_percent(&self) -> Option { - if self.checkable == 0 { + if self.checkable == 0 || self.undetermined > 0 { return None; } #[allow(clippy::cast_precision_loss)] @@ -333,7 +341,12 @@ mod tests { assert_eq!(summary.checkable, 6); } - /// A run that read nothing is not a run that is 100% up to date. + /// A run that read half the manifest is not a run that is 100% up to date. + /// + /// The undetermined dependency stays out of the denominator — an unread version + /// is not evidence of currency — and the percentage is withheld rather than + /// rendered, because "100%" of a run that never read one of two dependencies is + /// the wrong headline for the one figure a reader remembers. #[test] fn an_undetermined_dependency_is_outside_the_up_to_date_denominator() { let report = report(vec![ManifestResults::new( @@ -348,7 +361,12 @@ mod tests { let summary = report.summary(); assert_eq!(summary.checkable, 1); - assert_eq!(summary.up_to_date_percent(), Some(100.0)); + assert_eq!(summary.undetermined, 1); + assert_eq!( + summary.up_to_date_percent(), + None, + "no single number describes a run that could not read everything" + ); } #[test] diff --git a/crates/dependable-report/tests/golden/full.html b/crates/dependable-report/tests/golden/full.html index 2b542d3..b7c5314 100644 --- a/crates/dependable-report/tests/golden/full.html +++ b/crates/dependable-report/tests/golden/full.html @@ -166,7 +166,7 @@

      1. Executive summary

      • 8dependencies
      • 2manifests
        • -
      • 16.7%of 6 checkable up to date
        • +
      • —of 6 checkable up to date
      • 2vulnerable
      • 1critical advisories
      • 9.8highest CVSS
        • From c06007bbfccefa17ead80f42f724175d2d107a88 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:20 -0400 Subject: [PATCH 23/25] fix(fetch): tell an unsearched workspace apart from an absent one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check_inner` inferred "the upward walk ran and found nothing" from `workspace == None`, but `check_manifest` passes `None` unconditionally — it takes content with no file behind it and never walks anything. So the public content-only API asserted the result of a search that never happened: an IDE checking an open `crates/foo/Cargo.toml` buffer whose root one directory up declares `serde` was told `serde` is declared `workspace = true`, but no workspace root was found above this manifest, which is false of the file on disk and contradicts what `check_path` says about it. Make the distinction a value rather than an absence. `WorkspaceContext` has three states — `Unsearched`, `NotFound`, `Found` — and only `NotFound` reaches `detached_inheritance`. A content-only check still reports the entry `Undetermined`; what it no longer does is explain that with a search it never ran. --- crates/dependable-fetch/src/check.rs | 71 +++++++++++++++++++----- crates/dependable-fetch/tests/checker.rs | 13 +++++ 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index b63bd12..7b223d3 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -189,6 +189,36 @@ struct FetchTask { cache_key: String, } +/// What the walk for a governing workspace root found — or that no walk ran. +/// +/// `Option` could not say both, and the difference is load-bearing: +/// [`Checker::check_manifest`] takes content with no file behind it and therefore +/// never walks anything, while [`Checker::check_path`] walks and may legitimately +/// come back empty. [`detached_inheritance`] asserts the second — "no root exists +/// above this file" — which is false of the first, whose caller may be an IDE +/// holding an open buffer of a member deep inside an ordinary workspace. +enum WorkspaceContext { + /// No search was performed: there is no path to walk up from. Says nothing at + /// all about whether a root exists on disk. + Unsearched, + /// The walk ran and reached the top of the tree without finding a root. + NotFound, + /// The walk found this root, and these are the dependencies it declares. + Found(PathBuf, Arc>), +} + +impl WorkspaceContext { + /// The root the walk found, for [`ManifestCheck::workspace_root`] — `None` + /// both when the walk found nothing and when no walk ran, because the field + /// reports a located root and neither case located one. + fn into_root(self) -> Option { + match self { + Self::Found(root, _) => Some(root), + Self::Unsearched | Self::NotFound => None, + } + } +} + /// The result of one fetch task: `(name, cache_key, versions-or-error)`. type FetchOutcome = (String, String, Result, String>); @@ -230,10 +260,12 @@ impl Checker { // Content with no file behind it: the manifest's first lockfile is the // only thing it can be attributed to. let lockfile = lockfile.and_then(|lock| Some((*kind.lockfiles().first()?, lock))); - // No file, so no tree above it: a `dep.workspace = true` here stays unresolved - // and reports as it always has. [`Checker::check_path`] is the entry point that - // can answer the question. - self.check_inner(kind, manifest, lockfile, None).await + // No file, so nothing to walk upwards from: a `dep.workspace = true` here stays + // unresolved and reports as it always has. `Unsearched` and not `NotFound` — + // this call never looked, so it must not say what a look would have found. + // [`Checker::check_path`] is the entry point that can answer the question. + self.check_inner(kind, manifest, lockfile, WorkspaceContext::Unsearched) + .await } /// Check a manifest on disk: detect its kind, read it (and, when @@ -494,7 +526,12 @@ impl Checker { // A member's `dep.workspace = true` states no version of its own; the constraint // is in the root above it. Only a path can find that root, which is why this // resolves here and not in `check_manifest`. - let workspace = self.workspace_source(path, kind, &manifest).await; + let workspace = match self.workspace_source(path, kind, &manifest).await { + Some((root, declarations)) => WorkspaceContext::Found(root, declarations), + // The walk ran and reached the top with nothing: that is a fact about the + // tree, and the one `detached_inheritance` is allowed to report. + None => WorkspaceContext::NotFound, + }; self.check_inner(kind, &manifest, lockfile, workspace).await } @@ -548,7 +585,7 @@ impl Checker { kind: ManifestKind, manifest: &str, lockfile: Option<(LockfileKind, &str)>, - workspace: Option<(PathBuf, Arc>)>, + workspace: WorkspaceContext, ) -> Result { let ecosystem = kind.ecosystem(); let fetcher = self @@ -566,12 +603,20 @@ impl Checker { // nothing in this file. let mut warnings = Vec::new(); warnings.extend(std::mem::take(&mut parsed.notices)); - if let Some((root, declarations)) = &workspace { - // The resolved names are the caller's business; the annotated items are ours. - let _ = resolve_workspace_inheritance(&mut parsed.items, declarations); - warnings.extend(undeclared_inheritance(&parsed.items, root)); - } else { - warnings.extend(detached_inheritance(&parsed.items, kind)); + match &workspace { + WorkspaceContext::Found(root, declarations) => { + // The resolved names are the caller's business; the annotated items are ours. + let _ = resolve_workspace_inheritance(&mut parsed.items, declarations); + warnings.extend(undeclared_inheritance(&parsed.items, root)); + } + WorkspaceContext::NotFound => { + warnings.extend(detached_inheritance(&parsed.items, kind)); + } + // Nothing was walked, so nothing is known about what sits above this + // content — least of all that there is nothing. The entry still reports + // `Undetermined`; what it does not do is explain that with a search that + // never ran. + WorkspaceContext::Unsearched => {} } // Apply the lockfile to annotate locked versions, dispatching on the file @@ -635,7 +680,7 @@ impl Checker { ecosystem, results, warnings, - workspace_root: workspace.map(|(root, _)| root), + workspace_root: workspace.into_root(), }; // Enrichment is a post-pass over the finished results, so it can equally diff --git a/crates/dependable-fetch/tests/checker.rs b/crates/dependable-fetch/tests/checker.rs index be18ec4..baaba07 100644 --- a/crates/dependable-fetch/tests/checker.rs +++ b/crates/dependable-fetch/tests/checker.rs @@ -1053,6 +1053,19 @@ async fn a_member_is_checked_against_the_workspace_roots_constraint() { ); assert!(serde.item.version_constraint.is_empty()); assert!(detached.workspace_root.is_none()); + // And it says nothing about a root, because it never went looking for one. The + // same buffer on disk (above) resolves against a root one directory up, so a + // content-only check claiming "no workspace root was found above this manifest" + // would contradict `check_path` about the very same file — the shape an IDE + // checking an open buffer hits every keystroke. + assert!( + detached + .warnings + .iter() + .all(|w| !w.contains("no workspace root was found")), + "no search ran, so nothing may be reported about what one would have found: {:?}", + detached.warnings + ); } /// A root declaring a crate by `path` lends the member a path dependency, not a registry From a6a55ceed388a6670a27c39a6dae6bbc82b86aae Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:30 -0400 Subject: [PATCH 24/25] fix(core): read a POM version independently of its coordinate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When either coordinate half was unresolvable the version was forced to `Unknown` without `version_source` ever being called, so the standard multi-module idiom ${project.groupId} app-core 2.0.13 lost its `2.0.13` entirely. `list` printed the entry with no version, and the check then explained the blank with its `` / `` / undeclared-property notice — false twice over, because the version *was* stated and no parent is involved. Under `--fail-on any` the run exited 1 pointing at a parent POM that does not exist. Read the two independently. A coordinate this file cannot spell now has its own source, `PackageSource::Unidentified`: nothing is fetched under a name no registry has heard of, a check reports it `Undetermined` — not `Local`, which would claim the package has no registry rather than that this file never said which package it is — and the parser emits its own notice naming it, so the version story is left to the entries it is actually about. --- crates/dependable-core/src/item.rs | 22 +++ crates/dependable-core/src/parsers/pom_xml.rs | 176 +++++++++++++++--- crates/dependable-fetch/src/check.rs | 6 + crates/dependable/src/output/list.rs | 4 + crates/dependable/tests/fixture_maven.rs | 65 +++++++ 5 files changed, 243 insertions(+), 30 deletions(-) diff --git a/crates/dependable-core/src/item.rs b/crates/dependable-core/src/item.rs index 5bcf9ef..4f3bbd5 100644 --- a/crates/dependable-core/src/item.rs +++ b/crates/dependable-core/src/item.rs @@ -187,6 +187,28 @@ pub enum PackageSource { /// all, and a check reports /// [`DependencyStatus::Undetermined`](crate::result::DependencyStatus::Undetermined). Inherited, + /// The entry names a package this manifest cannot identify, whatever version it + /// states beside it. + /// + /// A Maven POM is the case that needs it: `${project.groupId}` + /// names a built-in this file does not state, and a `` may omit + /// `` altogether and inherit it from a ``. Either way the + /// coordinate is not a name any registry could answer for, so the entry is never + /// fetched — and a check reports it + /// [`Undetermined`](crate::result::DependencyStatus::Undetermined), because what + /// went unread is which package this is. + /// + /// Distinct from [`Inherited`](Self::Inherited), whose name *is* known and whose + /// missing half is the version: the two get different explanations, and an entry + /// whose version is stated right there in the file must never be told it takes + /// that version from somewhere else. Distinct from [`Local`](Self::Local), which + /// asserts there is no registry behind the package rather than that this file + /// could not say which package it is. + /// + /// The version, when the entry states one, is still recorded in + /// [`version_constraint`](Item::version_constraint) — it is written in this file, + /// and dropping it would report a manifest as stating less than it does. + Unidentified, } #[cfg(test)] diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index 78ded79..c4aa6b3 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -45,10 +45,15 @@ //! The same holds for a coordinate: `${project.groupId}` — the //! standard idiom for a sibling module in a multi-module build — names a built-in //! this file does not state, and the dependency is reported under that literal -//! rather than dropped. +//! rather than dropped. Such an entry is +//! [`PackageSource::Unidentified`]: nothing is fetched under a name this file never +//! spelled. Its `` is read and reported all the same, because the two are +//! independent — a POM stating `2.0.13` beside +//! `${project.groupId}` states that version whatever else it leaves to Maven, and +//! blanking it would then have to be explained by a `` that is not involved. //! -//! A dependency whose version or coordinate those rules leave unknown is -//! **reported anyway**, with no constraint and no span — the shape a Cargo member's unresolved +//! A dependency whose version those rules leave unknown is **reported anyway**, with +//! no constraint and no span — the shape a Cargo member's unresolved //! `dep.workspace = true` already has, which the CLI renders as `(unresolved)` and //! never fetches, fixes, or claims a status for. Dropping it instead, as the //! `csproj` parser drops an MSBuild `$(…)` version, would report a POM that @@ -132,17 +137,32 @@ impl Parser for PomXmlParser { let items = declared .iter() - .map(|entry| match &entry.version { - Source::Literal(version) => item(entry, version, &starts), - Source::Property(name) => { - let located = &properties[name.as_str()]; - if uses.get(name.as_str()).copied() == Some(1) { - item(entry, located, &starts) - } else { - inherited(entry, &located.value) - } + .map(|entry| { + // The version this entry reports, and whether the line it is written + // on belongs to this entry alone. A `` value several + // dependencies read is nobody's line to rewrite; a literal beside the + // dependency is its own. Read for every entry, including one whose + // coordinate did not resolve: a version stated in this file is + // reported whether or not anything can be fetched under the name + // beside it, because it *is* the version, and reporting nothing there + // would then be explained by a `` that has nothing to do + // with it. + let (located, sole) = match &entry.version { + Source::Literal(version) => (Some(version), true), + Source::Property(name) => ( + Some(&properties[name.as_str()]), + uses.get(name.as_str()).copied() == Some(1), + ), + Source::Unknown => (None, false), + }; + let Some(located) = located else { + return unresolved(entry); + }; + if sole { + item(entry, located, &starts) + } else { + inherited(entry, &located.value) } - Source::Unknown => unresolved(entry), }) .collect(); @@ -150,7 +170,10 @@ impl Parser for PomXmlParser { kind: ManifestKind::PomXml, items, alternate_registries: Vec::new(), - notices: profile_notice(project).into_iter().collect(), + notices: profile_notice(project) + .into_iter() + .chain(unnameable_notice(&declared)) + .collect(), }) } } @@ -213,17 +236,21 @@ fn read_dependencies<'a>( .unwrap_or_default(); out.push(Declared { name, - // A coordinate this file cannot state is a coordinate nothing can be - // fetched for, whatever version sits beside it. - version: if known { - version_source(node, properties) - } else { - Source::Unknown - }, + // Read whatever the entry states, whether or not the coordinate beside it + // resolved. The two are independent facts: forcing the version to + // `Unknown` here discarded a `2.0.13` the reader can + // see in their own file, and left its absence to be explained by + // `` inheritance that was never involved. + version: version_source(node, properties), // A `system` dependency is a jar at a path on this machine, not - // something a registry has ever heard of. - source: match scope.as_str() { - "system" => PackageSource::Local, + // something a registry has ever heard of. A coordinate this file cannot + // state is not a name any registry could answer for either — a different + // fact, and one with its own token, because `Local` would claim the + // package has no registry rather than that this file could not say which + // package it is. + source: match (scope.as_str(), known) { + ("system", _) => PackageSource::Local, + (_, false) => PackageSource::Unidentified, _ => PackageSource::Registry, }, kind: dependency_kind(node, &scope), @@ -434,6 +461,42 @@ fn profile_notice(project: roxmltree::Node<'_, '_>) -> Option { )) } +/// Say that some entries name a coordinate this file cannot state, so nothing was +/// fetched for them however clearly they state a version. +/// +/// The sibling of the `` notice above, and for the same reason: an entry +/// that reports no status reads as an oversight unless the run says why. It is kept +/// apart from the check's own "this version comes from a ``" notice because +/// the two are different failures — here the *name* went unread, and the version may +/// be stated right there in the file, so borrowing the version story would tell the +/// reader two false things at once and point them at a parent POM that does not +/// exist. +fn unnameable_notice(declared: &[Declared]) -> Option { + let mut names: Vec<&str> = declared + .iter() + .filter(|entry| entry.source == PackageSource::Unidentified) + .map(|entry| entry.name.as_str()) + .collect(); + names.sort_unstable(); + names.dedup(); + if names.is_empty() { + return None; + } + let (subject, verb) = if names.len() == 1 { + ("dependency names a coordinate", "does") + } else { + ("dependencies name coordinates", "do") + }; + Some(format!( + "{} {subject} this file {verb} not state — a `${{project.*}}` built-in, a \ + `` left to a ``, or a property declared elsewhere — so \ + nothing was fetched for {}: {}", + names.len(), + if names.len() == 1 { "it" } else { "them" }, + names.join(", ") + )) +} + /// The first direct child element named `tag`. fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option> { node.children() @@ -834,9 +897,9 @@ mod tests { /// `${project.groupId}` is *the* idiom for a sibling module in a multi-module /// build, and a POM using it for two of its three dependencies must not list as - /// depending on one. Reported under the literal, with no constraint — the shape - /// a parent-deferred version already has, which the CLI renders `(unresolved)` - /// and never fetches — rather than dropped in silence. + /// depending on one. Reported under the literal it states, as + /// [`PackageSource::Unidentified`] — never fetched, because no registry has heard + /// of a package by that name — rather than dropped in silence. #[test] fn a_group_this_file_cannot_resolve_is_reported_not_dropped() { let content = pom(" \n\ @@ -854,18 +917,66 @@ mod tests { let m = parse(&content); assert_eq!(m.items.len(), 2, "both dependencies are declared"); let sibling = find(&m, "${project.groupId}:app-core"); + // `${project.version}` is a built-in too, so this entry states no version + // either — the coordinate and the version are unread independently. assert_eq!(sibling.version_constraint, ""); - assert_eq!(sibling.source, PackageSource::Inherited); + assert_eq!(sibling.source, PackageSource::Unidentified); assert!( !sibling.is_checkable(), "no coordinate, so nothing to fetch" ); assert!(!sibling.is_rewritable()); assert_eq!(find(&m, "org.slf4j:slf4j-api").version_constraint, "2.0.13"); + assert_eq!( + m.notices.len(), + 1, + "the reader is told why one of the two reports nothing: {:?}", + m.notices + ); + assert!( + m.notices[0].contains("${project.groupId}:app-core"), + "{:?}", + m.notices + ); + } + + /// A coordinate this file cannot state and a version it can are independent + /// facts, and the standard multi-module idiom states both at once. Blanking the + /// version because the name did not resolve dropped a `2.0.13` written in plain + /// sight, and the check then explained its absence with `` inheritance + /// that is not involved. + #[test] + fn an_unnameable_coordinate_still_reports_the_version_it_states() { + let content = pom(" \n\ + \x20 \n\ + \x20 ${project.groupId}\n\ + \x20 app-core\n\ + \x20 2.0.13\n\ + \x20 \n\ + \x20 \n"); + let m = parse(&content); + let sibling = find(&m, "${project.groupId}:app-core"); + assert_eq!( + sibling.version_constraint, "2.0.13", + "the version is written in this file and is reported" + ); + assert_eq!(sibling.source, PackageSource::Unidentified); + assert!( + !sibling.is_checkable(), + "no registry has heard of a package named by an unresolved `${{…}}`" + ); + assert!(!sibling.is_rewritable()); + assert_eq!(m.notices.len(), 1, "{:?}", m.notices); + assert!( + m.notices[0].contains("app-core") && m.notices[0].contains("coordinate"), + "the unread coordinate is the story, not a version borrowed elsewhere: {:?}", + m.notices + ); } /// A `` this file never states is the same omission by a shorter - /// route: the artifact is named, so the entry is reported under what there is. + /// route: the artifact is named, so the entry is reported under what there is — + /// and so is the version beside it, which the file states outright. /// A `` naming neither half states nothing at all and is skipped. #[test] fn a_missing_coordinate_half_is_still_reported_under_the_other() { @@ -881,8 +992,13 @@ mod tests { let m = parse(&content); let names: Vec<&str> = m.items.iter().map(|i| i.name.as_str()).collect(); assert_eq!(names, vec![":orphan"]); - assert_eq!(m.items[0].version_constraint, ""); + assert_eq!( + m.items[0].version_constraint, "1.0.0", + "the version is stated here; only the coordinate is not" + ); + assert_eq!(m.items[0].source, PackageSource::Unidentified); assert!(!m.items[0].is_checkable()); + assert!(!m.items[0].is_rewritable()); } /// A `system` jar is a file on this machine whatever shape its version takes. diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 7b223d3..7bda56f 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -931,6 +931,12 @@ fn evaluate_item( // which of `spring-boot-starter-web` is simply false, and is the wrong // token for a CI consumer to read. PackageSource::Inherited => DependencyStatus::Undetermined, + // A coordinate this manifest could not state is the same shape of + // ignorance reached through the name instead of the version: nothing was + // asked, so nothing is known. `Local` would again say the wrong thing — + // that there is no registry behind the entry, rather than that this file + // never said which package it is. + PackageSource::Unidentified => DependencyStatus::Undetermined, _ => DependencyStatus::Local, }; return CheckResult::new(item.clone(), status); diff --git a/crates/dependable/src/output/list.rs b/crates/dependable/src/output/list.rs index 49d8878..e910c10 100644 --- a/crates/dependable/src/output/list.rs +++ b/crates/dependable/src/output/list.rs @@ -278,6 +278,7 @@ fn source_token(source: PackageSource) -> &'static str { PackageSource::Local => "local", PackageSource::Git => "git", PackageSource::Inherited => "inherited", + PackageSource::Unidentified => "unidentified", _ => "unknown", } } @@ -305,6 +306,9 @@ fn annotation(item: &Item) -> &'static str { // would otherwise render as a bare `—` that reads like a parse failure. A // resolved one falls through to its section, so a `dev` dep still says so. PackageSource::Inherited if item.version_constraint.is_empty() => " (unresolved)", + // The version beside it may be perfectly clear; what this file never stated is + // which package it belongs to, so nothing was fetched under that name. + PackageSource::Unidentified => " (unidentified)", _ => match item.kind { DependencyKind::Dev => " (dev)", DependencyKind::Build => " (build)", diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs index 20525e5..db7ccc6 100644 --- a/crates/dependable/tests/fixture_maven.rs +++ b/crates/dependable/tests/fixture_maven.rs @@ -518,6 +518,71 @@ fn a_group_this_file_cannot_resolve_is_still_listed() { assert_eq!(doc["summary"]["undetermined"], 2, "{}", doc["summary"]); } +/// The multi-module idiom states a version *and* a coordinate this file cannot +/// spell, and the two are read independently. +/// +/// Forcing the version to unknown because the coordinate did not resolve threw away +/// a `2.0.13` written in plain sight: `list` showed the entry with no version at +/// all, and the run then explained the blank with a `` notice — false twice, +/// because the version was stated and no parent is involved. Under `--fail-on any` +/// that pointed the reader at a parent POM that does not exist. +#[test] +fn a_version_beside_an_unnameable_coordinate_is_read_and_reported() { + let dir = pom_dir( + "maven_unnameable_with_version", + " \n \ + \n \ + ${project.groupId}\n \ + app-core\n \ + 2.0.13\n \ + \n \ + \n \ + org.springframework.boot\n \ + spring-boot-starter-web\n \ + \n \ + \n", + ); + + let listed = run(&dir, &["list", "."]); + let stdout = String::from_utf8_lossy(&listed.stdout); + assert!( + stdout.contains("2.0.13"), + "the version is written in the file and must appear: {stdout}" + ); + + let stderr = String::from_utf8_lossy(&listed.stderr); + assert!( + stderr.contains("coordinate") && stderr.contains("${project.groupId}:app-core"), + "the unread coordinate gets its own notice: {stderr}" + ); + + // `check` still declines to claim a status for it — nothing was fetched under a + // name no registry has heard of — but reports the version it read. + let doc = check_json(&dir, &[]); + let core = status_of(&doc, "${project.groupId}:app-core"); + assert_eq!(core["status"], "UNDETERMINED", "{core}"); + assert_eq!(core["current"], "2.0.13", "{core}"); + // The entry whose version really does come from the `` keeps that story, + // and keeps it to itself. + let checked = run(&dir, &["check", "."]); + let stderr = String::from_utf8_lossy(&checked.stderr); + assert!( + parent_story(&stderr).contains("spring-boot-starter-web"), + "{stderr}" + ); + assert!(!parent_story(&stderr).contains("app-core"), "{stderr}"); +} + +/// The one stderr line that blames a `` / `` / +/// undeclared property, or the empty string when the run emitted none. +fn parent_story(stderr: &str) -> String { + stderr + .lines() + .find(|line| line.contains("takes its version from") || line.contains("take their version")) + .unwrap_or_default() + .to_owned() +} + /// The `system` scope survives a version this parser had to reconstruct. /// /// A comment inside `` costs the entry its byte-faithful span, and the From adcff2fbc99b7a480a965af09ddcf0a55f76845c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:44:36 -0400 Subject: [PATCH 25/25] fix(core): read a POM element's text the same way in both readers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two readers of one element applied different rules: `project::pom` read `Node::text` — the first text node only — while `pom_xml::text_of` concatenates every text node, on the stated and correct grounds that the join is the value Maven reads. So `1.0.0` read `1.0` as a project's own version and `1.0.0` as a dependency's: one file, one element, two answers, and a monorepo could not match a module to the dependency on it. Share `text_of`. Its rule is authoritative — its rationale is written down, and the alternative reports a version nobody wrote and then fetches it. --- crates/dependable-core/src/parsers/pom_xml.rs | 12 +++-- crates/dependable-core/src/parsers/project.rs | 53 +++++++++++++++++-- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/crates/dependable-core/src/parsers/pom_xml.rs b/crates/dependable-core/src/parsers/pom_xml.rs index c4aa6b3..d19dfed 100644 --- a/crates/dependable-core/src/parsers/pom_xml.rs +++ b/crates/dependable-core/src/parsers/pom_xml.rs @@ -86,8 +86,8 @@ pub struct PomXmlParser; /// A version literal and where it is written, before it is known whether the /// dependency that uses it may rewrite it. -struct Located { - value: String, +pub(super) struct Located { + pub(super) value: String, span: Option>, } @@ -519,7 +519,13 @@ fn child<'a>(node: roxmltree::Node<'a, 'a>, tag: &str) -> Option) -> Option { +/// +/// [`project`](super::project)'s POM reader shares this, rather than reading +/// `Node::text` itself: two readers of one element that disagree about what its text +/// *is* make a POM's own `` and a dependency's `` two different +/// rules, and `1.0.0` then reads as `1.0.0` in one place and `1.0` in the +/// other. +pub(super) fn text_of(node: roxmltree::Node<'_, '_>) -> Option { let mut texts = node.children().filter(roxmltree::Node::is_text); let first = texts.next()?; let raw = first.text()?; diff --git a/crates/dependable-core/src/parsers/project.rs b/crates/dependable-core/src/parsers/project.rs index 14d6ef0..abf158a 100644 --- a/crates/dependable-core/src/parsers/project.rs +++ b/crates/dependable-core/src/parsers/project.rs @@ -226,6 +226,12 @@ fn mix(content: &str) -> ProjectMeta { /// [`pom_xml`](super::pom_xml)); the bare `artifactId` is reported rather than a /// coordinate that would be half guessed. A `` that is a property /// (`${revision}`) is likewise not a literal this file states. +/// +/// The element's text is read by [`pom_xml::text_of`](super::pom_xml::text_of) and +/// not by `Node::text`, so this reader and the dependency reader agree on what an +/// element says: `Node::text` returns the *first* text node only, which makes +/// `1.0.0` read `1.0` here and `1.0.0` there — +/// one file, one element, two answers, and one of them a version nobody wrote. fn pom(content: &str) -> ProjectMeta { let Ok(doc) = roxmltree::Document::parse(content) else { return unnamed(); @@ -235,10 +241,9 @@ fn pom(content: &str) -> ProjectMeta { project .children() .find(|child| child.is_element() && child.tag_name().name() == tag) - .and_then(|child| child.text()) - .map(str::trim) - .filter(|text| !text.is_empty() && !text.contains('$')) - .map(str::to_owned) + .and_then(super::pom_xml::text_of) + .map(|located| located.value) + .filter(|text| !text.contains('$')) }; let name = field("artifactId").map(|artifact| match field("groupId") { Some(group) => format!("{group}:{artifact}"), @@ -466,6 +471,46 @@ mod tests { ); } + /// One element, one rule. `Node::text` stops at the first text node, so a + /// `` a comment or a character reference splits in two used to read as + /// its first half here while the dependency reader read it whole — the project + /// then claimed version `1.0` of itself while a sibling POM depending on it read + /// `1.0.0`, and a monorepo could not match the two. + #[test] + fn a_split_element_reads_the_same_here_as_in_the_dependency_reader() { + let split = parse_project( + ManifestKind::PomXml, + "\n org.example\n \ + demo\n \ + 1.0.0\n\n", + ); + assert_eq!(split.name.as_deref(), Some("org.example:demo")); + assert_eq!(split.literal_version(), Some("1.0.0")); + + // And the dependency reader, over the same shape, agrees. + let items = crate::parsers::parse( + ManifestKind::PomXml, + "\n \n \n \ + org.example\n \ + demo\n \ + 1.0.0\n \ + \n \n\n", + ) + .expect("the POM parses") + .items; + assert_eq!(items.len(), 1); + assert_eq!(items[0].name, "org.example:demo"); + assert_eq!(items[0].version_constraint, "1.0.0"); + + // A character reference is read whole by both too. + let entity = parse_project( + ManifestKind::PomXml, + "\n demo\n \ + 1.0.0\n\n", + ); + assert_eq!(entity.literal_version(), Some("1.0.0")); + } + #[test] fn manifests_named_by_their_file_are_unnamed() { assert_eq!(