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/README.md b/README.md index 7928165..196016e 100644 --- a/README.md +++ b/README.md @@ -410,9 +410,19 @@ 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. 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 b5e3e39..257e77a 100644 --- a/crates/dependable-core/src/graph.rs +++ b/crates/dependable-core/src/graph.rs @@ -40,9 +40,14 @@ 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 *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 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..252f531 --- /dev/null +++ b/crates/dependable-core/src/semver/pin.rs @@ -0,0 +1,340 @@ +//! 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 *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::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)`). 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. +/// +/// 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 +/// 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 (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> { + 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, 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) +} + +/// 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), + // 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")), + // 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")), + ("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")), + // 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), + (">=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. + /// + /// 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 [ + // 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!( + 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:?})"); + // 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(pin).is_ok(), + "{probe:?} ({ecosystem:?}) yielded {pin:?}" + ); + } + } + } +} 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..19227be 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`, 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. #[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,197 @@ 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")), + ); +} + +/// 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" +/// — 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")); +} 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")); +}