From 6a3b844137516f004f0a5b28570899a1a51530d6 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 20:26:52 -0400 Subject: [PATCH 1/7] feat(core): recognize a constraint that names exactly one release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A manifest usually declares a range, but some declarations name a single release outright — Cargo's `=1.2.3`, PEP 440's `==2.28.1`, NuGet's `[1.2.3]`, a bare Maven or Hex version. A caller holding only a manifest had no way to tell those apart from a range, so it had to report every dependency's version as unknown even where the manifest had already answered. `exact_pin` decides by translation rather than by a table of spellings: the constraint goes through the same `to_semver_constraint` every version check already uses, and it is a pin exactly when that translation is one `=` comparator at full precision. So no reading is invented here that an ecosystem's own translator does not already make, and a bare `1.2.3` comes back as a pin for Maven and Hex and as `None` for Cargo, npm, Python, and NuGet — which is what those translators already say. Two properties the tests pin down: - What comes back is a slice of the *declared* constraint, never the translation. `to_semver_constraint` pads and rewrites to make a string the comparison engine accepts (`1.0` → `1.0.0`, `6.4.4.Final` → `6.4.4`, `1.0.0.4` → `1.0.0`), and none of those names a published artifact. A version shown to a user has to be one the registry has. - A pin whose spelling `semver::Version` cannot parse (`1.2.3.4`, `6.4.4.Final`) is not reported at all. It is exact beyond doubt, but every consumer compares with `semver::Version`, and a version that fails to parse there is treated as no version — which surfaces as a false "up to date" rather than as an honest unknown. --- crates/dependable-core/src/lib.rs | 4 +- crates/dependable-core/src/semver/mod.rs | 2 + crates/dependable-core/src/semver/pin.rs | 271 +++++++++++++++++++++++ 3 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 crates/dependable-core/src/semver/pin.rs diff --git a/crates/dependable-core/src/lib.rs b/crates/dependable-core/src/lib.rs index a31f285..b4cf8e3 100644 --- a/crates/dependable-core/src/lib.rs +++ b/crates/dependable-core/src/lib.rs @@ -42,4 +42,6 @@ pub use parsers::{ 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}; +pub use semver::{ + Evaluation, UnstableFilter, check_version, exact_pin, is_prerelease, to_semver_constraint, +}; diff --git a/crates/dependable-core/src/semver/mod.rs b/crates/dependable-core/src/semver/mod.rs index 054f9a8..256743b 100644 --- a/crates/dependable-core/src/semver/mod.rs +++ b/crates/dependable-core/src/semver/mod.rs @@ -5,9 +5,11 @@ pub mod elixir; pub mod maven; pub mod normalize; pub mod nuget; +pub mod pin; pub mod python; pub use checker::{Evaluation, check_version, to_version_req}; pub use normalize::{ UnstableFilter, is_prerelease, normalize_constraint, normalize_version, to_semver_constraint, }; +pub use pin::exact_pin; diff --git a/crates/dependable-core/src/semver/pin.rs b/crates/dependable-core/src/semver/pin.rs new file mode 100644 index 0000000..09466d5 --- /dev/null +++ b/crates/dependable-core/src/semver/pin.rs @@ -0,0 +1,271 @@ +//! Recognizing a constraint that names exactly one release. +//! +//! Most constraints are ranges: they say which releases a project would accept, +//! not which one it uses. A few name a single release outright — Cargo's +//! `=1.2.3`, PEP 440's `==2.28.1`, NuGet's `[1.2.3]`, a bare Maven or Hex +//! version — and for those the manifest has already answered the question a +//! lockfile would otherwise have to. [`exact_pin`] is how a caller with only a +//! manifest in hand tells the two apart. +//! +//! The decision is made by *translation*, not by a per-ecosystem table of +//! spellings: the constraint goes through the same +//! [`to_semver_constraint`](crate::semver::to_semver_constraint) every version +//! check already uses, and it is a pin exactly when that translation is a single +//! `=` comparator at full precision. Nothing here invents a reading an ecosystem's +//! translator does not already make, so a package this reports a version for is a +//! package [`check_version`](crate::semver::check_version) would call satisfied by +//! that same version and no other. + +use ::semver::{Op, Version, VersionReq}; + +use crate::ecosystem::Ecosystem; +use crate::semver::normalize::{normalize_version, to_semver_constraint}; + +/// Characters that cannot appear inside a single published version, and whose +/// presence means the extracted literal is still part of a range, a union, or a +/// build-system expression rather than a version. +/// +/// `-` and `+` are deliberately absent: they open semver's pre-release and build +/// metadata (`32.1.3-jre`, `1.2.3+sha.5114f85`), which are part of the version. +const NOT_IN_A_VERSION: &[char] = &[ + ',', '[', ']', '(', ')', '=', '<', '>', '~', '^', '!', '*', '|', '$', '"', '\'', +]; + +/// The single version a constraint names, or `None` when it names anything else. +/// +/// Returns a slice of `constraint` itself — the version **as the manifest spells +/// it**, with only surrounding whitespace, an enclosing single-version interval +/// (`[1.2.3]`), and a leading exact-match operator (`=`, `==`, `===`) removed. It +/// is never the translated form. `to_semver_constraint` pads, truncates, and +/// rewrites to produce a string the comparison engine accepts: a Maven or NuGet +/// `1.0` becomes `1.0.0`, a Maven `6.4.4.Final` becomes `6.4.4`, a NuGet `1.0.0.4` +/// becomes `1.0.0`, and none of those names the artifact the manifest asked for. A +/// version reported to a user has to be one the registry actually publishes, so the +/// declared spelling is what comes back and the translation is used only to decide. +/// +/// `None` for everything that admits more than one release: a range +/// (`^1.2`, `>=1, <2`, `[1.0,2.0)`), a union, a wildcard or floating selector +/// (`1.2.+`, `1.*`), a dist-tag (`latest`, `latest.release`), a partial-precision +/// exact requirement whose ecosystem leaves the rest free (Cargo `=1.2`), an +/// unparseable string, and an unexpanded build-system property +/// (`$(SerilogVersion)`). +/// +/// It is also `None` for a pin whose spelling the comparison engine cannot parse — +/// a four-segment NuGet `1.2.3.4`, a Maven `6.4.4.Final`. Such a version is exact +/// beyond doubt, but every consumer of it compares with `semver::Version`, and one +/// that fails to parse is silently treated as *no* version at all: the comparison +/// falls back to the newest compatible release and reports the dependency as up to +/// date. Reporting nothing is honest; reporting a version that turns into a false +/// "ok" downstream is not. +/// +/// Note that "exact" is the ecosystem's reading, not the string's shape. A bare +/// `1.2.3` is an exact version in Maven and Hex, a caret range in Cargo, npm, and +/// Python, and an open lower bound in NuGet — this reports a pin only where that +/// ecosystem's own translator already says so. +/// +/// Pure: consults no registry, filesystem, or network. +/// +/// # Examples +/// ``` +/// use dependable_core::{Ecosystem, exact_pin}; +/// +/// assert_eq!(exact_pin("=1.2.3", Ecosystem::Rust), Some("1.2.3")); +/// assert_eq!(exact_pin("1.2.3", Ecosystem::Rust), None); +/// assert_eq!(exact_pin("==2.28.1", Ecosystem::Python), Some("2.28.1")); +/// // The declared spelling, never the translation (`1.0.0` names no artifact here). +/// assert_eq!(exact_pin("1.0", Ecosystem::Jvm), Some("1.0")); +/// ``` +#[must_use] +pub fn exact_pin(constraint: &str, ecosystem: Ecosystem) -> Option<&str> { + let literal = pin_literal(constraint)?; + + // The ecosystem's own translation decides. One comparator, `=`, at full + // precision: anything else names a set, however exact it looks. + let req = VersionReq::parse(&to_semver_constraint(constraint, ecosystem)).ok()?; + let [only] = req.comparators.as_slice() else { + return None; + }; + if only.op != Op::Exact || only.minor.is_none() || only.patch.is_none() { + return None; + } + + // And the literal has to survive the trip every consumer makes with it. + Version::parse(&normalize_version(literal)).ok()?; + Some(literal) +} + +/// Strip a constraint down to the version literal it is built around, without +/// interpreting it. Returns `None` when what is left could not be a version. +fn pin_literal(constraint: &str) -> Option<&str> { + let mut c = constraint.trim(); + // An interval naming one version rather than a range: `[1.2.3]`. + if let Some(inner) = c.strip_prefix('[').and_then(|rest| rest.strip_suffix(']')) { + c = inner.trim(); + } + // Cargo's `=`, PEP 440's `==` and `===`, Hex's `==`. + c = c.trim_start_matches('=').trim_start(); + if c.is_empty() || c.contains(NOT_IN_A_VERSION) || c.contains(char::is_whitespace) { + return None; + } + Some(c) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// One table over every ecosystem whose translator this consults, so a change + /// to any of them shows up here rather than in a graph test. + #[test] + fn recognizes_exactly_the_constraints_that_name_one_release() { + let cases: &[(&str, Ecosystem, Option<&str>)] = &[ + // -- C# / NuGet ------------------------------------------------ + // A bare `Version` is an inclusive *minimum* in NuGet, so it names a + // set. See `semver::nuget`; #113 tracks whether that reading is right. + ("13.0.1", Ecosystem::CSharp, None), + ("[2.10.0,3.0.0)", Ecosystem::CSharp, None), + ("[1.2.3]", Ecosystem::CSharp, Some("1.2.3")), + ("1.*", Ecosystem::CSharp, None), + // An unexpanded MSBuild property is not a version. + ("$(SerilogVersion)", Ecosystem::CSharp, None), + // Exact, but four segments: `semver::Version` cannot read it, so a + // consumer would compare against nothing at all. + ("[1.2.3.4]", Ecosystem::CSharp, None), + // -- JVM / Maven + Gradle -------------------------------------- + ("4.12.0", Ecosystem::Jvm, Some("4.12.0")), + ("1.9.24", Ecosystem::Jvm, Some("1.9.24")), + ("3.14.0", Ecosystem::Jvm, Some("3.14.0")), + // The declared spelling, not the translation: `maven_to_semver` + // makes this `32.1.3`, which names no artifact on Maven Central. + ("32.1.3-jre", Ecosystem::Jvm, Some("32.1.3-jre")), + ("[1.0,2.0)", Ecosystem::Jvm, None), + ("1.2.+", Ecosystem::Jvm, None), + ("latest.release", Ecosystem::Jvm, None), + // Exact in Maven, unreadable to `semver::Version`. + ("6.4.4.Final", Ecosystem::Jvm, None), + // -- Rust / Cargo ---------------------------------------------- + ("=1.2.3", Ecosystem::Rust, Some("1.2.3")), + ("= 1.2.3", Ecosystem::Rust, Some("1.2.3")), + ("1.2.3", Ecosystem::Rust, None), + ("=1.2", Ecosystem::Rust, None), + ("^1.2", Ecosystem::Rust, None), + ("*", Ecosystem::Rust, None), + (">=1, <2", Ecosystem::Rust, None), + ("=1.2.3-alpha.1", Ecosystem::Rust, Some("1.2.3-alpha.1")), + // -- npm -------------------------------------------------------- + ("^18.0.0", Ecosystem::Npm, None), + ("latest", Ecosystem::Npm, None), + ("1.2.3", Ecosystem::Npm, None), + ("=1.3.0", Ecosystem::Npm, Some("1.3.0")), + // -- Python ----------------------------------------------------- + ("==2.28.1", Ecosystem::Python, Some("2.28.1")), + ("==0.20", Ecosystem::Python, Some("0.20")), + (">=2.0", Ecosystem::Python, None), + ("==1.0.*", Ecosystem::Python, None), + ("~=1.4.2", Ecosystem::Python, None), + (">=1.0,<2.0", Ecosystem::Python, None), + // -- Elixir / Hex ----------------------------------------------- + ("3.10.3", Ecosystem::Elixir, Some("3.10.3")), + ("== 3.10.3", Ecosystem::Elixir, Some("3.10.3")), + ("~> 3.10", Ecosystem::Elixir, None), + (">= 3.0.0", Ecosystem::Elixir, None), + // -- Dart, Go, PHP ---------------------------------------------- + ("6.0.5", Ecosystem::Dart, None), + ("^1.1.0", Ecosystem::Dart, None), + ("v1.6.0", Ecosystem::Go, None), + ("^2.0", Ecosystem::Php, None), + // -- Nothing at all --------------------------------------------- + ("", Ecosystem::Rust, None), + (" ", Ecosystem::Jvm, None), + ("workspace:*", Ecosystem::Npm, None), + ]; + + for &(constraint, ecosystem, want) in cases { + assert_eq!( + exact_pin(constraint, ecosystem), + want, + "{constraint:?} ({ecosystem:?})" + ); + } + } + + /// The reported version has to exist on the registry, so it is a slice of what + /// the manifest wrote. Every row here is one the translation *rewrites*, which + /// is what makes returning the translated string a live hazard rather than a + /// theoretical one. + #[test] + fn reports_the_declared_spelling_and_never_the_translation() { + for (constraint, ecosystem) in [ + ("1.0", Ecosystem::Jvm), + ("[1.0]", Ecosystem::CSharp), + ("==0.20", Ecosystem::Python), + ] { + let pin = exact_pin(constraint, ecosystem).expect("a pin"); + assert!( + constraint.contains(pin), + "{pin:?} must be a slice of {constraint:?}" + ); + assert_ne!( + pin, + to_semver_constraint(constraint, ecosystem).trim_start_matches('='), + "{constraint:?} must not be reported in its translated form" + ); + } + } + + /// Whatever comes back is a version, not the empty string and not a fragment + /// of the range it was cut out of — the invariant every caller relies on when + /// it puts the result straight into a graph node. + #[test] + fn a_reported_pin_is_always_a_usable_version() { + let probes = [ + "", + " ", + "=", + "==", + "===", + "[]", + "[,]", + "[1.0,2.0]", + "[1.0],[2.0]", + "(,1.0],[1.2,)", + ">=1", + "1.x", + "*", + "+", + "1.+", + "latest", + "^", + "~>", + "$(Prop)", + "==1.0.*", + "=1.2.3.4", + "git+https://example.com/x#v1.2.3", + "workspace:^1.0.0", + "1.2.3 || 2.0.0", + "=1.2.3, =1.2.3", + ]; + for ecosystem in [ + Ecosystem::Rust, + Ecosystem::Go, + Ecosystem::Npm, + Ecosystem::Python, + Ecosystem::Php, + Ecosystem::Dart, + Ecosystem::CSharp, + Ecosystem::Elixir, + Ecosystem::Jvm, + ] { + for probe in probes { + let Some(pin) = exact_pin(probe, ecosystem) else { + continue; + }; + assert!(!pin.is_empty(), "{probe:?} ({ecosystem:?})"); + assert!( + Version::parse(&normalize_version(pin)).is_ok(), + "{probe:?} ({ecosystem:?}) yielded {pin:?}" + ); + } + } + } +} From 3d301e211496ab52b6dd9e198ba7de68420a3977 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:22:52 -0400 Subject: [PATCH 2/7] feat(fetch): report a dependency its manifest already pinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A manifest-only graph reported every dependency's version as unknown, including the ones the manifest had already settled. `serde = "=1.0.200"` admits exactly one release and nothing else; so does a NuGet `[1.2.3]` and a bare Gradle `4.12.0`. Calling those unknown understated what had been read, and did it in the same graph that already reports a workspace member's declared version for exactly the same reason. The constraint was being thrown away one line before the node was built: `build_project_graph` mapped the parsed items to bare names, and `shallow_graph` passed `None` for every external package. Both now carry the pin `declared_pin` reads off the declaration. Candidacy is `Item::is_checkable()`, the existing predicate for "there is a version string here worth asking a registry about". That keeps a git or path reference, and an `Inherited` entry no root has supplied a constraint for, unknown — without a second rule that could drift from the first. An inherited entry the root *did* supply is resolved before the graph is assembled, so a centrally pinned crate reports the root's pin. Where two declarations of one name do not agree on a pin, the node reports nothing. They collapse into one node, so taking the first would make the answer depend on the order the manifest lists them in, or the order the directory walk found the members in. That is not a resolution. `a_manifest_only_graph_leaves_every_dependency_version_unknown` asserted the behaviour this changes; it is rewritten as a per-node expectation naming the exact spelling of each version rather than deleted, because the spelling is the part worth protecting: guava must report `32.1.3-jre`, and Maven Central publishes no `32.1.3` at all. What this does not do: a `*.csproj` `` still reports unknown. NuGet's translator reads a bare version as an inclusive minimum, not a pin, so admitting it here would make `tree` claim a resolution for a line `check` reports as satisfied by every later release. That reading is itself a defect and is filed as #113. --- crates/dependable-fetch/src/tree.rs | 156 ++++++++++----- .../dependable-fetch/tests/project_graph.rs | 186 ++++++++++++++++-- crates/dependable-fetch/tests/tree.rs | 127 ++++++++++++ 3 files changed, 405 insertions(+), 64 deletions(-) diff --git a/crates/dependable-fetch/src/tree.rs b/crates/dependable-fetch/src/tree.rs index 9bca71b..c42e9aa 100644 --- a/crates/dependable-fetch/src/tree.rs +++ b/crates/dependable-fetch/src/tree.rs @@ -7,16 +7,18 @@ //! graph already lives in `Cargo.lock`. //! //! When no `Cargo.lock` is present it degrades to a **shallow** graph built from -//! the manifests alone (members plus their direct declared dependencies, with -//! versions left unresolved), flagged via [`GraphSource::Manifests`]. +//! the manifests alone (members plus their direct declared dependencies), flagged +//! via [`GraphSource::Manifests`]. Such a dependency's version is normally unknown +//! — a manifest declares a constraint, not a resolution — except where the +//! constraint names exactly one release, which [`declared_pin`] reads off it. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use dependable_core::{ - CargoTomlParser, DependencyGraph, DependencyKind, Item, LockedPackage, LockfileKind, - ManifestKind, PackageSource, ParseError, Parser, ResolvedLockfile, parse, parse_bun_lock_graph, - parse_cargo_lock_graph, parse_composer_lock_graph, parse_mix_lock_graph, + CargoTomlParser, DependencyGraph, DependencyKind, Ecosystem, Item, LockedPackage, LockfileKind, + ManifestKind, PackageSource, ParseError, Parser, ResolvedLockfile, exact_pin, parse, + parse_bun_lock_graph, parse_cargo_lock_graph, parse_composer_lock_graph, parse_mix_lock_graph, parse_package_lock_graph, parse_package_name, parse_project, parse_workspace, resolve_workspace_inheritance, }; @@ -256,7 +258,9 @@ fn walk_members( /// is not resolved against anything, so what the manifest declares **is** its /// version, whether or not a lockfile exists. Its dependencies are a different /// matter: a manifest declares a constraint, not a resolution, so those stay -/// unknown. +/// unknown — unless the constraint names exactly one release (`= "1.0.200"`), in +/// which case the manifest has already resolved it and [`declared_pin`] reads it +/// off. /// /// `version.workspace = true` is resolved against the root's `[workspace.package]` /// only for a member the root actually governs (see [`Member::governed_by_root`]). @@ -286,7 +290,7 @@ fn shallow_graph( .unwrap_or_default(); let mut member_pkgs: Vec = Vec::new(); let mut external_pkgs: Vec = Vec::new(); - let mut external_seen: HashSet = HashSet::new(); + let mut external_seen: HashMap = HashMap::new(); for member in members { let mut items = CargoTomlParser @@ -297,24 +301,42 @@ fn shallow_graph( let mut deps: Vec = Vec::new(); for item in &items { deps.push(item.name.clone()); - if !workspace_names.contains(&item.name) && external_seen.insert(item.name.clone()) { - // Synthesize a source so classification matches the item's kind. An - // inherited entry has already taken its root declaration's source above, - // so a centrally-declared `path` crate lands on the `Local` arm and a - // centrally-declared registry crate does not. - let source = match item.source { - PackageSource::Git => Some("git+".to_owned()), - PackageSource::Local => None, - _ => Some("registry+".to_owned()), - }; - external_pkgs.push(LockedPackage::new( - item.name.clone(), - // A manifest declares a constraint, not a resolved version; - // nothing here read one. - None, - source, - Vec::new(), - )); + if workspace_names.contains(&item.name) { + continue; + } + // Items are inheritance-resolved by now, so a member's + // `dep.workspace = true` pointing at a root `= "1.0.200"` is read here + // as the pin the root declared. + let pin = declared_pin(item, Ecosystem::Rust).map(str::to_owned); + match external_seen.get(&item.name) { + // One node for the name, so a version survives only where every + // member that declares it agrees. First-wins would make the graph + // depend on the order the directory walk happened to find them in. + Some(&idx) => { + if external_pkgs[idx].version != pin { + external_pkgs[idx].version = None; + } + } + None => { + // Synthesize a source so classification matches the item's kind. An + // inherited entry has already taken its root declaration's source above, + // so a centrally-declared `path` crate lands on the `Local` arm and a + // centrally-declared registry crate does not. + let source = match item.source { + PackageSource::Git => Some("git+".to_owned()), + PackageSource::Local => None, + _ => Some("registry+".to_owned()), + }; + external_seen.insert(item.name.clone(), external_pkgs.len()); + external_pkgs.push(LockedPackage::new( + item.name.clone(), + // Usually `None`: a manifest declares a constraint, not a + // resolved version, and nothing here read one. + pin, + source, + Vec::new(), + )); + } } } deps.sort(); @@ -388,10 +410,13 @@ pub fn build_project_graph( let root_version: Option = meta.literal_version().map(str::to_owned); // The project's own declared dependencies, used as the root's edges whenever the - // lockfile carries no entry for the project itself. - let direct: Vec = parse(kind, &content) - .map(|parsed| parsed.items.into_iter().map(|i| i.name).collect()) + // lockfile carries no entry for the project itself. Kept as whole items: a + // constraint that names one release is the only version a manifest-only graph + // will ever have for these, and mapping to bare names here would discard it. + let direct: Vec = parse(kind, &content) + .map(|parsed| parsed.items) .unwrap_or_default(); + let direct_names: Vec = direct.iter().map(|item| item.name.clone()).collect(); let workspace_names: HashSet = std::iter::once(root_name.clone()).collect(); let roots: Vec = match &opts.package { @@ -404,6 +429,7 @@ pub fn build_project_graph( &root_name, root_version.as_deref(), &direct, + kind.ecosystem(), &workspace_names, &roots, ); @@ -418,6 +444,7 @@ pub fn build_project_graph( &root_name, root_version.as_deref(), &direct, + kind.ecosystem(), &workspace_names, &roots, ); @@ -437,6 +464,7 @@ pub fn build_project_graph( &root_name, root_version.as_deref(), &direct, + kind.ecosystem(), &workspace_names, &roots, ); @@ -447,7 +475,7 @@ pub fn build_project_graph( }; let resolved = parser(&read(&lock_path)?)?; - let resolved = with_root(resolved, &root_name, root_version.as_deref(), direct); + let resolved = with_root(resolved, &root_name, root_version.as_deref(), direct_names); Ok(WorkspaceGraph { graph: DependencyGraph::from_resolved(&resolved, &workspace_names, &roots), source: GraphSource::Lockfile, @@ -516,12 +544,34 @@ fn with_root( ResolvedLockfile::from_packages(packages) } -/// A two-level graph: the project and the dependencies it declares, versions -/// unresolved. Used when no resolved graph is available. +/// The version a declared dependency is already resolved to, when its constraint +/// names exactly one release; `None` otherwise. +/// +/// This is the whole difference between a manifest-only graph that reports +/// `unknown` for everything and one that reports what the manifest already +/// settled: `serde = "=1.0.200"` admits one release and nothing else, so calling +/// it unknown understates what was read, exactly as it would for a member's own +/// declared version. +/// +/// Gated on [`Item::is_checkable`], the existing predicate for "there is a version +/// string here worth asking a registry about". That is what keeps a git or path +/// reference — and an `Inherited` entry no root has supplied a constraint for — +/// unknown, without a second rule that could drift from the first. +fn declared_pin(item: &Item, ecosystem: Ecosystem) -> Option<&str> { + if !item.is_checkable() { + return None; + } + exact_pin(&item.version_constraint, ecosystem) +} + +/// A two-level graph: the project and the dependencies it declares. A version is +/// carried only where the declaration named one ([`declared_pin`]). Used when no +/// resolved graph is available. fn direct_graph( root_name: &str, root_version: Option<&str>, - direct: &[String], + direct: &[Item], + ecosystem: Ecosystem, workspace_names: &HashSet, roots: &[String], ) -> DependencyGraph { @@ -529,19 +579,35 @@ fn direct_graph( root_name.to_owned(), root_version.map(str::to_owned), None, - direct.to_vec(), + direct.iter().map(|item| item.name.clone()).collect(), )]; - let mut seen: HashSet<&str> = HashSet::new(); - for name in direct { - if name != root_name && seen.insert(name.as_str()) { - packages.push(LockedPackage::new( - name.clone(), - // A manifest names its dependencies; nothing here resolved one - // to a version, and saying so is the point of the `None`. - None, - Some("registry+".to_owned()), - Vec::new(), - )); + let mut seen: HashMap<&str, usize> = HashMap::new(); + for item in direct { + if item.name == root_name { + continue; + } + let pin = declared_pin(item, ecosystem).map(str::to_owned); + match seen.get(item.name.as_str()) { + // Two declarations of one name collapse into one node, so a version + // may only be carried when they agree on it. Taking the first would + // make the answer depend on the order the manifest happens to list + // them in, which is not a resolution of anything. + Some(&idx) => { + if packages[idx].version != pin { + packages[idx].version = None; + } + } + None => { + seen.insert(item.name.as_str(), packages.len()); + packages.push(LockedPackage::new( + item.name.clone(), + // A manifest names its dependencies and usually only + // constrains them; `None` is how the graph says so. + pin, + Some("registry+".to_owned()), + Vec::new(), + )); + } } } let resolved = ResolvedLockfile::from_packages(packages); diff --git a/crates/dependable-fetch/tests/project_graph.rs b/crates/dependable-fetch/tests/project_graph.rs index c750221..6d1a62e 100644 --- a/crates/dependable-fetch/tests/project_graph.rs +++ b/crates/dependable-fetch/tests/project_graph.rs @@ -215,33 +215,63 @@ fn an_ecosystem_without_edge_data_reports_unsupported() { assert!(names.len() > 1, "the direct dependencies are still shown"); } -/// A manifest names its dependencies; it does not resolve them. The graph must -/// say the version is *unknown* rather than record it as the empty string, which -/// downstream reads as a version and evaluates as though it were one. +/// Every version in the graph, by node name, so a change to any one of them has +/// to be stated rather than absorbed. +fn versions(graph: &DependencyGraph) -> Vec<(&str, Option<&str>)> { + graph + .nodes() + .iter() + .map(|n| (n.name.as_str(), n.version.as_deref())) + .collect() +} + +/// The version of one node, panicking if there is no such node — so an assertion +/// about a node cannot silently pass because the node is missing. +fn version_of<'g>(graph: &'g DependencyGraph, name: &str) -> Option<&'g str> { + graph + .nodes() + .iter() + .find(|n| n.name == name) + .unwrap_or_else(|| panic!("a node for {name}")) + .version + .as_deref() +} + +/// A manifest names its dependencies and usually only constrains them — but a +/// Gradle catalog states a version outright, and a constraint that admits exactly +/// one release has already resolved it. Reporting `unknown` for these understated +/// what the file plainly said. +/// +/// Asserted per node, with the exact spelling of each, because the two facts worth +/// protecting are both about *which* string comes back: +/// +/// - `guava` must be `32.1.3-jre` and never the translated `32.1.3`. Maven Central +/// publishes `32.1.3-jre` and `32.1.3-android` and nothing called `32.1.3`, so +/// the translation names no artifact at all. +/// - `kotlin-stdlib` and `kotlin-reflect` share one `[versions]` alias, which +/// reaches them as a *resolved* `Inherited` item. Those are checkable and so +/// report the alias's version; an alias no `[versions]` entry defines would not. #[test] -fn a_manifest_only_graph_leaves_every_dependency_version_unknown() { +fn a_manifest_only_graph_reports_the_versions_the_manifest_settled() { let built = build_project_graph( &fixture("sample-kotlin").join("gradle/libs.versions.toml"), &WorkspaceGraphOptions::default(), ) .expect("graph"); - let unresolved: Vec<&str> = built - .graph - .nodes() - .iter() - .filter(|n| n.kind == NodeKind::Registry) - .filter(|n| n.version.is_some()) - .map(|n| n.name.as_str()) - .collect(); - assert!( - !built.graph.nodes().is_empty(), - "the fixture must produce a graph to assert about" - ); assert_eq!( - unresolved, - Vec::<&str>::new(), - "nothing read a version for these, so none may claim one" + versions(&built.graph), + vec![ + // The catalog declares no project of its own; the root is its directory. + ("gradle", None), + ("org.jetbrains.kotlin:kotlin-stdlib", Some("1.9.24")), + ("org.jetbrains.kotlin:kotlin-reflect", Some("1.9.24")), + ("com.squareup.okhttp3:okhttp", Some("4.12.0")), + ("org.junit.jupiter:junit-jupiter", Some("5.10.2")), + // The declared spelling, not `maven_to_semver`'s `32.1.3`. + ("com.google.guava:guava", Some("32.1.3-jre")), + ("org.apache.commons:commons-lang3", Some("3.14.0")), + ], ); assert!( built @@ -251,6 +281,124 @@ fn a_manifest_only_graph_leaves_every_dependency_version_unknown() { .all(|n| n.version.as_deref() != Some("")), "an unknown version is `None`, never an empty string" ); + // A catalog entry stating no version at all — a BOM supplies it at build time + // — is not a dependency this file resolved, and is not in the graph. + assert!( + !built + .graph + .nodes() + .iter() + .any(|n| n.name.contains("jackson-databind")), + ); +} + +/// The case #107 opened with, and the one this change does **not** close. NuGet +/// reads a bare `Version` as an inclusive *minimum* (`>=13.0.1`), not a pin — see +/// `nuget_constraint_to_semver`, "A bare version is an inclusive minimum in NuGet" +/// — so `Newtonsoft.Json` still reports no version. +/// +/// Admitting it here would make `tree` claim a resolution for a line `check` +/// reports as satisfied by every later release, which is a disagreement about one +/// line of one file. The reading itself is the defect, and it is filed as #113; +/// this test is the record of what today's translation says, and is expected to +/// change with it. +#[test] +fn a_bare_nuget_version_is_a_minimum_and_so_resolves_nothing() { + let built = build_project_graph( + &fixture("sample-csharp").join("App.csproj"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(version_of(&built.graph, "Newtonsoft.Json"), None); + // An interval spanning two majors names a set by anyone's reading. + assert_eq!(version_of(&built.graph, "Serilog"), None); + // A reference whose version is an unexpanded MSBuild property, and one with no + // `Version` at all, state nothing to resolve — the parser drops both, so they + // are absent from the graph rather than present with a version of `None`. + for absent in ["FromProperty", "Microsoft.Extensions.Hosting"] { + assert!( + !built.graph.nodes().iter().any(|n| n.name == absent), + "{absent} states no version and is not a dependency this file resolved" + ); + } +} + +/// npm reads a bare `1.3.0` as a caret range, exactly as Cargo does, so neither of +/// these is a pin. The rule is about what the *constraint* admits, not about how +/// concrete it looks: `"left-pad": "1.3.0"` accepts every 1.x release npm ever +/// publishes. +#[test] +fn a_concrete_looking_npm_constraint_is_still_a_range() { + let dir = TempDir::new().expect("tempdir"); + write( + &dir.path().join("package.json"), + r#"{ "name": "app", "version": "1.0.0", + "dependencies": { "react": "^18.0.0", "left-pad": "1.3.0", "pinned": "=4.17.21" } }"#, + ); + + let built = build_project_graph( + &dir.path().join("package.json"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(built.source, GraphSource::Manifests); + assert_eq!(version_of(&built.graph, "react"), None); + assert_eq!(version_of(&built.graph, "left-pad"), None); + // npm's explicit `=` is the one form that does name a single release. + assert_eq!(version_of(&built.graph, "pinned"), Some("4.17.21")); +} + +/// Candidacy is `Item::is_checkable()`, the existing predicate for "there is a +/// version string here worth asking a registry about". A git or link spec fails it +/// however much of a version the spec has written into it, so no second rule is +/// needed to keep those unknown — and no such rule can drift from the first. +#[test] +fn a_git_or_local_dependency_stays_unknown_however_it_is_spelled() { + let dir = TempDir::new().expect("tempdir"); + write( + &dir.path().join("package.json"), + r#"{ "name": "app", "version": "1.0.0", + "dependencies": { + "fromgit": "git+https://example.com/x.git#1.2.3", + "linked": "link:../linked" } }"#, + ); + + let built = build_project_graph( + &dir.path().join("package.json"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(version_of(&built.graph, "fromgit"), None); + assert_eq!(version_of(&built.graph, "linked"), None); +} + +/// Two declarations of one name collapse into one node, so a version may only be +/// carried when they agree on it. Taking the first would make the graph depend on +/// the order the sections happen to be listed in — which is not a resolution of +/// anything, and would report a version half the file contradicts. +#[test] +fn two_declarations_that_disagree_resolve_to_nothing() { + let dir = TempDir::new().expect("tempdir"); + write( + &dir.path().join("package.json"), + r#"{ "name": "app", "version": "1.0.0", + "dependencies": { "split": "=1.0.0", "agreed": "=2.0.0" }, + "peerDependencies": { "split": "=2.0.0", "agreed": "=2.0.0" } }"#, + ); + + let built = build_project_graph( + &dir.path().join("package.json"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(version_of(&built.graph, "split"), None); + // Agreement is not ambiguity: two declarations naming the same release still + // name it. + assert_eq!(version_of(&built.graph, "agreed"), Some("2.0.0")); } #[test] diff --git a/crates/dependable-fetch/tests/tree.rs b/crates/dependable-fetch/tests/tree.rs index bc7e467..52e416c 100644 --- a/crates/dependable-fetch/tests/tree.rs +++ b/crates/dependable-fetch/tests/tree.rs @@ -127,6 +127,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" .unwrap(); } +/// The version of one node, panicking if there is no such node — so an assertion +/// about a node cannot silently pass because the node is missing. +fn version_of<'g>(graph: &'g DependencyGraph, name: &str) -> Option<&'g str> { + graph + .nodes() + .iter() + .find(|n| n.name == name) + .unwrap_or_else(|| panic!("node {name} missing")) + .version + .as_deref() +} + fn kind_of(graph: &DependencyGraph, name: &str) -> NodeKind { graph .nodes() @@ -427,3 +439,118 @@ fn the_root_decides_what_an_inherited_dependency_is_in_the_shallow_graph() { // And a member's own path entry, which never depended on the root at all. assert_eq!(kind_of(&built.graph, "vendored"), NodeKind::Path); } + +/// Without a lockfile a dependency's version is normally unknown, because a +/// manifest declares a constraint rather than a resolution. `= "1.0.200"` is the +/// exception: it admits exactly one release, so the manifest has already resolved +/// it and calling it unknown understates what was read. +/// +/// Cargo's `"1"` beside it is the control. It looks concrete and is a caret range +/// over every 1.x release, which is why the rule cannot be "the string has three +/// numbers in it". +#[test] +fn an_exactly_pinned_dependency_reports_its_version_without_a_lockfile() { + let tmp = TempDir::new().unwrap(); + fs::write( + tmp.path().join("Cargo.toml"), + "[workspace]\nresolver = \"2\"\nmembers = [\"crates/a\"]\n", + ) + .unwrap(); + fs::create_dir_all(tmp.path().join("crates/a")).unwrap(); + fs::write( + tmp.path().join("crates/a/Cargo.toml"), + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\n\ + serde = \"=1.0.200\"\n\ + regex = \"1\"\n\ + partial = \"=1.2\"\n\ + gitdep = { git = \"https://example.com/gitdep\", version = \"=1.2.3\" }\n\ + localdep = { path = \"../../vendor/v\", version = \"=4.5.6\" }\n", + ) + .unwrap(); + + let built = build_workspace_graph(tmp.path(), &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + let g = &built.graph; + + assert_eq!(version_of(g, "serde"), Some("1.0.200")); + // A caret range over every 1.x release names no single one of them. + assert_eq!(version_of(g, "regex"), None); + // `=1.2` is exact only to the minor: Cargo still accepts every 1.2.x patch. + assert_eq!(version_of(g, "partial"), None); + // Candidacy is `Item::is_checkable()`, which a git or path source fails + // whatever version is written beside it. These are the crate at that location, + // not the crate the registry publishes under that number. + assert_eq!(version_of(g, "gitdep"), None); + assert_eq!(version_of(g, "localdep"), None); + assert_eq!(kind_of(g, "gitdep"), NodeKind::Git); + assert_eq!(kind_of(g, "localdep"), NodeKind::Path); + + // The member's own version is read from `[package] version` and is untouched + // by any of this. + assert_eq!(version_of(g, "a"), Some("0.1.0")); + assert!( + g.nodes().iter().all(|n| n.version != Some(String::new())), + "and never the empty string" + ); +} + +/// A member's `dep.workspace = true` carries no version of its own; the root's +/// `[workspace.dependencies]` entry does, and inheritance is already resolved +/// before the graph is assembled. So a centrally pinned crate reports the pin the +/// root declared — the shape most workspaces that pin anything actually use. +#[test] +fn a_centrally_pinned_inherited_dependency_reports_the_roots_version() { + let tmp = TempDir::new().unwrap(); + fs::write( + tmp.path().join("Cargo.toml"), + "[workspace]\nresolver = \"2\"\nmembers = [\"crates/a\"]\n\n\ + [workspace.dependencies]\n\ + serde = \"=1.0.200\"\n\ + regex = \"1\"\n", + ) + .unwrap(); + fs::create_dir_all(tmp.path().join("crates/a")).unwrap(); + fs::write( + tmp.path().join("crates/a/Cargo.toml"), + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserde.workspace = true\nregex.workspace = true\n", + ) + .unwrap(); + + let built = build_workspace_graph(tmp.path(), &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + assert_eq!(version_of(&built.graph, "serde"), Some("1.0.200")); + assert_eq!(version_of(&built.graph, "regex"), None); +} + +/// One node per name across the whole workspace, so a version may only be carried +/// when every member that declares the crate agrees on it. Reporting the first +/// would make the graph depend on the order the directory walk found the members +/// in, and would state a version the other member's manifest contradicts. +#[test] +fn two_members_that_pin_one_crate_differently_resolve_to_nothing() { + let tmp = TempDir::new().unwrap(); + fs::write( + tmp.path().join("Cargo.toml"), + "[workspace]\nresolver = \"2\"\nmembers = [\"crates/a\", \"crates/b\"]\n", + ) + .unwrap(); + for (member, serde, agreed) in [("a", "=1.0.200", "=1.0.0"), ("b", "=1.0.201", "=1.0.0")] { + fs::create_dir_all(tmp.path().join("crates").join(member)).unwrap(); + fs::write( + tmp.path().join("crates").join(member).join("Cargo.toml"), + format!( + "[package]\nname = \"{member}\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserde = \"{serde}\"\nagreed = \"{agreed}\"\n" + ), + ) + .unwrap(); + } + + let built = build_workspace_graph(tmp.path(), &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + assert_eq!(version_of(&built.graph, "serde"), None); + // Agreement is not ambiguity: two members naming the same release still name it. + assert_eq!(version_of(&built.graph, "agreed"), Some("1.0.0")); +} From 0e862290626186a81b64fade9d43b8d3e11f75b8 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:22:52 -0400 Subject: [PATCH 3/7] docs: say when a manifest-only graph reports a dependency's version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Node::version` and the README's `tree` section both stated that a dependency in a manifest-only graph is always `null`, "because a manifest declares a constraint rather than a resolution". That is now the common case rather than the rule: a constraint admitting exactly one release has resolved it. Both say so, and both say the two things a reader needs in order to predict the output: the version reported is the one the manifest spells rather than a normalized form of it, and whether a bare version is a pin is the ecosystem's call rather than the string's shape — Cargo, npm and Python read `1.2.3` as a range, NuGet as a lower bound, Maven and Hex as exact. --- README.md | 13 ++++++++++--- crates/dependable-core/src/graph.rs | 9 ++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7928165..9d3d1f8 100644 --- a/README.md +++ b/README.md @@ -410,9 +410,16 @@ and the `ascii` and `dot` renderers drop the `vX.Y.Z` suffix for the same node. A shallow tree — built from manifests, with no `Cargo.lock` to resolve against — still reports each **workspace member's** declared version, because a member is not resolved against anything and what its manifest declares is what the crate -is; its **dependencies** are `null`, because a manifest declares a constraint -rather than a resolution. A version is never the empty string: a blank one in a -lockfile is read as no version at all. +is. A **dependency** is `null` there, because a manifest usually declares a +constraint rather than a resolution — with one exception: a constraint that +admits exactly one release (`serde = "=1.0.200"`, a PEP 440 `==2.28.1`, a NuGet +`[1.2.3]`, a bare Gradle or Hex version) has already resolved it, and the version +reported is the one the manifest spells, not a normalized form of it. Whether a +bare version is a pin is the ecosystem's call, not the string's shape: Cargo, +npm, and Python read `1.2.3` as a range, and NuGet reads it as a lower bound. A +git or path dependency is always `null`, whatever version sits beside it. A +version is never the empty string: a blank one in a lockfile is read as no +version at all. ``` my-app v0.1.0 (workspace) diff --git a/crates/dependable-core/src/graph.rs b/crates/dependable-core/src/graph.rs index b5e3e39..ab1c68c 100644 --- a/crates/dependable-core/src/graph.rs +++ b/crates/dependable-core/src/graph.rs @@ -40,9 +40,12 @@ pub struct Node { /// /// A workspace member carries the version its own manifest declares — a /// member is resolved against nothing, so its declaration *is* its version, - /// whether or not a lockfile exists. `None` is a dependency in a graph built - /// from manifests alone, where the manifest gave a constraint and nothing - /// resolved it, and a package a lockfile records without a version at all. + /// whether or not a lockfile exists. A dependency in a graph built from + /// manifests alone carries one only where its constraint named exactly one + /// release, which is the manifest resolving it rather than constraining it. + /// `None` is everything else: a constraint that admits a set and nothing + /// resolved it, a git or path reference, and a package a lockfile records + /// without a version at all. /// /// An [`Option`] rather than an empty-string sentinel, and never /// `Some("")`: a renderer must be able to say "unknown" rather than evaluate From 61edacf73cb620c55ba68b76c91e22b3d4cab42a Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:30 -0400 Subject: [PATCH 4/7] fix(core): prove a pin on the string its consumers actually parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `exact_pin`'s last guard checked the literal after `normalize_version` padded it, and nothing downstream pads. `declared_pin` puts the literal straight into a `Node::version`, and every reader of that field — the CLI renderers, the JSON and DOT emitters, the OSV query, and the TUI's lookup — parses it exactly as written. So the guard proved a claim about a string no consumer ever constructs, and the comment above it said the opposite of what the line did. A Gradle catalog pinning `junit:junit` at `4.12` therefore reached `check_version` as a raw `4.12`, which `Version::parse` rejects; that is read as no current version at all, the comparison falls back to the newest release, and the row rendered a green `ok` for a dependency three releases and nine years behind. That is issue #96's failure verbatim, reintroduced inside the branch stacked on its fix. Nothing caught it because its two consequences suppress each other: the same raw string also misses OSV, and `ui/tree.rs` draws the vulnerability badge ahead of the status badge, so wherever OSV does match, the false `ok` is hidden. Parse the literal as written instead. It is strictly a narrowing — `4.12`, `1.0`, `[1.0]` and `==0.20` drop back to `unknown`, which is what they reported before this branch — and it matches #107's own criterion, that the declared string parse cleanly as an exact `semver::Version`. Padding is not an alternative: `4.12.0` is a different artifact from `4.12` and Maven Central publishes only one of them, so the declared string is the only one that can be reported and therefore the only one worth testing. The three witnesses in the declared-spelling test were all padding cases, so the test proving that rule proved it only on inputs the rule now rejects. They are replaced with rewrites the guard still permits: a Maven release alias, a PEP 440 local segment, and a PEP 440 pre-release respelling. The usable-version invariant now parses the pin raw as well. --- crates/dependable-core/src/semver/pin.rs | 117 ++++++++++++++++++----- 1 file changed, 93 insertions(+), 24 deletions(-) diff --git a/crates/dependable-core/src/semver/pin.rs b/crates/dependable-core/src/semver/pin.rs index 09466d5..252f531 100644 --- a/crates/dependable-core/src/semver/pin.rs +++ b/crates/dependable-core/src/semver/pin.rs @@ -11,15 +11,24 @@ //! spellings: the constraint goes through the same //! [`to_semver_constraint`](crate::semver::to_semver_constraint) every version //! check already uses, and it is a pin exactly when that translation is a single -//! `=` comparator at full precision. Nothing here invents a reading an ecosystem's -//! translator does not already make, so a package this reports a version for is a -//! package [`check_version`](crate::semver::check_version) would call satisfied by -//! that same version and no other. +//! `=` comparator at full precision *and* the declared literal is itself a +//! `semver::Version`. Nothing here invents a reading an ecosystem's translator does +//! not already make, so a package this reports a version for is a package +//! [`check_version`](crate::semver::check_version) would call satisfied by that +//! same version and no other. +//! +//! The two conditions answer different questions and neither implies the other. +//! The translation says what the ecosystem *means*; the parse of the raw literal +//! says whether the string that comes back is one a consumer can compare with. A +//! Maven `4.12` passes the first and fails the second: it means exactly `4.12`, but +//! every consumer reads `Node::version` as written, and `Version::parse("4.12")` +//! fails — which `check_version` treats as no locked version at all and answers +//! with a green `ok` against a registry offering `4.13.2`. use ::semver::{Op, Version, VersionReq}; use crate::ecosystem::Ecosystem; -use crate::semver::normalize::{normalize_version, to_semver_constraint}; +use crate::semver::normalize::to_semver_constraint; /// Characters that cannot appear inside a single published version, and whose /// presence means the extracted literal is still part of a range, a union, or a @@ -48,15 +57,25 @@ const NOT_IN_A_VERSION: &[char] = &[ /// (`1.2.+`, `1.*`), a dist-tag (`latest`, `latest.release`), a partial-precision /// exact requirement whose ecosystem leaves the rest free (Cargo `=1.2`), an /// unparseable string, and an unexpanded build-system property -/// (`$(SerilogVersion)`). +/// (`$(SerilogVersion)`). A union of intervals (`[1.0],[2.0]`) is a set too, and +/// is rejected before translation: `maven::interval_range` keeps the last interval +/// of a union, so the translated form would look like a pin. +/// +/// It is also `None` for a pin whose spelling the comparison engine cannot parse +/// **as written** — a four-segment NuGet `1.2.3.4`, a Maven `6.4.4.Final`, and +/// equally a two-segment Maven `4.12`, a NuGet `[1.0]`, or a PEP 440 `==0.20`. +/// Such a version is exact beyond doubt, but every consumer of it compares with +/// `semver::Version` on the declared string, and one that fails to parse is +/// silently treated as *no* version at all: the comparison falls back to the +/// newest compatible release and reports the dependency as up to date. A +/// `junit:junit` pinned at `4.12` would render a green `ok` against a registry +/// offering `4.13.2`. Reporting nothing is honest; reporting a version that turns +/// into a false "ok" downstream is not. /// -/// It is also `None` for a pin whose spelling the comparison engine cannot parse — -/// a four-segment NuGet `1.2.3.4`, a Maven `6.4.4.Final`. Such a version is exact -/// beyond doubt, but every consumer of it compares with `semver::Version`, and one -/// that fails to parse is silently treated as *no* version at all: the comparison -/// falls back to the newest compatible release and reports the dependency as up to -/// date. Reporting nothing is honest; reporting a version that turns into a false -/// "ok" downstream is not. +/// Padding the literal to make it parse is not available: `4.12.0` is a different +/// artifact from `4.12` and the registry may publish neither, so the only string +/// that can be reported is the declared one, and the only test that means anything +/// is whether *that* string parses. /// /// Note that "exact" is the ecosystem's reading, not the string's shape. A bare /// `1.2.3` is an exact version in Maven and Hex, a caret range in Cargo, npm, and @@ -72,8 +91,11 @@ const NOT_IN_A_VERSION: &[char] = &[ /// assert_eq!(exact_pin("=1.2.3", Ecosystem::Rust), Some("1.2.3")); /// assert_eq!(exact_pin("1.2.3", Ecosystem::Rust), None); /// assert_eq!(exact_pin("==2.28.1", Ecosystem::Python), Some("2.28.1")); -/// // The declared spelling, never the translation (`1.0.0` names no artifact here). -/// assert_eq!(exact_pin("1.0", Ecosystem::Jvm), Some("1.0")); +/// // The declared spelling, never the translation (Maven reads a release alias as +/// // contributing nothing, so the translated `1.0.0` names a different artifact). +/// assert_eq!(exact_pin("1.0.0-RELEASE", Ecosystem::Jvm), Some("1.0.0-RELEASE")); +/// // Exact, but not a `semver::Version` as written, so no consumer could use it. +/// assert_eq!(exact_pin("4.12", Ecosystem::Jvm), None); /// ``` #[must_use] pub fn exact_pin(constraint: &str, ecosystem: Ecosystem) -> Option<&str> { @@ -89,8 +111,13 @@ pub fn exact_pin(constraint: &str, ecosystem: Ecosystem) -> Option<&str> { return None; } - // And the literal has to survive the trip every consumer makes with it. - Version::parse(&normalize_version(literal)).ok()?; + // And the literal has to survive the trip every consumer makes with it, which + // is `Version::parse` on the string exactly as written. Nothing downstream + // pads: `declared_pin` puts this straight into a `Node::version`, and the + // renderers, the JSON and DOT emitters, the OSV query, and the TUI's lookup + // all read that field raw. Normalizing here would prove a claim about a string + // no consumer ever constructs. + Version::parse(literal).ok()?; Some(literal) } @@ -131,18 +158,41 @@ mod tests { // Exact, but four segments: `semver::Version` cannot read it, so a // consumer would compare against nothing at all. ("[1.2.3.4]", Ecosystem::CSharp, None), + // Exact, but two segments, and unreadable for the same reason. The + // translation pads to `=1.0.0`; nothing downstream pads, so reporting + // `1.0` would hand every consumer a string it reads as no version. + ("[1.0]", Ecosystem::CSharp, None), + // A union of intervals names a set. `nuget::interval_range` cannot read + // one, but the Maven translator deliberately keeps the *last* interval, + // so both are asserted rather than left to the character screen. + ("[1.0],[2.0]", Ecosystem::CSharp, None), // -- JVM / Maven + Gradle -------------------------------------- ("4.12.0", Ecosystem::Jvm, Some("4.12.0")), ("1.9.24", Ecosystem::Jvm, Some("1.9.24")), ("3.14.0", Ecosystem::Jvm, Some("3.14.0")), - // The declared spelling, not the translation: `maven_to_semver` - // makes this `32.1.3`, which names no artifact on Maven Central. + // A build variant, which `semver::Version` reads as a pre-release and + // Maven keeps: reported as declared, because `32.1.3-android` is a + // different artifact and Maven Central publishes no bare `32.1.3`. ("32.1.3-jre", Ecosystem::Jvm, Some("32.1.3-jre")), + // A release alias the translation drops (`=1.0.0`). The declared + // spelling is what the repository serves, so it is what comes back. + ("1.0.0-RELEASE", Ecosystem::Jvm, Some("1.0.0-RELEASE")), ("[1.0,2.0)", Ecosystem::Jvm, None), ("1.2.+", Ecosystem::Jvm, None), ("latest.release", Ecosystem::Jvm, None), // Exact in Maven, unreadable to `semver::Version`. ("6.4.4.Final", Ecosystem::Jvm, None), + // Exact in Maven and unreadable for the same reason: `junit:junit` is + // published as `4.12`, not `4.12.0`, so the padded form names no + // artifact and the declared form no consumer can parse. Reporting it + // rendered a green `ok` against Maven Central's `4.13.2`. + ("4.12", Ecosystem::Jvm, None), + ("1.0", Ecosystem::Jvm, None), + // `maven::interval_range` uses `rfind`, so it reads a union as its last + // interval and would translate this to a single full-precision `=`. It + // is a set, and stays one. + ("[1.0],[2.0]", Ecosystem::Jvm, None), + ("(,1.0],[1.2,)", Ecosystem::Jvm, None), // -- Rust / Cargo ---------------------------------------------- ("=1.2.3", Ecosystem::Rust, Some("1.2.3")), ("= 1.2.3", Ecosystem::Rust, Some("1.2.3")), @@ -159,7 +209,11 @@ mod tests { ("=1.3.0", Ecosystem::Npm, Some("1.3.0")), // -- Python ----------------------------------------------------- ("==2.28.1", Ecosystem::Python, Some("2.28.1")), - ("==0.20", Ecosystem::Python, Some("0.20")), + // Exact under PEP 440, but `0.20` is not a `semver::Version`. + ("==0.20", Ecosystem::Python, None), + // A local version identifier is part of the distribution's version and + // parses, so it survives even though the translation drops it. + ("==1.2.3+local", Ecosystem::Python, Some("1.2.3+local")), (">=2.0", Ecosystem::Python, None), ("==1.0.*", Ecosystem::Python, None), ("~=1.4.2", Ecosystem::Python, None), @@ -193,12 +247,24 @@ mod tests { /// the manifest wrote. Every row here is one the translation *rewrites*, which /// is what makes returning the translated string a live hazard rather than a /// theoretical one. + /// + /// The rewrites here are all *lossy* ones the guard permits — a Maven build + /// variant, a Maven release alias, a PEP 440 local segment — never a padding of + /// partial precision. Padding is how the translation used to differ, and those + /// inputs are `None` now, so a witness that padded would prove the rule on a + /// case the rule rejects. #[test] fn reports_the_declared_spelling_and_never_the_translation() { for (constraint, ecosystem) in [ - ("1.0", Ecosystem::Jvm), - ("[1.0]", Ecosystem::CSharp), - ("==0.20", Ecosystem::Python), + // Maven reads a release alias as contributing nothing, so this + // translates to a bare `=1.0.0` the repository does not publish. + ("1.0.0-RELEASE", Ecosystem::Jvm), + // PEP 440 local segments are dropped by the translation but are part of + // the version the index serves. + ("==1.2.3+local", Ecosystem::Python), + // The translation re-spells a pre-release into semver's dotted form; + // `1.2.3-rc.1` is not what the index has. + ("==1.2.3-rc1", Ecosystem::Python), ] { let pin = exact_pin(constraint, ecosystem).expect("a pin"); assert!( @@ -261,8 +327,11 @@ mod tests { continue; }; assert!(!pin.is_empty(), "{probe:?} ({ecosystem:?})"); + // Parsed exactly as written — the trip every consumer of a + // `Node::version` makes. Normalizing here would test a string + // nothing downstream ever builds. assert!( - Version::parse(&normalize_version(pin)).is_ok(), + Version::parse(pin).is_ok(), "{probe:?} ({ecosystem:?}) yielded {pin:?}" ); } From 3cf8c323976287cbcddda42330c6628c0afca037 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:45 -0400 Subject: [PATCH 5/7] test(fetch): cover the partial-precision and NuGet pin paths in a graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every graph-level assertion about a manifest pin used a three-segment version. The committed Gradle catalog contains no two-segment entry and the committed csproj contains no `[x.y.z]` interval, so the whole NuGet accept path had no test above `pin.rs` and the partial-precision reject path had none either — the defect in the guard could not have surfaced here. Two cases close that. A catalog holding `4.12` beside `4.12.0` asserts the first resolves nothing and the second resolves, in one file through one code path. A csproj holding `[1.2.3]`, `[1.0]` and `[1.0,2.0)` asserts NuGet's single-version interval is the one spelling in that ecosystem which pins. Also corrects the guava note: `maven_to_semver` keeps the `jre` token, so the translation is not `32.1.3`. What makes the declared spelling the right answer is that Maven Central publishes `32.1.3-jre` and `32.1.3-android` and no bare `32.1.3`. --- .../dependable-fetch/tests/project_graph.rs | 79 ++++++++++++++++++- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/crates/dependable-fetch/tests/project_graph.rs b/crates/dependable-fetch/tests/project_graph.rs index 6d1a62e..19227be 100644 --- a/crates/dependable-fetch/tests/project_graph.rs +++ b/crates/dependable-fetch/tests/project_graph.rs @@ -245,9 +245,9 @@ fn version_of<'g>(graph: &'g DependencyGraph, name: &str) -> Option<&'g str> { /// Asserted per node, with the exact spelling of each, because the two facts worth /// protecting are both about *which* string comes back: /// -/// - `guava` must be `32.1.3-jre` and never the translated `32.1.3`. Maven Central -/// publishes `32.1.3-jre` and `32.1.3-android` and nothing called `32.1.3`, so -/// the translation names no artifact at all. +/// - `guava` must be `32.1.3-jre`, the variant Maven Central actually publishes +/// alongside `32.1.3-android`. Truncating it to `32.1.3` would name no artifact +/// at all, so the declared spelling is what a node carries. /// - `kotlin-stdlib` and `kotlin-reflect` share one `[versions]` alias, which /// reaches them as a *resolved* `Inherited` item. Those are checkable and so /// report the alias's version; an alias no `[versions]` entry defines would not. @@ -292,6 +292,79 @@ fn a_manifest_only_graph_reports_the_versions_the_manifest_settled() { ); } +/// A Maven version does not have to have three segments, and a two-segment one is +/// exact: `junit:junit` is published as `4.12` and there is no `4.12.0`. It still +/// resolves nothing, because a `Node::version` is read as written by everything +/// downstream of it — the renderers, the JSON and DOT emitters, the OSV query, and +/// the TUI's lookup — and `Version::parse("4.12")` fails. `check_version` treats a +/// current version it cannot parse as *no* current version, falls back to the +/// newest release, and answers `UpToDate`, which is a green `ok` for a dependency +/// three releases behind. That is #96's failure verbatim, so the string never +/// enters the graph. +/// +/// The three-segment entry beside it is the control: same file, same code path, +/// and it does resolve. +#[test] +fn a_two_segment_catalog_version_is_exact_and_still_resolves_nothing() { + let dir = TempDir::new().expect("tempdir"); + write( + &dir.path().join("gradle/libs.versions.toml"), + r#" +[versions] +junit = "4.12" +okhttp = "4.12.0" + +[libraries] +junit = { module = "junit:junit", version.ref = "junit" } +okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" } +"#, + ); + + let built = build_project_graph( + &dir.path().join("gradle/libs.versions.toml"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(version_of(&built.graph, "junit:junit"), None); + assert_eq!( + version_of(&built.graph, "com.squareup.okhttp3:okhttp"), + Some("4.12.0") + ); +} + +/// NuGet's single-version interval is the one spelling in that ecosystem which +/// names exactly one release, and until now nothing above `pin.rs` exercised it — +/// the committed C# fixture has no `[x.y.z]` reference at all, so the whole accept +/// path was untested at graph level while the reject path was not. +/// +/// `[1.0]` beside it is exact under NuGet's own reading and still resolves +/// nothing, for the same reason a two-segment Maven version does. +#[test] +fn a_nuget_single_version_interval_resolves_and_a_two_segment_one_does_not() { + let dir = TempDir::new().expect("tempdir"); + write( + &dir.path().join("App.csproj"), + r#" + + + + + +"#, + ); + + let built = build_project_graph( + &dir.path().join("App.csproj"), + &WorkspaceGraphOptions::default(), + ) + .expect("graph"); + + assert_eq!(version_of(&built.graph, "Pinned"), Some("1.2.3")); + assert_eq!(version_of(&built.graph, "PinnedShort"), None); + assert_eq!(version_of(&built.graph, "Ranged"), None); +} + /// The case #107 opened with, and the one this change does **not** close. NuGet /// reads a bare `Version` as an inclusive *minimum* (`>=13.0.1`), not a pin — see /// `nuget_constraint_to_semver`, "A bare version is an inclusive minimum in NuGet" From 416d13ed29c6b675f01a78ea66ebfd6d8f2c8a0d Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:45 -0400 Subject: [PATCH 6/7] test(tui): take a manifest pin through selection into the lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The crate's `unknown` tests build their graphs by hand with `DependencyGraph::from_resolved`, so none of them calls `build_project_graph` and none can reach `exact_pin` at all. "The `dependable-tui` suite passing untouched" was therefore not evidence about this change. These start from a real Gradle catalog on disk, build the project the way `data::discover_projects` does, walk the row through `App::selected_key`, and then make the same `check_version` call `data::lookup` makes with the key that comes out — the whole path a version travels from a manifest to a badge. The second test is the one that would have caught the guard defect: it asserts `junit:junit` at `4.12` carries no version and spawns no lookup, and then runs the call the pipeline *would* have made, showing it answers `UpToDate` against a registry offering `4.13.2`. That false `ok` is why the version must never reach there. `tempfile` joins the crate as a dev-dependency because reaching `build_project_graph` requires a manifest on disk. --- Cargo.lock | 1 + crates/dependable-tui/Cargo.toml | 3 + crates/dependable-tui/tests/pinned_lookup.rs | 129 +++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 crates/dependable-tui/tests/pinned_lookup.rs diff --git a/Cargo.lock b/Cargo.lock index e1eb3f4..58ac74d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -615,6 +615,7 @@ dependencies = [ "globset", "jiff", "ratatui", + "tempfile", "thiserror 2.0.18", "tokio", "tracing", diff --git a/crates/dependable-tui/Cargo.toml b/crates/dependable-tui/Cargo.toml index 508d706..09e1c64 100644 --- a/crates/dependable-tui/Cargo.toml +++ b/crates/dependable-tui/Cargo.toml @@ -15,3 +15,6 @@ jiff.workspace = true tokio.workspace = true thiserror.workspace = true tracing.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/dependable-tui/tests/pinned_lookup.rs b/crates/dependable-tui/tests/pinned_lookup.rs new file mode 100644 index 0000000..eb29514 --- /dev/null +++ b/crates/dependable-tui/tests/pinned_lookup.rs @@ -0,0 +1,129 @@ +//! What the TUI asks a registry about a version the *manifest* pinned. +//! +//! The other `unknown` tests in this crate build their graph by hand with +//! `DependencyGraph::from_resolved`, so none of them can reach `exact_pin` at all. +//! These start from a real Gradle catalog, build the project exactly as +//! `data::discover_projects` does, walk the row through `App::selected_key`, and +//! then make the same `check_version` call `data::lookup` makes with the key that +//! comes out. That is the whole path a version travels from a manifest to a badge. + +use std::fs; + +use dependable_fetch::core::check_version; +use dependable_fetch::{ + DependencyStatus, Ecosystem, ManifestKind, WorkspaceGraphOptions, build_project_graph, +}; +use dependable_tui::app::{Action, App, End}; +use dependable_tui::model::{Project, key}; +use tempfile::TempDir; + +/// A catalog holding one three-segment version and one two-segment one. Both are +/// exact under Maven's reading; only one of them is a `semver::Version` as written. +const CATALOG: &str = r#" +[versions] +okhttp = "4.12.0" +junit = "4.12" + +[libraries] +okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" } +junit = { module = "junit:junit", version.ref = "junit" } +"#; + +/// Build the project the way `data::discover_projects` does: from the file on +/// disk, through `build_project_graph`, with the ecosystem the manifest's own kind +/// reports. +fn catalog_project() -> (TempDir, Project) { + let dir = TempDir::new().expect("tempdir"); + let manifest = dir.path().join("gradle/libs.versions.toml"); + fs::create_dir_all(manifest.parent().expect("parent")).expect("mkdir"); + fs::write(&manifest, CATALOG).expect("write"); + + let kind = ManifestKind::detect(&manifest).expect("a known manifest kind"); + let built = build_project_graph(&manifest, &WorkspaceGraphOptions::default()).expect("graph"); + let project = Project { + label: "gradle/libs.versions.toml".to_owned(), + manifest, + ecosystem: kind.ecosystem(), + graph: built.graph, + source: built.source, + }; + (dir, project) +} + +/// Move the selection onto the row for `name`, expanding as it goes, so the test +/// exercises the same navigation a user performs rather than reaching into state. +fn select_named(app: &mut App, name: &str) { + app.apply(Action::JumpTo(End::Top)); + for _ in 0..64 { + if app.selected().is_some_and(|row| row.name == name) { + return; + } + app.apply(Action::Expand); + app.apply(Action::Move(1)); + } + panic!("no row named {name} in {:?}", rows_of(app)); +} + +fn rows_of(app: &App) -> Vec { + app.rows().iter().map(|row| row.name.clone()).collect() +} + +/// The version a catalog states outright is the version the detail pane asks +/// about — no lockfile involved, and no fallback to "whatever is newest". +#[test] +fn a_pinned_dependency_is_looked_up_at_the_version_the_manifest_named() { + let (_dir, project) = catalog_project(); + assert_eq!(project.ecosystem, Ecosystem::Jvm); + let mut app = App::new(vec![project]); + + select_named(&mut app, "com.squareup.okhttp3:okhttp"); + assert_eq!( + app.selected_key(), + Some(key(Ecosystem::Jvm, "com.squareup.okhttp3:okhttp", "4.12.0")), + "the catalog settled this version, so it is what the lookup is about" + ); + + // The call `data::lookup` makes with that key, against what the registry + // publishes. `4.12.0` is behind `5.0.0`, and the pipeline says so. + let published = ["4.12.0".to_owned(), "5.0.0".to_owned()]; + let evaluation = check_version("*", &published, Some("4.12.0")); + assert_eq!(evaluation.status, DependencyStatus::UpdateAvailable); + assert_eq!(evaluation.latest_available.as_deref(), Some("5.0.0")); +} + +/// The regression this file exists for. `junit:junit` is published as `4.12` — +/// exact beyond doubt, and not a `semver::Version`. Carrying it into the graph put +/// a string into `Node::version` that `check_version` reads as *no* version at +/// all, so the comparison fell back to the newest release and the row rendered a +/// green `ok` for a dependency three releases behind. +/// +/// The second half of the test is that failure, run directly: it is what the +/// pipeline would answer if the version were ever carried, which is why the graph +/// must not carry it and why no lookup is spawned. +#[test] +fn a_pin_the_comparison_engine_cannot_read_is_never_looked_up() { + let (_dir, project) = catalog_project(); + let mut app = App::new(vec![project]); + + select_named(&mut app, "junit:junit"); + assert_eq!( + app.selected().and_then(|row| row.version.clone()), + None, + "a version no consumer can parse is not a version this graph reports" + ); + assert_eq!( + app.selected_key(), + None, + "with nothing to ask about there is no lookup to make" + ); + + let published = ["4.11", "4.12", "4.13", "4.13.1", "4.13.2"].map(str::to_owned); + let evaluation = check_version("*", &published, Some("4.12")); + assert_eq!( + evaluation.status, + DependencyStatus::UpToDate, + "the answer the pipeline gives for an unparseable current version — a \ + false `ok`, which is why the version must never reach here" + ); + assert_eq!(evaluation.latest_available.as_deref(), Some("4.13.2")); +} From d87b77a9b48e419e1b45af9542e06092dcf6a38e Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:45 -0400 Subject: [PATCH 7/7] docs: say which pin spellings a graph node can carry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Node::version` and the README both said a manifest-only dependency carries a version wherever its constraint named one release. That is half the rule: the spelling also has to be one this crate can parse as written, because a `Node::version` no consumer can compare with is read downstream as no version at all. Name the forms that fall out — `4.12`, `1.2.3.4`, `6.4.4.Final` — and why. --- README.md | 11 +++++++---- crates/dependable-core/src/graph.rs | 10 ++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 9d3d1f8..196016e 100644 --- a/README.md +++ b/README.md @@ -416,10 +416,13 @@ admits exactly one release (`serde = "=1.0.200"`, a PEP 440 `==2.28.1`, a NuGet `[1.2.3]`, a bare Gradle or Hex version) has already resolved it, and the version reported is the one the manifest spells, not a normalized form of it. Whether a bare version is a pin is the ecosystem's call, not the string's shape: Cargo, -npm, and Python read `1.2.3` as a range, and NuGet reads it as a lower bound. A -git or path dependency is always `null`, whatever version sits beside it. A -version is never the empty string: a blank one in a lockfile is read as no -version at all. +npm, and Python read `1.2.3` as a range, and NuGet reads it as a lower bound. It +is also `null` where the spelling itself is not a version the comparison engine +can read as written — a two-segment `4.12`, a four-segment `1.2.3.4`, a Maven +`6.4.4.Final` — because reporting one of those would put a string downstream that +every consumer reads as no version at all. A git or path dependency is always +`null`, whatever version sits beside it. A version is never the empty string: a +blank one in a lockfile is read as no version at all. ``` my-app v0.1.0 (workspace) diff --git a/crates/dependable-core/src/graph.rs b/crates/dependable-core/src/graph.rs index ab1c68c..257e77a 100644 --- a/crates/dependable-core/src/graph.rs +++ b/crates/dependable-core/src/graph.rs @@ -42,10 +42,12 @@ pub struct Node { /// member is resolved against nothing, so its declaration *is* its version, /// whether or not a lockfile exists. A dependency in a graph built from /// manifests alone carries one only where its constraint named exactly one - /// release, which is the manifest resolving it rather than constraining it. - /// `None` is everything else: a constraint that admits a set and nothing - /// resolved it, a git or path reference, and a package a lockfile records - /// without a version at all. + /// release *and* spelled it as a version this crate can parse, which is the + /// manifest resolving it rather than constraining it. `None` is everything + /// else: a constraint that admits a set and nothing resolved it, a spelling + /// no consumer of this field could compare with (`4.12`, `6.4.4.Final`), a + /// git or path reference, and a package a lockfile records without a version + /// at all. /// /// An [`Option`] rather than an empty-string sentinel, and never /// `Some("")`: a renderer must be able to say "unknown" rather than evaluate