diff --git a/Cargo.lock b/Cargo.lock index e45e787..2ca65ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -91,6 +91,15 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + [[package]] name = "bumpalo" version = "3.20.2" @@ -111,7 +120,7 @@ checksum = "c06acb4f71407ba205a07cb453211e0e6a67b21904e47f6ba1f9589e38f2e454" dependencies = [ "semver", "serde", - "toml", + "toml 0.8.23", "url", ] @@ -121,14 +130,31 @@ version = "0.1.8" dependencies = [ "anyhow", "cargo-lock", + "cargo_toml", "chrono", "clap", + "flate2", + "glob", + "semver", "serde", "serde_json", + "sha2", + "tar", "tempfile", "ureq", ] +[[package]] +name = "cargo_toml" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82f4b26e751e711a5302649417f2da046dce6391b2ea30a4820f37462314f0b9" +dependencies = [ + "semver", + "serde", + "toml 1.1.5+spec-1.1.0", +] + [[package]] name = "cc" version = "1.2.62" @@ -203,6 +229,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "const-oid" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" + [[package]] name = "cookie" version = "0.18.1" @@ -238,6 +270,15 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" +[[package]] +name = "cpufeatures" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566" +dependencies = [ + "libc", +] + [[package]] name = "crc32fast" version = "1.5.0" @@ -247,6 +288,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "hybrid-array", +] + [[package]] name = "deranged" version = "0.5.8" @@ -256,6 +306,17 @@ dependencies = [ "powerfmt", ] +[[package]] +name = "digest" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2" +dependencies = [ + "block-buffer", + "const-oid", + "crypto-common", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -298,6 +359,16 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -306,12 +377,13 @@ checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" [[package]] name = "flate2" -version = "1.1.9" +version = "1.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" +checksum = "6e634e2e0ebac1ee034020da1ca582e17ffe4e0f5e985823721e168928136dcb" dependencies = [ "crc32fast", "miniz_oxide", + "zlib-rs", ] [[package]] @@ -353,6 +425,12 @@ dependencies = [ "wasip3", ] +[[package]] +name = "glob" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4eba85ea1d0a966a983acd07deee566e67395d2d96b6fb39e62b5a833f1eb0b" + [[package]] name = "hashbrown" version = "0.15.5" @@ -390,6 +468,15 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + [[package]] name = "iana-time-zone" version = "0.1.65" @@ -601,9 +688,9 @@ checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "miniz_oxide" -version = "0.8.9" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fa76a2c86f704bdb222d66965fb3d63269ce38518b83cb0575fca855ebb6316" +checksum = "b63fbc4a50860e98e7b2aa7804ded1db5cbc3aff9193adaff57a6931bf7c4b4c" dependencies = [ "adler2", "simd-adler32", @@ -821,6 +908,26 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + +[[package]] +name = "sha2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "shlex" version = "1.3.0" @@ -879,6 +986,17 @@ dependencies = [ "syn", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "tempfile" version = "3.27.0" @@ -940,11 +1058,26 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" dependencies = [ "serde", - "serde_spanned", - "toml_datetime", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", "toml_edit", ] +[[package]] +name = "toml" +version = "1.1.5+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12c0ba9680044b4ce98d391a62094047eada0d64860b80166c39f4a6b5640785" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned 1.1.1", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 1.0.4", +] + [[package]] name = "toml_datetime" version = "0.6.11" @@ -954,6 +1087,15 @@ dependencies = [ "serde", ] +[[package]] +name = "toml_datetime" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3165f65f62e28e0115a00b2ebdd37eb6f3b641855f9d636d3cd4103767159ad7" +dependencies = [ + "serde_core", +] + [[package]] name = "toml_edit" version = "0.22.27" @@ -962,10 +1104,19 @@ checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" dependencies = [ "indexmap", "serde", - "serde_spanned", - "toml_datetime", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", "toml_write", - "winnow", + "winnow 0.7.15", +] + +[[package]] +name = "toml_parser" +version = "1.1.3+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" +dependencies = [ + "winnow 1.0.4", ] [[package]] @@ -974,6 +1125,18 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" + +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -1322,6 +1485,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "winnow" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" + [[package]] name = "wit-bindgen" version = "0.51.0" @@ -1422,6 +1591,16 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ffae5123b2d3fc086436f8834ae3ab053a283cfac8fe0a0b8eaae044768a4c4" +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix", +] + [[package]] name = "yoke" version = "0.8.2" @@ -1505,6 +1684,12 @@ dependencies = [ "syn", ] +[[package]] +name = "zlib-rs" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34b31d188d9d685a4f9c7b46d6e36631b07058d2cfe190267adce54dc230bf12" + [[package]] name = "zmij" version = "1.0.21" diff --git a/Cargo.toml b/Cargo.toml index 2fdecc8..fe7bcad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,8 +28,14 @@ ureq = { version = "3", features = ["rustls", "json"] } serde = { version = "1", features = ["derive"] } serde_json = "1" anyhow = "1" +semver = "1.0.28" +glob = "0.3.4" +cargo_toml = "1.0.1" [dev-dependencies] +flate2 = "1.1.10" +sha2 = "0.11.0" +tar = "0.4.46" tempfile = "3" # Optimize for small binary size - reduces CI cache/download time diff --git a/README.md b/README.md index 2b8d486..c322ef1 100644 --- a/README.md +++ b/README.md @@ -28,11 +28,33 @@ cargo-oxidate Cargo.lock --min-age-days 14 --max-age-days 730 | `--exclude-missing` | Don't flag packages with unknown publish dates | | `--timeout N` | HTTP timeout in seconds (default: 10) | | `--suggest-fix` | For "too new" violations, suggest `cargo update` commands to downgrade | +| `--include-prerelease` | Consider prerelease versions as suggestion candidates (requires `--suggest-fix`); ordinary SemVer requirements (e.g. `^1.2`) still generally don't match prereleases, so most will still be rejected | | `--cache-path PATH` | Enable response caching at PATH (or set `CARGO_OXIDATE_CACHE_PATH`) | | `--cache-max-age-hours N` | Max age for cached version listings (default: 24) | At least one of `--min-age-days` or `--max-age-days` must be specified. +## `--suggest-fix` + +`--suggest-fix` prints a `cargo update --precise` command for the newest eligible downgrade of +each package that is too new. It checks dependency requirements it can verify from `Cargo.lock` +and workspace manifests. + +A candidate must be old enough, not yanked, older than the locked version, and in its compatible +version zone. The zone keeps the same major version, except that `0.x` keeps the same minor and +`0.0.x` keeps the same patch. Prereleases are excluded unless you pass `--include-prerelease` or +the locked version is itself a prerelease. + +Registry requirements come from the crates.io index. An optional registry declaration that cannot +be confirmed active is shown as unverified. Target-specific registry declarations are enforced. +For local and workspace manifests, the tool does not determine feature or target activation, so it +treats every declared requirement, including optional and target-specific ones, as mandatory. + +Suggestions are best effort. The tool does not run Cargo's resolver or build your project, so Cargo +can still reject a suggested command. Apply suggestions in order, then run the command again and +run your tests. If the tool cannot find an eligible downgrade, it reports the requirement that +blocks one when it knows that requirement. + ## Exit Codes - `0` — No violations found diff --git a/src/api.rs b/src/api.rs index b9ddd8b..0e05858 100644 --- a/src/api.rs +++ b/src/api.rs @@ -2,6 +2,7 @@ use anyhow::Result; use chrono::{DateTime, Duration as ChronoDuration, Utc}; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use std::num::NonZeroU32; use std::path::Path; use std::time::Duration; @@ -30,6 +31,59 @@ pub struct CrateVersionInfo { pub yanked: bool, } +/// One version's record from the crates.io sparse index: its own version +/// string, whether it's yanked, and the requirements it places on its own +/// dependencies (used to check whether a candidate downgrade would still +/// satisfy a dependent). +#[derive(Deserialize, Serialize, Clone)] +pub struct IndexRecord { + pub vers: String, + #[serde(default)] + pub yanked: bool, + #[serde(default)] + pub deps: Vec, +} + +#[derive(Deserialize, Serialize, Clone)] +pub struct IndexDep { + pub name: String, + pub req: String, + #[serde(default)] + pub kind: Option, + #[serde(default)] + pub target: Option, + #[serde(default)] + pub optional: Option, + /// The original crate name, present when `name` is a rename alias. + #[serde(default)] + pub package: Option, + /// The alternate registry this dependency is resolved from, if any. + #[serde(default)] + pub registry: Option, +} + +/// Computes the sparse-index path fragment for a crate name, per the rules +/// at : +/// 1-char names live under `1/`, 2-char under `2/`, 3-char under +/// `3//`, and everything else is split into two two-character +/// prefix directories. Matching is done on the lowercased name. +pub fn sparse_index_path(name: &str) -> String { + let lower = name.to_lowercase(); + match lower.len() { + 1 => format!("1/{lower}"), + 2 => format!("2/{lower}"), + 3 => { + let c0 = &lower[0..1]; + format!("3/{c0}/{lower}") + } + _ => { + let c01 = &lower[0..2]; + let c23 = &lower[2..4]; + format!("{c01}/{c23}/{lower}") + } + } +} + /// Classifies API fetch errors for retry decision-making. #[derive(Debug)] pub enum FetchError { @@ -137,6 +191,9 @@ pub struct CratesIoClient { cache: ResponseCache, cache_max_age_hours: u64, retry_policy: RetryPolicy, + /// Per-run memo of successfully fetched index records, keyed by crate + /// name. Confined to this process; never persisted. + fetched_index_records: HashMap>, } impl CratesIoClient { @@ -166,6 +223,7 @@ impl CratesIoClient { cache: ResponseCache::load(cache_path), cache_max_age_hours, retry_policy, + fetched_index_records: HashMap::new(), } } @@ -197,6 +255,21 @@ impl CratesIoClient { url: &str, subject: &str, ) -> Result, FetchError> { + let Some(body) = self.fetch_body(url, subject)? else { + return Ok(None); + }; + + serde_json::from_slice(&body) + .map(Some) + .map_err(|e| FetchError::Permanent(format!("Failed to parse response {subject}: {e}"))) + } + + /// Issues a GET and classifies HTTP errors, without interpreting the + /// body. Used by `fetch_json` and by the index fetch, whose body is + /// newline-delimited JSON rather than a single document. + /// + /// Returns `Ok(None)` on HTTP 404, same as `fetch_json`. + fn fetch_body(&self, url: &str, subject: &str) -> Result>, FetchError> { let response = self .transport .get(url) @@ -211,11 +284,7 @@ impl CratesIoClient { status if (400..500).contains(&status) => Err(FetchError::Permanent(format!( "Client error {status} {subject}" ))), - _ => serde_json::from_slice(&response.body) - .map(Some) - .map_err(|e| { - FetchError::Permanent(format!("Failed to parse response {subject}: {e}")) - }), + _ => Ok(Some(response.body)), } } @@ -301,6 +370,69 @@ impl CratesIoClient { self.pace(); result } + + fn fetch_index_record_uncached(&self, name: &str) -> Result, FetchError> { + let url = format!("https://index.crates.io/{}", sparse_index_path(name)); + let subject = format!("fetching index record for {name}"); + let Some(body) = self.fetch_body(&url, &subject)? else { + return Ok(vec![]); + }; + let text = String::from_utf8_lossy(&body); + + text.lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| { + serde_json::from_str(line).map_err(|e| { + FetchError::Permanent(format!("Failed to parse index record {subject}: {e}")) + }) + }) + .collect() + } + + /// Fetches every published version's index record for `name` from the + /// crates.io sparse index — one line of JSON per version, each listing + /// that version's own dependency requirements. An unknown crate yields + /// an empty vector rather than an error. + /// + /// A lookup first consults a per-run memo of index records already + /// fetched successfully in this process, keyed by crate name; a memo + /// hit is returned as-is, with no age check. Otherwise, a persisted + /// cache entry satisfies the lookup only when it is fresh and contains + /// a record whose `vers` equals `needed_version`; a non-empty entry + /// that lacks that version (e.g. a dependent published after the + /// cache entry was written) is treated as a miss, same as an empty + /// entry. A miss falls through to the network, and a successful fetch + /// is written to both the memo and the persisted cache. + /// + /// Unlike the other two fetches, this one is not subject to the + /// crates.io API's inter-request pacing: the sparse index is a static + /// endpoint outside that rate limit. + pub fn fetch_index_record( + &mut self, + name: &str, + needed_version: &str, + ) -> Result, FetchError> { + if let Some(records) = self.fetched_index_records.get(name) { + return Ok(records.clone()); + } + + let max_age = ChronoDuration::hours(self.cache_max_age_hours as i64); + + if let Some(records) = self.cache.get_index_records(name, max_age) + && records.iter().any(|r| r.vers == needed_version) + { + return Ok(records); + } + + self.with_retry(|client| { + let result = client.fetch_index_record_uncached(name)?; + client.cache.set_index_records(name, result.clone()); + client + .fetched_index_records + .insert(name.to_string(), result.clone()); + Ok(result) + }) + } } /// Test-only fake `Transport` and URL helpers, shared by this module's own @@ -367,14 +499,19 @@ pub(crate) mod test_support { pub(crate) fn versions_url(name: &str) -> String { format!("https://crates.io/api/v1/crates/{name}") } + + pub(crate) fn index_url(name: &str) -> String { + format!("https://index.crates.io/{}", super::sparse_index_path(name)) + } } #[cfg(test)] mod tests { - use super::test_support::{FakeTransport, ScriptedResponse, versions_url}; + use super::test_support::{FakeTransport, ScriptedResponse, index_url, versions_url}; use super::*; use std::num::NonZeroU32; use std::time::Instant; + use tempfile::tempdir; fn version_url(name: &str, version: &str) -> String { format!("https://crates.io/api/v1/crates/{name}/{version}") @@ -586,4 +723,290 @@ mod tests { assert_eq!(result.len(), 1); assert_eq!(result[0].num, "1.0.0"); } + + #[test] + fn sparse_index_path_prefix_rules() { + assert_eq!(sparse_index_path("a"), "1/a"); + assert_eq!(sparse_index_path("io"), "2/io"); + assert_eq!(sparse_index_path("syn"), "3/s/syn"); + assert_eq!(sparse_index_path("serde"), "se/rd/serde"); + assert_eq!(sparse_index_path("Serde"), "se/rd/serde"); + } + + #[test] + fn fetch_index_record_parses_multiple_lines_with_defaults() { + let url = index_url("serde"); + let transport = FakeTransport::new(); + transport.push( + &url, + ScriptedResponse::Http( + 200, + concat!( + r#"{"vers":"1.0.0","yanked":false,"deps":[{"name":"quote","req":"^1.0"}]}"#, + "\n", + r#"{"vers":"1.0.1","yanked":true}"#, + "\n", + ) + .to_string(), + ), + ); + + let mut client = fast_client(transport); + let records = client.fetch_index_record("serde", "1.0.0").unwrap(); + + assert_eq!(records.len(), 2); + assert_eq!(records[0].vers, "1.0.0"); + assert!(!records[0].yanked); + assert_eq!(records[0].deps.len(), 1); + assert_eq!(records[0].deps[0].name, "quote"); + assert_eq!(records[0].deps[0].req, "^1.0"); + assert_eq!(records[0].deps[0].kind, None); + assert_eq!(records[0].deps[0].package, None); + + assert_eq!(records[1].vers, "1.0.1"); + assert!(records[1].yanked); + assert!(records[1].deps.is_empty()); + } + + #[test] + fn fetch_index_record_missing_crate_yields_empty_result() { + let url = index_url("does-not-exist"); + let transport = FakeTransport::new(); + transport.push(&url, ScriptedResponse::Http(404, String::new())); + + let mut client = fast_client(transport); + let records = client + .fetch_index_record("does-not-exist", "1.0.0") + .unwrap(); + assert!(records.is_empty()); + } + + #[test] + fn empty_sparse_index_result_is_cached_within_one_client() { + let url = index_url("does-not-exist"); + let transport = FakeTransport::new(); + transport.push(&url, ScriptedResponse::Http(404, String::new())); + + let mut client = fast_client(transport); + assert!( + client + .fetch_index_record("does-not-exist", "1.0.0") + .unwrap() + .is_empty() + ); + assert!( + client + .fetch_index_record("does-not-exist", "2.0.0") + .unwrap() + .is_empty() + ); + assert_eq!(client.transport.call_count(), 1); + } + + #[test] + fn blank_sparse_index_response_is_cached_within_one_client() { + let url = index_url("empty-index"); + let transport = FakeTransport::new(); + transport.push(&url, ScriptedResponse::Http(200, " \n\t\n ".to_string())); + + let mut client = fast_client(transport); + assert!( + client + .fetch_index_record("empty-index", "1.0.0") + .unwrap() + .is_empty() + ); + assert!( + client + .fetch_index_record("empty-index", "2.0.0") + .unwrap() + .is_empty() + ); + assert_eq!(client.transport.call_count(), 1); + } + + #[test] + fn empty_sparse_index_result_is_refetched_by_a_new_client() { + // A new client opened against a cache file holding an empty entry + // must not treat that entry as a hit: it issues a request rather + // than reusing the stale empty result. + let dir = tempdir().unwrap(); + let cache_path = dir.path().join("cache.json"); + let url = index_url("does-not-exist"); + let first_transport = FakeTransport::new(); + first_transport.push(&url, ScriptedResponse::Http(404, String::new())); + + let mut first_client = CratesIoClient::with_transport( + first_transport, + Some(&cache_path), + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::ZERO, + pacing_delay: Duration::ZERO, + }, + ); + assert!( + first_client + .fetch_index_record("does-not-exist", "1.0.0") + .unwrap() + .is_empty() + ); + first_client.finish(); + + let second_transport = FakeTransport::new(); + second_transport.push(&url, ScriptedResponse::Http(404, String::new())); + let mut second_client = CratesIoClient::with_transport( + second_transport, + Some(&cache_path), + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::ZERO, + pacing_delay: Duration::ZERO, + }, + ); + assert!( + second_client + .fetch_index_record("does-not-exist", "2.0.0") + .unwrap() + .is_empty() + ); + assert_eq!(second_client.transport.call_count(), 1); + } + + #[test] + fn sparse_index_transport_failure_is_not_cached() { + let url = index_url("retryable"); + let transport = FakeTransport::new(); + transport.push(&url, ScriptedResponse::Error); + transport.push(&url, ScriptedResponse::Http(404, String::new())); + + let mut client = CratesIoClient::with_transport( + transport, + None, + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::ZERO, + pacing_delay: Duration::ZERO, + }, + ); + assert!(matches!( + client.fetch_index_record("retryable", "1.0.0"), + Err(FetchError::Retryable(_)) + )); + assert!( + client + .fetch_index_record("retryable", "1.0.0") + .unwrap() + .is_empty() + ); + assert_eq!(client.transport.call_count(), 2); + } + + #[test] + fn fetch_index_record_cache_hit_issues_no_request() { + let transport = FakeTransport::new(); + let mut client = fast_client(transport); + client.cache.set_index_records( + "serde", + vec![IndexRecord { + vers: "1.0.0".to_string(), + yanked: false, + deps: vec![], + }], + ); + + let records = client.fetch_index_record("serde", "1.0.0").unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].vers, "1.0.0"); + assert_eq!(client.transport.call_count(), 0); + } + + #[test] + fn fetch_index_record_refetches_when_cached_records_miss_needed_version() { + let url = index_url("serde"); + let transport = FakeTransport::new(); + transport.push( + &url, + ScriptedResponse::Http(200, r#"{"vers":"1.0.1","yanked":false}"#.to_string()), + ); + + let mut client = fast_client(transport); + client.cache.set_index_records( + "serde", + vec![IndexRecord { + vers: "1.0.0".to_string(), + yanked: false, + deps: vec![], + }], + ); + + let records = client.fetch_index_record("serde", "1.0.1").unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].vers, "1.0.1"); + assert_eq!(client.transport.call_count(), 1); + } + + #[test] + fn fetch_index_record_missing_version_is_refetched_once_per_run() { + // A non-empty cached entry that lacks the needed version is fetched + // once per run across repeated lookups, not once per lookup. + let url = index_url("serde"); + let transport = FakeTransport::new(); + transport.push( + &url, + ScriptedResponse::Http(200, r#"{"vers":"1.0.1","yanked":false}"#.to_string()), + ); + + let mut client = fast_client(transport); + client.cache.set_index_records( + "serde", + vec![IndexRecord { + vers: "1.0.0".to_string(), + yanked: false, + deps: vec![], + }], + ); + + for _ in 0..2 { + let records = client.fetch_index_record("serde", "1.0.1").unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].vers, "1.0.1"); + } + assert_eq!(client.transport.call_count(), 1); + } + + #[test] + fn fetch_index_record_memo_holds_records_even_with_zero_cache_age() { + // With `cache_max_age_hours = 0`, two lookups in one run issue one + // request and both return the real records. This must fail if the + // memo were a name set that re-reads the cache, since a + // just-written cache entry would then be judged expired. + let url = index_url("serde"); + let transport = FakeTransport::new(); + transport.push( + &url, + ScriptedResponse::Http(200, r#"{"vers":"1.0.0","yanked":false}"#.to_string()), + ); + + let mut client = CratesIoClient::with_transport( + transport, + None, + 0, + RetryPolicy { + retry_count: NonZeroU32::new(3).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ); + + for _ in 0..2 { + let records = client.fetch_index_record("serde", "1.0.0").unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].vers, "1.0.0"); + } + assert_eq!(client.transport.call_count(), 1); + } } diff --git a/src/cache.rs b/src/cache.rs index 0f82414..33cb1e4 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use crate::api::CrateVersionInfo; +use crate::api::{CrateVersionInfo, IndexRecord}; const CACHE_VERSION: u32 = 1; @@ -13,6 +13,8 @@ struct CacheData { version: u32, publish_dates: HashMap>, all_versions: HashMap, + #[serde(default)] + index_records: HashMap, } #[derive(Serialize, Deserialize)] @@ -21,6 +23,12 @@ struct AllVersionsEntry { versions: Vec, } +#[derive(Serialize, Deserialize)] +struct IndexRecordsEntry { + fetched_at: DateTime, + records: Vec, +} + pub struct ResponseCache { path: Option, data: CacheData, @@ -107,6 +115,26 @@ impl ResponseCache { self.dirty = true; } + pub fn get_index_records(&self, name: &str, max_age: Duration) -> Option> { + let entry = self.data.index_records.get(name)?; + let age = Utc::now() - entry.fetched_at; + + if age > max_age { + return None; + } + + Some(entry.records.clone()) + } + + pub fn set_index_records(&mut self, name: &str, records: Vec) { + let entry = IndexRecordsEntry { + fetched_at: Utc::now(), + records, + }; + self.data.index_records.insert(name.to_string(), entry); + self.dirty = true; + } + pub fn save(&self) -> Result<()> { if !self.dirty { return Ok(()); @@ -200,6 +228,23 @@ mod tests { ); } + #[test] + fn empty_index_record_cache_entry_expires() { + let mut cache = ResponseCache::load(None); + cache.set_index_records("does-not-exist", vec![]); + + assert!( + cache + .get_index_records("does-not-exist", Duration::hours(1)) + .is_some_and(|records| records.is_empty()) + ); + assert!( + cache + .get_index_records("does-not-exist", Duration::seconds(-1)) + .is_none() + ); + } + #[test] fn corrupt_file_starts_fresh() { let dir = tempdir().unwrap(); @@ -257,6 +302,34 @@ mod tests { cache.save().unwrap(); } + #[test] + fn cache_file_without_index_records_still_loads() { + let dir = tempdir().unwrap(); + let path = dir.path().join("cache.json"); + + // A cache file as written by a release before index_records existed. + std::fs::write( + &path, + r#"{"version":1,"publish_dates":{"serde/1.0.0":"2020-01-01T00:00:00Z"},"all_versions":{}}"#, + ) + .unwrap(); + + let cache = ResponseCache::load(Some(&path)); + assert_eq!( + cache.get_publish_date("serde", "1.0.0"), + Some( + DateTime::parse_from_rfc3339("2020-01-01T00:00:00Z") + .unwrap() + .with_timezone(&Utc) + ) + ); + assert!( + cache + .get_index_records("serde", Duration::hours(1)) + .is_none() + ); + } + #[test] fn load_wrong_version_starts_fresh() { let dir = tempdir().unwrap(); diff --git a/src/lockfile.rs b/src/lockfile.rs index a7ecf19..c979106 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -1,25 +1,64 @@ use anyhow::{Context, Result}; -use std::path::Path; +use std::path::{Path, PathBuf}; -/// A dependency from a lockfile, checked against the crates.io registry. +/// Resolves a possibly-relative lockfile path against `working_dir`, without +/// touching the filesystem. Used by `load` before it canonicalizes and +/// validates the result. +fn resolve_path(path: &Path, working_dir: &Path) -> PathBuf { + if path.is_absolute() { + path.to_path_buf() + } else { + working_dir.join(path) + } +} + +/// A name/version pair identifying a package, used both for lockfile entries +/// and for the dependency edges between them. `source` is the package's (or +/// dependency edge's) origin — crates.io, an alternate registry, git, or a +/// local path — encoded as `cargo_lock::SourceId`'s canonical string, so +/// same-name/same-version packages from different origins aren't confused +/// for one another. +pub struct PackageRef { + pub name: String, + pub version: String, + pub source: Option, +} + +/// An entry from `Cargo.lock`. Includes path and git packages (not just +/// crates.io ones) so that workspace members can appear as dependents in the +/// requirement graph; `is_registry` tells callers which entries are eligible +/// for the age check itself. pub struct Package { pub name: String, pub version: String, + pub is_registry: bool, + pub source: Option, + pub dependencies: Vec, +} + +/// The validated canonical lockfile path together with its parsed packages. +/// Callers that need the lockfile's directory (e.g. to locate the manifest +/// beside it) should derive it from `path` rather than re-resolving the +/// caller-supplied path themselves, since `path` has already had symlinks +/// and `..` components resolved. +pub struct LoadedLockfile { + pub path: PathBuf, + pub packages: Vec, } -/// Loads the crates.io registry packages from a lockfile. +/// Loads every package recorded in a lockfile, registry and non-registry +/// alike, together with the validated canonical lockfile path. /// /// `path` is the lockfile path as given by the caller (relative or /// absolute), resolved against `working_dir` if relative. Rejects anything /// that is not a regular file within `working_dir` (`..` traversal and -/// symlink escapes included), then keeps only packages sourced from the -/// default registry, since path and git dependencies aren't on crates.io. -pub fn load(path: &Path, working_dir: &Path) -> Result> { - let resolved = if path.is_absolute() { - path.to_path_buf() - } else { - working_dir.join(path) - }; +/// symlink escapes included). +/// +/// `cargo_lock` resolves each dependency edge to a concrete version itself +/// (lockfiles may omit a dependency's version when only one instance of it +/// exists), so every `PackageRef` here already carries one. +pub fn load(path: &Path, working_dir: &Path) -> Result { + let resolved = resolve_path(path, working_dir); // Canonicalize to resolve symlinks and ".." components // (file must exist for canonicalize to succeed) @@ -55,17 +94,27 @@ pub fn load(path: &Path, working_dir: &Path) -> Result> { let packages = lockfile .packages .into_iter() - .filter(|p| { - // Only check packages from crates.io registry - p.source.as_ref().is_some_and(|s| s.is_default_registry()) - }) .map(|p| Package { name: p.name.as_str().to_string(), version: p.version.to_string(), + is_registry: p.source.as_ref().is_some_and(|s| s.is_default_registry()), + source: p.source.as_ref().map(|s| s.to_string()), + dependencies: p + .dependencies + .iter() + .map(|d| PackageRef { + name: d.name.as_str().to_string(), + version: d.version.to_string(), + source: d.source.as_ref().map(|s| s.to_string()), + }) + .collect(), }) .collect(); - Ok(packages) + Ok(LoadedLockfile { + path: canonical, + packages, + }) } #[cfg(test)] @@ -87,6 +136,28 @@ checksum = "0000000000000000000000000000000000000000000000000000000000000000" ) } + fn registry_entry_with_deps(name: &str, version: &str, deps: &[&str]) -> String { + let deps_line = if deps.is_empty() { + String::new() + } else { + let list = deps + .iter() + .map(|d| format!("\"{d}\"")) + .collect::>() + .join(",\n "); + format!("dependencies = [\n {list},\n]\n") + }; + format!( + r#" +[[package]] +name = "{name}" +version = "{version}" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" +{deps_line}"# + ) + } + fn git_entry(name: &str, version: &str) -> String { format!( r#" @@ -108,6 +179,17 @@ version = "{version}" ) } + fn alt_registry_entry(name: &str, version: &str) -> String { + format!( + r#" +[[package]] +name = "{name}" +version = "{version}" +source = "registry+https://example.com/index" +"# + ) + } + fn write_lockfile(dir: &Path, contents: &str) -> std::path::PathBuf { let path = dir.join("Cargo.lock"); std::fs::write(&path, format!("{LOCKFILE_HEADER}{contents}")).unwrap(); @@ -115,7 +197,7 @@ version = "{version}" } #[test] - fn only_crates_io_registry_packages_are_kept() { + fn non_registry_packages_are_kept_with_the_flag_set() { let dir = tempdir().unwrap(); let contents = format!( "{}{}{}", @@ -125,11 +207,165 @@ version = "{version}" ); write_lockfile(dir.path(), &contents); - let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap(); + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + + assert_eq!(packages.len(), 3); + let serde = packages.iter().find(|p| p.name == "serde").unwrap(); + assert!(serde.is_registry); + let rand = packages.iter().find(|p| p.name == "rand").unwrap(); + assert!(!rand.is_registry); + let local = packages.iter().find(|p| p.name == "local-crate").unwrap(); + assert!(!local.is_registry); + } + + #[test] + fn dependency_with_explicit_version_resolves() { + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}", + registry_entry_with_deps("a", "1.0.0", &["b 2.0.0"]), + registry_entry("b", "2.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + let a = packages.iter().find(|p| p.name == "a").unwrap(); + assert_eq!(a.dependencies.len(), 1); + assert_eq!(a.dependencies[0].name, "b"); + assert_eq!(a.dependencies[0].version, "2.0.0"); + } + + #[test] + fn dependency_with_omitted_version_resolves_by_name() { + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}", + registry_entry_with_deps("a", "1.0.0", &["b"]), + registry_entry("b", "2.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + let a = packages.iter().find(|p| p.name == "a").unwrap(); + assert_eq!(a.dependencies.len(), 1); + assert_eq!(a.dependencies[0].name, "b"); + assert_eq!(a.dependencies[0].version, "2.0.0"); + } + + #[test] + fn registry_and_git_packages_carry_distinct_sources() { + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}", + registry_entry("serde", "1.0.0"), + git_entry("serde-fork", "1.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + + let registry = packages.iter().find(|p| p.name == "serde").unwrap(); + let git = packages.iter().find(|p| p.name == "serde-fork").unwrap(); + assert!(registry.source.is_some()); + assert!(git.source.is_some()); + assert_ne!(registry.source, git.source); + } + + #[test] + fn dependency_source_omitted_in_the_lockfile_still_resolves() { + // The dependency line ("b" with no version, no source) is the + // ordinary, unambiguous case: only one "b" package exists. + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}", + registry_entry_with_deps("a", "1.0.0", &["b"]), + registry_entry("b", "2.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + let a = packages.iter().find(|p| p.name == "a").unwrap(); + let b = packages.iter().find(|p| p.name == "b").unwrap(); + assert_eq!(a.dependencies[0].source, b.source); + } + + #[test] + fn dependency_edges_carry_the_selected_packages_resolved_source() { + // Proves the invariant `resolve_dependency_sources` used to + // re-derive: `cargo_lock` already resolves each dependency edge to + // its selected package's source while parsing. A root package + // depends, without source qualification, on a crates.io package, a + // git package, an alternate-registry package, and a path package, + // plus one dangling versioned edge to a package absent from the + // lockfile. + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}{}{}{}", + registry_entry_with_deps( + "root", + "1.0.0", + &[ + "crates-dep 1.0.0", + "git-dep 1.0.0", + "alt-dep 1.0.0", + "path-dep 1.0.0", + "missing-dep 9.9.9", + ], + ), + registry_entry("crates-dep", "1.0.0"), + git_entry("git-dep", "1.0.0"), + alt_registry_entry("alt-dep", "1.0.0"), + path_entry("path-dep", "1.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + let root = packages.iter().find(|p| p.name == "root").unwrap(); + let target = |name: &str| packages.iter().find(|p| p.name == name).unwrap(); + let edge = |name: &str| root.dependencies.iter().find(|d| d.name == name).unwrap(); + + assert_eq!(edge("crates-dep").source, target("crates-dep").source); + // A git edge's resolved source drops the commit hash that the + // target package's own source keeps (`normalize_git_source_for_dependency`), + // so it's still `Some`, but not identical to the target's source. + assert!(edge("git-dep").source.is_some()); + assert_eq!(edge("alt-dep").source, target("alt-dep").source); + assert_eq!(edge("path-dep").source, target("path-dep").source); + assert_eq!(edge("path-dep").source, None); + assert_eq!(edge("missing-dep").source, None); + } + + #[test] + fn source_qualified_edge_keeps_its_declared_source_over_a_same_identity_path_package() { + // A path package and a crates.io package share a name and version. + // The dependent's edge to it is source-qualified, so it must keep + // that declared source rather than resolving to the path package. + let dir = tempdir().unwrap(); + let contents = format!( + "{}{}{}", + registry_entry_with_deps( + "root", + "1.0.0", + &["shared 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)"], + ), + registry_entry("shared", "1.0.0"), + path_entry("shared", "1.0.0"), + ); + write_lockfile(dir.path(), &contents); + + let packages = load(Path::new("Cargo.lock"), dir.path()).unwrap().packages; + let root = packages.iter().find(|p| p.name == "root").unwrap(); + let edge = root + .dependencies + .iter() + .find(|d| d.name == "shared") + .unwrap(); + let registry_shared = packages + .iter() + .find(|p| p.name == "shared" && p.source.is_some()) + .unwrap(); - assert_eq!(packages.len(), 1); - assert_eq!(packages[0].name, "serde"); - assert_eq!(packages[0].version, "1.0.0"); + assert_eq!(edge.source, registry_shared.source); } #[test] diff --git a/src/main.rs b/src/main.rs index 4b50f29..74b7530 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,7 @@ use std::process::ExitCode; mod api; mod cache; mod lockfile; +mod manifest; mod policy; mod report; mod suggest; @@ -46,6 +47,14 @@ struct Cli { #[arg(long, requires = "min_age_days")] suggest_fix: bool, + /// Consider prerelease versions as suggestion candidates (requires --suggest-fix) + /// + /// This only admits prerelease versions as candidates; it does not change + /// requirement matching. Ordinary SemVer requirements (e.g. `^1.2`) still + /// generally do not match prereleases, so most will still be rejected. + #[arg(long, requires = "suggest_fix")] + include_prerelease: bool, + /// Path to the response cache file (enables caching) #[arg(long, env = "CARGO_OXIDATE_CACHE_PATH")] cache_path: Option, @@ -116,7 +125,8 @@ fn run(cli: Cli) -> Result { let working_dir = std::env::current_dir().context("Failed to get current directory")?; // Parse lockfile - let packages = lockfile::load(&cli.cargo_lock, &working_dir)?; + let loaded_lockfile = lockfile::load(&cli.cargo_lock, &working_dir)?; + let packages = loaded_lockfile.packages; // Build API client let mut client = api::CratesIoClient::new( @@ -129,8 +139,11 @@ fn run(cli: Cli) -> Result { let mut violations = Vec::new(); let now = chrono::Utc::now(); - let total = packages.len(); - for (i, pkg) in packages.iter().enumerate() { + let registry_packages: Vec<&lockfile::Package> = + packages.iter().filter(|p| p.is_registry).collect(); + + let total = registry_packages.len(); + for (i, pkg) in registry_packages.iter().enumerate() { if freshness_policy.is_exempt(&pkg.name) { continue; } @@ -150,11 +163,32 @@ fn run(cli: Cli) -> Result { report::print_report(&violations); // Generate suggestions if requested + let has_too_new = violations + .iter() + .any(|v| matches!(v.kind, report::ViolationKind::TooNew(_))); if let Some(min_age) = suggest_min_age - && let Some(suggestions) = - suggest::generate_suggestions(&mut client, &violations, min_age, now) + && has_too_new { - report::print_suggestions(&suggestions); + let lockfile_dir = loaded_lockfile.path.parent().unwrap_or(&working_dir); + + let (direct_requirements, manifest_warnings) = + manifest::load_direct_requirements(lockfile_dir); + for warning in &manifest_warnings { + eprintln!(" Warning: {warning}"); + } + + if let Some(outcomes) = suggest::generate_suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + lockfile_dir, + min_age, + cli.include_prerelease, + now, + ) { + report::print_suggestions(&outcomes); + } } client.finish(); @@ -178,4 +212,27 @@ mod tests { let result = Cli::try_parse_from(["cargo-oxidate", "--suggest-fix", "--min-age-days", "7"]); assert!(result.is_ok()); } + + #[test] + fn include_prerelease_without_suggest_fix_fails_to_parse() { + let result = Cli::try_parse_from([ + "cargo-oxidate", + "--include-prerelease", + "--min-age-days", + "7", + ]); + assert!(result.is_err()); + } + + #[test] + fn include_prerelease_with_suggest_fix_parses() { + let result = Cli::try_parse_from([ + "cargo-oxidate", + "--suggest-fix", + "--min-age-days", + "7", + "--include-prerelease", + ]); + assert!(result.is_ok()); + } } diff --git a/src/manifest.rs b/src/manifest.rs new file mode 100644 index 0000000..95f8a6a --- /dev/null +++ b/src/manifest.rs @@ -0,0 +1,864 @@ +use cargo_toml::{Dependency, DepsSet, Manifest}; +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +/// A manifest registry identity; aliases are not lockfile source URLs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RequirementSource { + /// The default registry or its accepted name/index identities. + CratesIo, + /// An alternate registry alias or raw index, retained for diagnostics. + Registry(String), +} + +/// Registry values Cargo treats as crates.io. +const CRATES_IO_REGISTRY_NAME: &str = "crates-io"; +const CRATES_IO_GIT_INDEX: &str = "https://github.com/rust-lang/crates.io-index"; +const CRATES_IO_SPARSE_INDEX: &str = "sparse+https://index.crates.io/"; + +fn is_crates_io_identity(registry: &str) -> bool { + registry == CRATES_IO_REGISTRY_NAME + || registry == CRATES_IO_GIT_INDEX + || registry == CRATES_IO_SPARSE_INDEX +} + +/// A registry requirement scoped to its declaring package and registry. +/// A missing `declaring_version` is unresolved, never a wildcard. +pub struct DirectRequirement { + pub manifest: PathBuf, + pub declaring_package: String, + pub declaring_version: Option, + pub crate_name: String, + pub req: semver::VersionReq, + pub source: RequirementSource, +} + +/// Collects registry requirements from the root, members, and one-level path +/// dependencies. Members and in-tree, non-excluded path dependencies include +/// dev dependencies. Missing or invalid manifests produce warnings and are +/// excluded. +pub fn load_direct_requirements(lockfile_dir: &Path) -> (Vec, Vec) { + let mut warnings = Vec::new(); + let root_path = lockfile_dir.join("Cargo.toml"); + + if !root_path.is_file() { + warnings.push(format!( + "No Cargo.toml found beside the lockfile at {}; direct dependency requirements were not checked", + lockfile_dir.display() + )); + return (vec![], warnings); + } + + let mut seen = HashSet::new(); + seen.insert(canonical_or(lockfile_dir)); + + // `bool` marks whether the manifest is a workspace member (root or a + // `workspace.members` entry) as opposed to a followed path dependency. + let mut manifests: Vec<(PathBuf, Manifest, bool)> = Vec::new(); + let mut has_workspace = false; + let mut workspace_exclude: Vec = Vec::new(); + match load_manifest(&root_path) { + Ok(root) => { + if let Some(ws) = &root.workspace { + has_workspace = true; + workspace_exclude = ws.exclude.clone(); + for member_dir in expand_members(lockfile_dir, ws) { + if !seen.insert(canonical_or(&member_dir)) { + continue; + } + let member_path = member_dir.join("Cargo.toml"); + match load_manifest(&member_path) { + Ok(m) => manifests.push((member_path, m, true)), + Err(e) => warnings.push(e), + } + } + } + manifests.push((root_path, root, true)); + } + Err(e) => warnings.push(e), + } + + // Follow one level; only in-tree, non-excluded dependencies are members. + let canonical_root_dir = canonical_or(lockfile_dir); + let mut followed = Vec::new(); + for (path, manifest, _) in &manifests { + let base_dir = path.parent().unwrap_or(lockfile_dir); + for dep_dir in path_dependency_dirs(base_dir, manifest) { + if !seen.insert(canonical_or(&dep_dir)) { + continue; + } + let dep_path = dep_dir.join("Cargo.toml"); + let canonical_dep_dir = canonical_or(&dep_dir); + let is_member = has_workspace + && canonical_dep_dir.starts_with(&canonical_root_dir) + && !is_excluded(&canonical_root_dir, &canonical_dep_dir, &workspace_exclude); + match load_manifest(&dep_path) { + Ok(m) => followed.push((dep_path, m, is_member)), + Err(e) => warnings.push(e), + } + } + } + manifests.extend(followed); + + let mut requirements = Vec::new(); + for (path, manifest, is_member) in &manifests { + collect_requirements(path, manifest, *is_member, &mut requirements, &mut warnings); + } + + (requirements, warnings) +} + +fn canonical_or(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + +fn load_manifest(path: &Path) -> Result { + Manifest::from_path(path) + .map_err(|e| format!("Failed to parse manifest {}: {e}", path.display())) +} + +/// Expands `workspace.members` glob patterns against `root_dir`, dropping +/// anything matching `workspace.exclude`. +fn expand_members(root_dir: &Path, workspace: &cargo_toml::Workspace) -> Vec { + let mut dirs = Vec::new(); + + for pattern in &workspace.members { + let full_pattern = root_dir.join(pattern); + let Some(pattern_str) = full_pattern.to_str() else { + continue; + }; + let Ok(paths) = glob::glob(pattern_str) else { + continue; + }; + for entry in paths.flatten() { + if is_excluded(root_dir, &entry, &workspace.exclude) { + continue; + } + dirs.push(entry); + } + } + + dirs +} + +fn is_excluded(root_dir: &Path, member_dir: &Path, exclude: &[String]) -> bool { + let Ok(relative) = member_dir.strip_prefix(root_dir) else { + return false; + }; + exclude.iter().any(|pattern| relative.starts_with(pattern)) +} + +/// Directories of every path dependency declared in `manifest`'s normal, +/// dev, build, or target-specific dependency tables. +fn path_dependency_dirs(base_dir: &Path, manifest: &Manifest) -> Vec { + let mut dirs = Vec::new(); + for deps in all_dep_sets(manifest, true) { + for dep in deps.values() { + if let Some(detail) = dep.detail() + && let Some(rel_path) = &detail.path + { + dirs.push(base_dir.join(rel_path)); + } + } + } + dirs +} + +fn all_dep_sets(manifest: &Manifest, include_dev: bool) -> Vec<&DepsSet> { + let mut sets = vec![&manifest.dependencies, &manifest.build_dependencies]; + if include_dev { + sets.push(&manifest.dev_dependencies); + } + for target in manifest.target.values() { + sets.push(&target.dependencies); + sets.push(&target.build_dependencies); + if include_dev { + sets.push(&target.dev_dependencies); + } + } + sets +} + +fn collect_requirements( + manifest_path: &Path, + manifest: &Manifest, + include_dev: bool, + out: &mut Vec, + warnings: &mut Vec, +) { + // A workspace root without [package] has no crate identity to scope. + let Some(package) = manifest.package.as_ref() else { + return; + }; + let declaring_package = package.name().to_string(); + // Only unresolved `workspace = true` versions fail here; from_path + // already applies resolvable workspace inheritance. + let declaring_version = package.version.get().ok().map(|v| v.to_string()); + + for deps in all_dep_sets(manifest, include_dev) { + for (key, dep) in deps { + // Path and git dependencies aren't registry-versioned. + if let Some(detail) = dep.detail() + && (detail.path.is_some() || detail.git.is_some()) + { + continue; + } + + let crate_name = dep.package().unwrap_or(key).to_string(); + let source = requirement_source(dep); + match dep.try_req() { + Ok(req) => out.push(DirectRequirement { + manifest: manifest_path.to_path_buf(), + declaring_package: declaring_package.clone(), + declaring_version: declaring_version.clone(), + crate_name, + req: req.clone(), + source, + }), + Err(e) => warnings.push(format!( + "Could not determine requirement for {crate_name} in {}: {e}", + manifest_path.display() + )), + } + } + } +} + +/// The registry a dependency declaration names. `dep` has already passed +/// through workspace inheritance by the time it reaches here (`from_path` +/// resolves it), so a `{ workspace = true }` dependency backed by a +/// workspace-level `registry` is read the same way as one declared directly. +fn requirement_source(dep: &Dependency) -> RequirementSource { + match dep + .detail() + .and_then(|d| d.registry.clone().or_else(|| d.registry_index.clone())) + { + Some(registry) if is_crates_io_identity(®istry) => RequirementSource::CratesIo, + Some(registry) => RequirementSource::Registry(registry), + None => RequirementSource::CratesIo, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + fn write(dir: &Path, rel: &str, contents: &str) -> PathBuf { + let path = dir.join(rel); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, contents).unwrap(); + path + } + + fn find<'a>(reqs: &'a [DirectRequirement], name: &str) -> &'a DirectRequirement { + reqs.iter() + .find(|r| r.crate_name == name) + .unwrap_or_else(|| panic!("no requirement collected for {name}")) + } + + #[test] + fn plain_string_requirement() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +serde = "1.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + find(&reqs, "serde").req, + semver::VersionReq::parse("1.0").unwrap() + ); + assert_eq!(find(&reqs, "serde").declaring_package, "root"); + assert_eq!( + find(&reqs, "serde").declaring_version.as_deref(), + Some("0.1.0") + ); + } + + #[test] + fn workspace_inherited_declaring_version_is_resolved() { + // "member"'s own `version` is inherited from the workspace root, not + // written directly — the requirement's declaring identity must + // still resolve to the workspace-supplied version, not go missing. + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["member"] + +[workspace.package] +version = "2.3.4" +"#, + ); + write( + dir.path(), + "member/Cargo.toml", + r#" +[package] +name = "member" +version.workspace = true + +[dependencies] +serde = "1.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + find(&reqs, "serde").declaring_version.as_deref(), + Some("2.3.4") + ); + } + + #[test] + fn unresolvable_declaring_version_is_reported_as_unavailable() { + // "member" inherits its version from the workspace, but the + // workspace root supplies no `[workspace.package]` value for it — + // the version genuinely can't be determined, and must come back as + // `None` rather than some guessed-at value. + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["member"] +"#, + ); + write( + dir.path(), + "member/Cargo.toml", + r#" +[package] +name = "member" +version.workspace = true + +[dependencies] +serde = "1.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!( + !warnings.is_empty(), + "expected a warning about the unresolved workspace field" + ); + assert!( + reqs.is_empty(), + "the member's manifest failed to load, so it collects no requirements" + ); + } + + #[test] + fn detailed_dependency_with_rename() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +my_serde = { package = "serde", version = "1.0" } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!(reqs.len(), 1); + assert_eq!(reqs[0].crate_name, "serde"); + } + + #[test] + fn workspace_inherited_requirement() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["member"] + +[workspace.dependencies] +serde = "1.0" +"#, + ); + write( + dir.path(), + "member/Cargo.toml", + r#" +[package] +name = "member" +version = "0.1.0" + +[dependencies] +serde = { workspace = true } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + find(&reqs, "serde").req, + semver::VersionReq::parse("1.0").unwrap() + ); + } + + #[test] + fn target_specific_table() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[target.'cfg(unix)'.dependencies] +libc = "0.2" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + find(&reqs, "libc").req, + semver::VersionReq::parse("0.2").unwrap() + ); + } + + #[test] + fn workspace_members_glob_with_exclude() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["crates/*"] +exclude = ["crates/skip-me"] +"#, + ); + write( + dir.path(), + "crates/a/Cargo.toml", + r#" +[package] +name = "a" +version = "0.1.0" + +[dependencies] +serde = "1.0" +"#, + ); + write( + dir.path(), + "crates/skip-me/Cargo.toml", + r#" +[package] +name = "skip-me" +version = "0.1.0" + +[dependencies] +rand = "0.8" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert!(reqs.iter().any(|r| r.crate_name == "serde")); + assert!(!reqs.iter().any(|r| r.crate_name == "rand")); + } + + #[test] + fn path_dependency_followed_one_level() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +helper = { path = "helper" } +"#, + ); + write( + dir.path(), + "helper/Cargo.toml", + r#" +[package] +name = "helper" +version = "0.1.0" + +[dependencies] +serde = "1.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert!(reqs.iter().any(|r| r.crate_name == "serde")); + // The path dependency itself is skipped: it's not registry-versioned. + assert!(!reqs.iter().any(|r| r.crate_name == "helper")); + } + + #[test] + fn followed_path_dependency_dev_dependencies_are_skipped() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +helper = { path = "helper" } + +[dev-dependencies] +baz = "3" +"#, + ); + write( + dir.path(), + "helper/Cargo.toml", + r#" +[package] +name = "helper" +version = "0.1.0" + +[dependencies] +foo = "1" + +[dev-dependencies] +bar = "2" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + // helper is not a workspace member, so Cargo ignores its + // dev-dependencies. + assert!(reqs.iter().any(|r| r.crate_name == "foo")); + assert!(!reqs.iter().any(|r| r.crate_name == "bar")); + // The root is a workspace member, so its dev-dependencies are + // still collected. + assert!(reqs.iter().any(|r| r.crate_name == "baz")); + } + + #[test] + fn in_tree_path_dependency_of_workspace_is_a_member() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = [] + +[package] +name = "root" +version = "0.1.0" + +[dependencies] +helper = { path = "helper" } +"#, + ); + write( + dir.path(), + "helper/Cargo.toml", + r#" +[package] +name = "helper" +version = "0.1.0" + +[dev-dependencies] +foo = "=1.9.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + let foo = find(&reqs, "foo"); + assert_eq!(foo.req, semver::VersionReq::parse("=1.9.0").unwrap()); + assert_eq!(foo.declaring_package, "helper"); + } + + #[test] + fn excluded_in_tree_path_dependency_is_not_a_member() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = [] +exclude = ["helper"] + +[package] +name = "root" +version = "0.1.0" + +[dependencies] +helper = { path = "helper" } +"#, + ); + write( + dir.path(), + "helper/Cargo.toml", + r#" +[package] +name = "helper" +version = "0.1.0" + +[dependencies] +bar = "1" + +[dev-dependencies] +foo = "=1.9.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert!(!reqs.iter().any(|r| r.crate_name == "foo")); + assert!(reqs.iter().any(|r| r.crate_name == "bar")); + } + + #[test] + fn out_of_tree_path_dependency_is_not_a_member() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "ws/Cargo.toml", + r#" +[workspace] +members = [] + +[package] +name = "root" +version = "0.1.0" + +[dependencies] +helper = { path = "../helper" } +"#, + ); + write( + dir.path(), + "helper/Cargo.toml", + r#" +[package] +name = "helper" +version = "0.1.0" + +[dev-dependencies] +foo = "=1.9.0" +"#, + ); + + let (reqs, warnings) = load_direct_requirements(&dir.path().join("ws")); + assert!(warnings.is_empty()); + assert!(!reqs.iter().any(|r| r.crate_name == "foo")); + } + + #[test] + fn member_path_dependency_on_root_does_not_duplicate_root_requirements() { + // "member" declares a path dependency back to the workspace root + // (e.g. `app = { path = ".." }`). The root is already collected + // as a workspace member, so following that path dependency must not + // collect it a second time. + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["member"] + +[package] +name = "root" +version = "0.1.0" + +[dependencies] +serde = "1.0" +"#, + ); + write( + dir.path(), + "member/Cargo.toml", + r#" +[package] +name = "member" +version = "0.1.0" + +[dependencies] +app = { path = ".." } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + reqs.iter().filter(|r| r.crate_name == "serde").count(), + 1, + "the root manifest's requirements must be collected exactly once" + ); + } + + #[test] + fn dependency_table_records_registry_identity() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +serde = "1.0" +priv_serde = { package = "serde", version = "1.0", registry = "priv" } + +[dev-dependencies] +rand = { version = "0.8", registry = "priv" } + +[build-dependencies] +libc = { version = "0.2", registry = "priv" } + +[target.'cfg(unix)'.dependencies] +nix = { version = "0.2", registry = "priv" } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + + let crates_io_serde = reqs + .iter() + .find(|r| r.crate_name == "serde" && r.source == RequirementSource::CratesIo) + .expect("plain serde dependency should resolve to crates.io"); + assert_eq!( + crates_io_serde.req, + semver::VersionReq::parse("1.0").unwrap() + ); + + let priv_serde = reqs + .iter() + .find(|r| r.crate_name == "serde" && r.source != RequirementSource::CratesIo) + .expect("renamed serde dependency should record its alternate registry"); + assert_eq!( + priv_serde.source, + RequirementSource::Registry("priv".to_string()) + ); + + for name in ["rand", "libc", "nix"] { + assert_eq!( + find(&reqs, name).source, + RequirementSource::Registry("priv".to_string()), + "{name} should record its alternate registry" + ); + } + } + + #[test] + fn workspace_inherited_registry_metadata_is_preserved() { + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["member"] + +[workspace.dependencies] +serde = { version = "1.0", registry = "priv" } +"#, + ); + write( + dir.path(), + "member/Cargo.toml", + r#" +[package] +name = "member" +version = "0.1.0" + +[dependencies] +serde = { workspace = true } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + assert_eq!( + find(&reqs, "serde").source, + RequirementSource::Registry("priv".to_string()) + ); + } + + #[test] + fn explicit_crates_io_registry_identity_is_recognized() { + // `registry = "crates-io"` is Cargo's reserved name for the default + // registry, and a `registry-index` naming crates.io's own index URL + // directly is the same registry under its literal address. Neither + // is an alternate registry, so both must be `CratesIo`. + let dir = tempdir().unwrap(); + write( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "root" +version = "0.1.0" + +[dependencies] +by_name = { package = "serde", version = "1.0", registry = "crates-io" } +by_git_index = { package = "rand", version = "0.8", registry-index = "https://github.com/rust-lang/crates.io-index" } +by_sparse_index = { package = "libc", version = "0.2", registry-index = "sparse+https://index.crates.io/" } +"#, + ); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty()); + for name in ["serde", "rand", "libc"] { + assert_eq!( + find(&reqs, name).source, + RequirementSource::CratesIo, + "{name} should resolve to crates.io" + ); + } + } + + #[test] + fn missing_manifest_degrades_to_a_warning() { + let dir = tempdir().unwrap(); + + let (reqs, warnings) = load_direct_requirements(dir.path()); + assert!(reqs.is_empty()); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("No Cargo.toml")); + } +} diff --git a/src/policy.rs b/src/policy.rs index 84fc6a7..fd98a06 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -105,6 +105,9 @@ mod tests { Package { name: "serde".to_string(), version: "1.0.0".to_string(), + is_registry: true, + source: None, + dependencies: vec![], } } diff --git a/src/report.rs b/src/report.rs index 1cf8ea4..6c6e250 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,4 +1,4 @@ -use crate::suggest; +use crate::suggest::Outcome; use chrono::{DateTime, Utc}; /// A package's publish date and its resulting age, shared by both @@ -93,27 +93,99 @@ pub fn print_report(violations: &[Violation]) { } } -pub fn print_suggestions(suggestions: &[suggest::Suggestion]) { - if suggestions.is_empty() { - println!("\n⚠️ No compliant versions found for any \"too new\" violations."); - println!(" Consider adding these packages to --exempt if they are trusted.\n"); +pub fn print_suggestions(outcomes: &[Outcome]) { + if outcomes.is_empty() { + println!("\n⚠️ Could not check \"too new\" violations for compliant versions."); + println!(" The registry may have been unreachable, or their versions unparsable.\n"); return; } - println!("\n💡 Suggested fixes for \"too new\" violations:\n"); + let has_suggestion = outcomes + .iter() + .any(|o| matches!(o, Outcome::Suggest { .. })); - for s in suggestions { + if has_suggestion { println!( - " cargo update -p {} --precise {} # {} days old", - s.package, s.suggested_version, s.suggested_age_days + "\n💡 Suggested fixes for \"too new\" violations (apply top to bottom, then re-run):\n" ); + + for outcome in outcomes { + if let Outcome::Suggest { + package_spec, + locked_version, + suggested_version, + suggested_age_days, + unverified_dependents, + .. + } = outcome + { + let annotation = if unverified_dependents.is_empty() { + String::new() + } else { + format!( + " (requirement of {} unverified)", + unverified_dependents.join(", ") + ) + }; + println!( + " cargo update -p {package_spec}@{locked_version} --precise {suggested_version} # {suggested_age_days} days old{annotation}" + ); + } + } } - println!( - r#" - Note: These suggestions pick the newest version that satisfies --min-age-days. - They may not be compatible with your Cargo.toml version requirements. - For transitive dependencies, run `cargo tree -i ` to find the parent. + let blocked: Vec<&Outcome> = outcomes + .iter() + .filter(|o| !matches!(o, Outcome::Suggest { .. })) + .collect(); + + if !blocked.is_empty() { + println!("\n⛔ No compatible compliant version:\n"); + for outcome in blocked { + match outcome { + Outcome::Blocked { + package, + locked_version, + newest_compliant, + blocker, + } => { + let source = match &blocker.version { + Some(v) => format!("{} {v}", blocker.name), + None => blocker.name.clone(), + }; + let also_suggested = if blocker.also_suggested { + format!( + " ({source} also has a suggested downgrade above; applying it may unblock this, so re-run to check)" + ) + } else { + String::new() + }; + println!( + " {package} {locked_version}: newest compliant is {newest_compliant}, but {source} requires {}{also_suggested}", + blocker.req + ); + } + Outcome::NoCompliantVersion { + package, + locked_version, + } => { + println!( + " {package} {locked_version}: no eligible downgrade at least the minimum age old within its compatible range" + ); + } + Outcome::Suggest { .. } => unreachable!(), + } + } + } + + if has_suggestion { + println!( + r#" + Suggestions satisfy, on a best-effort basis, the version requirements verified from + Cargo.lock and your manifests. Requirements marked "unverified" above were not checked + and Cargo may still reject that suggestion. Source compatibility is not verified: build + or test after applying. "# - ); + ); + } } diff --git a/src/suggest.rs b/src/suggest.rs index 855abd1..943f3fd 100644 --- a/src/suggest.rs +++ b/src/suggest.rs @@ -1,318 +1,577 @@ use crate::api::{CrateVersionInfo, CratesIoClient, Transport}; +use crate::lockfile::Package; +use crate::manifest::{DirectRequirement, RequirementSource}; use crate::report::{Violation, ViolationKind}; use chrono::{DateTime, Utc}; +use semver::{Version, VersionReq}; +use std::collections::{HashMap, HashSet}; +use std::path::Path; + +/// A requirement placed by a lockfile dependent or the user's manifest. +pub struct Constraint { + pub blocker_name: String, + pub blocker_version: Option, + pub req: VersionReq, +} + +/// The package and requirement blocking a downgrade. +pub struct Blocker { + pub name: String, + pub version: Option, + pub req: String, + /// Whether this run also suggests downgrading this locked package. + pub also_suggested: bool, +} + +/// The result of checking one "too new" violation. +pub enum Outcome { + Suggest { + package: String, + /// Bare name unless a same-name, same-version package makes Cargo's + /// abbreviated pkgid ambiguous, then `{source}#{package}`. + package_spec: String, + locked_version: String, + suggested_version: String, + suggested_age_days: i64, + unverified_dependents: Vec, + }, + Blocked { + package: String, + locked_version: String, + newest_compliant: String, + blocker: Blocker, + }, + NoCompliantVersion { + package: String, + locked_version: String, + }, +} -pub struct Suggestion { - pub package: String, - pub suggested_version: String, - pub suggested_age_days: i64, +/// Whether `a` and `b` share Cargo's symmetric caret-compatible zone: major, +/// minor when major is zero, or patch when both are zero. +fn same_compatible_zone(a: &Version, b: &Version) -> bool { + if a.major != 0 || b.major != 0 { + a.major == b.major + } else if a.minor != 0 || b.minor != 0 { + a.minor == b.minor + } else { + a.patch == b.patch + } } -/// Finds the newest non-yanked version at least `min_age_days` old as of -/// `now`, together with its age in days. -fn find_compliant_version( +/// Keeps old-enough, non-yanked compatible versions older than `locked`, +/// ordered by semver precedence descending (publish date breaks ties). +/// Excludes prereleases unless allowed or `locked` is one. +fn filter_candidates( versions: &[CrateVersionInfo], + locked: &Version, min_age_days: u64, now: DateTime, -) -> Option<(String, i64)> { + allow_prerelease: bool, +) -> Vec<(Version, i64)> { let min_age_threshold = now - chrono::Duration::days(min_age_days as i64); + let allow_pre = allow_prerelease || !locked.pre.is_empty(); - versions + let mut candidates: Vec<(Version, DateTime, i64)> = versions .iter() .filter(|v| !v.yanked && v.created_at <= min_age_threshold) - .max_by_key(|v| v.created_at) - .map(|v| { + .filter_map(|v| { + let parsed = Version::parse(&v.num).ok()?; + if !allow_pre && !parsed.pre.is_empty() { + return None; + } + if !same_compatible_zone(locked, &parsed) { + return None; + } + // `Version`'s `Ord` breaks precedence ties on build metadata, + // so a version differing from `locked` only in build metadata + // would otherwise slip past a plain `>=` comparison despite + // having equal semantic precedence. `cmp_precedence` follows + // the semver spec instead: it ignores build metadata, and + // orders prereleases below the release they precede. + if parsed.cmp_precedence(locked) != std::cmp::Ordering::Less { + return None; + } let age_days = (now - v.created_at).num_days(); - (v.num.clone(), age_days) + Some((parsed, v.created_at, age_days)) }) -} - -/// Generates suggested compliant versions for every "too new" violation. -/// Returns `None` when there are no "too new" violations, so the caller -/// prints nothing; returns `Some` (possibly empty) once the flow has run. -/// -/// A package whose fetch fails, or which has no compliant version, is -/// simply absent from the result rather than aborting the whole operation. -pub fn generate_suggestions( - client: &mut CratesIoClient, - violations: &[Violation], - min_age_days: u64, - now: DateTime, -) -> Option> { - let too_new: Vec<&Violation> = violations - .iter() - .filter(|v| matches!(v.kind, ViolationKind::TooNew(_))) .collect(); - if too_new.is_empty() { - return None; - } + candidates.sort_by(|(a_ver, a_created, _), (b_ver, b_created, _)| { + b_ver + .cmp_precedence(a_ver) + .then_with(|| b_created.cmp(a_created)) + }); + candidates.into_iter().map(|(v, _, age)| (v, age)).collect() +} - let mut suggestions = Vec::new(); - eprintln!("\nFetching version suggestions..."); - for (i, violation) in too_new.iter().enumerate() { - eprintln!(" [{}/{}] {}", i + 1, too_new.len(), violation.package); +enum WalkResult { + Suggest(Version, i64), + Blocked { + newest_compliant: Version, + blocker: Constraint, + }, + NoCompliantVersion, +} - match client.fetch_all_versions(&violation.package) { - Ok(versions) => { - if let Some((suggested_version, age_days)) = - find_compliant_version(&versions, min_age_days, now) - { - suggestions.push(Suggestion { - package: violation.package.clone(), - suggested_version, - suggested_age_days: age_days, - }); - } - } - Err(e) => { - eprintln!( - "\n Warning: failed to fetch versions for {}: {e}", - violation.package - ); - } +/// Walks `candidates` (already filtered and sorted newest first) looking for +/// the first one every constraint accepts. When none does, reports the +/// newest candidate and the constraint responsible for the block. +fn walk(candidates: Vec<(Version, i64)>, mut constraints: Vec) -> WalkResult { + let Some((newest, _)) = candidates.first() else { + return WalkResult::NoCompliantVersion; + }; + let newest = newest.clone(); + + for (version, age_days) in &candidates { + if constraints.iter().all(|c| c.req.matches(version)) { + return WalkResult::Suggest(version.clone(), *age_days); } } - Some(suggestions) + // Prefer a constraint that rejects every candidate: that one alone makes + // the downgrade impossible. Falling back to the first constraint the + // newest candidate fails only misattributes when no single constraint + // blocks everything — there the block is a genuine combination, and this + // still explains why the newest candidate was rejected. + let blocker_idx = constraints + .iter() + .position(|c| candidates.iter().all(|(v, _)| !c.req.matches(v))) + .or_else(|| constraints.iter().position(|c| !c.req.matches(&newest))) + .expect("newest candidate was rejected, so some constraint must reject it"); + let blocker = constraints.swap_remove(blocker_idx); + WalkResult::Blocked { + newest_compliant: newest, + blocker, + } } -#[cfg(test)] -mod tests { - use super::*; - use chrono::TimeZone; - - fn now() -> DateTime { - Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap() - } +/// Constraints plus dependent labels whose requirements remain unverified. +struct GatheredConstraints { + constraints: Vec, + unverified_dependents: Vec, +} - fn make_version(version: &str, days_ago: i64, yanked: bool) -> CrateVersionInfo { - let created_at = now() - chrono::Duration::days(days_ago); - CrateVersionInfo { - num: version.to_string(), - created_at, - yanked, +/// Dependents keyed by full package identity, including source, so equal name +/// and version from different origins never share constraints. +type DependentsIndex<'a> = HashMap<(&'a str, &'a str, Option<&'a str>), Vec<&'a Package>>; + +/// Packages keyed by name and version for source lookup and pkgid ambiguity. +type NameVersionIndex<'a> = HashMap<(&'a str, &'a str), Vec<&'a Package>>; + +fn build_indexes(all_packages: &[Package]) -> (DependentsIndex<'_>, NameVersionIndex<'_>) { + let mut dependents = DependentsIndex::new(); + let mut names_and_versions = NameVersionIndex::new(); + for pkg in all_packages { + names_and_versions + .entry((pkg.name.as_str(), pkg.version.as_str())) + .or_default() + .push(pkg); + for dep in &pkg.dependencies { + dependents + .entry(( + dep.name.as_str(), + dep.version.as_str(), + dep.source.as_deref(), + )) + .or_default() + .push(pkg); } } + (dependents, names_and_versions) +} - #[test] - fn test_find_compliant_version_basic() { - let versions = vec![ - make_version("1.0.0", 100, false), - make_version("1.1.0", 50, false), - make_version("1.2.0", 20, false), - make_version("1.3.0", 5, false), - ]; - - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_some()); - let (version, age_days) = result.unwrap(); - assert_eq!(version, "1.1.0"); // Newest version older than 30 days - assert_eq!(age_days, 50); - } +/// Count versions within the eligible source, ignoring duplicate edges. +fn eligible_locked_versions(dependent: &Package, name: &str, source: Option<&str>) -> usize { + dependent + .dependencies + .iter() + .filter(|d| d.name == name && d.source.as_deref() == source) + .map(|d| d.version.as_str()) + .collect::>() + .len() +} - #[test] - fn test_find_compliant_version_filters_yanked() { - let versions = vec![ - make_version("1.0.0", 100, false), - make_version("1.1.0", 50, true), // yanked - make_version("1.2.0", 20, false), - ]; - - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_some()); - let (version, _) = result.unwrap(); - assert_eq!(version, "1.0.0"); // Skips yanked 1.1.0 - } +/// A single declaration's parsed requirement plus the evidence the shared +/// attribution policy needs: whether it is definitely active (mandatory) or +/// merely possible (optional/aliased). Target-specific declarations are +/// always mandatory too: Cargo's resolver evaluates every target table +/// regardless of the host platform. Local declarations are always mandatory, +/// since their representation drops that distinction. +struct NormalizedDeclaration { + req: VersionReq, + mandatory: bool, +} - #[test] - fn test_find_compliant_version_no_compliant() { - let versions = vec![ - make_version("1.0.0", 10, false), - make_version("1.1.0", 5, false), - make_version("1.2.0", 2, false), - ]; +/// The shared attribution policy's decision: which parsed requirements are +/// verified enforceable, plus whether the parent must still be marked +/// unverified. Both can hold at once (a mandatory constraint alongside an +/// uncertain declaration). +struct AttributionResult { + enforced: Vec, + unverified: bool, +} - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_none()); +/// Applies the requirement-attribution policy shared by local manifests and +/// registry index records: dedupe by parsed requirement (aliases with an +/// equal requirement count once), decide multi-version ambiguity, and decide +/// which requirements are verified enforceable. +fn attribute_requirements( + declarations: &[NormalizedDeclaration], + locked_versions: usize, +) -> AttributionResult { + let mut deduped: Vec = Vec::new(); + for decl in declarations { + match deduped.iter_mut().find(|existing| existing.req == decl.req) { + Some(existing) => existing.mandatory |= decl.mandatory, + None => deduped.push(NormalizedDeclaration { + req: decl.req.clone(), + mandatory: decl.mandatory, + }), + } } - #[test] - fn test_find_compliant_version_all_yanked() { - let versions = vec![ - make_version("1.0.0", 100, true), - make_version("1.1.0", 50, true), - ]; - - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_none()); + if declarations.is_empty() || locked_versions > 1 && deduped.len() > 1 { + return AttributionResult { + enforced: Vec::new(), + unverified: true, + }; } - #[test] - fn test_find_compliant_version_empty() { - let versions: Vec = vec![]; - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_none()); + let mut enforced = Vec::new(); + let mut unverified = false; + if deduped.iter().any(|d| d.mandatory) { + // A mandatory declaration makes every matching mandatory requirement + // definite; matching uncertain declarations remain annotations. + for d in deduped { + if d.mandatory { + enforced.push(d.req); + } else { + unverified = true; + } + } + } else if let [d] = deduped.as_slice() { + // One uncertain declaration is the unique explanation for the edge. + enforced.push(d.req.clone()); } - #[test] - fn test_find_compliant_version_exact_threshold() { - let versions = vec![ - make_version("1.0.0", 30, false), - make_version("1.1.0", 29, false), - ]; - - let result = find_compliant_version(&versions, 30, now()); - assert!(result.is_some()); - let (version, age_days) = result.unwrap(); - assert_eq!(version, "1.0.0"); // Exactly 30 days should be compliant - assert_eq!(age_days, 30); + if enforced.is_empty() { + unverified = true; } - #[test] - fn test_find_compliant_version_picks_newest_compliant() { - let versions = vec![ - make_version("1.0.0", 100, false), - make_version("1.1.0", 90, false), - make_version("1.2.0", 80, false), - make_version("1.3.0", 70, false), - make_version("1.4.0", 10, false), // Too new - ]; - - let result = find_compliant_version(&versions, 50, now()); - assert!(result.is_some()); - let (version, age_days) = result.unwrap(); - assert_eq!(version, "1.3.0"); // Newest among compliant versions - assert_eq!(age_days, 70); + AttributionResult { + enforced, + unverified, } +} - mod generate_suggestions_tests { - use super::*; - use crate::api::RetryPolicy; - use crate::api::test_support::{FakeTransport, ScriptedResponse, versions_url}; - use crate::report::Aged; - use std::num::NonZeroU32; - use std::time::Duration; - - trait FakeTransportExt { - fn ok(&self, name: &str, versions_json: &str); - fn error(&self, name: &str); - } - - impl FakeTransportExt for FakeTransport { - fn ok(&self, name: &str, versions_json: &str) { - self.push( - &versions_url(name), - ScriptedResponse::Http(200, versions_json.to_string()), - ); - } - - fn error(&self, name: &str) { - self.push(&versions_url(name), ScriptedResponse::Error); - } - } +/// Gathers requirements from registry dependents and local manifests. +#[allow(clippy::too_many_arguments)] +fn gather_constraints( + client: &mut CratesIoClient, + dependents_index: &DependentsIndex, + direct_requirements: &[DirectRequirement], + working_dir: &Path, + name: &str, + locked_version: &str, + source: Option<&str>, +) -> GatheredConstraints { + let mut constraints = Vec::new(); + let mut unverified_dependents = Vec::new(); + + let dependents = dependents_index + .get(&(name, locked_version, source)) + .into_iter() + .flatten() + .copied(); + + for dependent in dependents { + let unverified = if dependent.is_registry { + let result = + registry_dependent_constraints(client, dependent, name, locked_version, source); + let unverified = result.unverified; + constraints.extend(result.constraints); + unverified + } else if dependent.source.is_none() { + // Only an unsourced non-registry dependent is local; other + // sources cannot be verified through a workspace manifest. + // Match the declaring package's name and version, and only its + // crates.io declarations, to avoid unrelated local requirements. + let matching: Vec<&DirectRequirement> = direct_requirements + .iter() + .filter(|r| r.crate_name == name && r.declaring_package == dependent.name) + .filter(|r| r.declaring_version.as_deref() == Some(dependent.version.as_str())) + .filter(|r| r.source == RequirementSource::CratesIo) + .filter(|r| { + Version::parse(locked_version).is_ok_and(|version| r.req.matches(&version)) + }) + .collect(); - fn versions_body(entries: &[(&str, i64, bool)], now: DateTime) -> String { - let versions: Vec = entries + let declarations: Vec = matching .iter() - .map(|(num, days_ago, yanked)| { - let created_at = now - chrono::Duration::days(*days_ago); - format!( - r#"{{"num":"{num}","created_at":"{}","yanked":{yanked}}}"#, - created_at.to_rfc3339() - ) + .map(|r| NormalizedDeclaration { + req: r.req.clone(), + mandatory: true, }) .collect(); - format!(r#"{{"versions":[{}]}}"#, versions.join(",")) + let locked_versions = eligible_locked_versions(dependent, name, source); + let result = attribute_requirements(&declarations, locked_versions); + + if result.unverified { + true + } else { + // Every local declaration is mandatory, so on this + // non-ambiguous path `result.enforced` always contains every + // deduped requirement from `matching`; no filter is needed to + // decide which of `matching` to keep. + for req in &matching { + constraints.push(Constraint { + blocker_name: manifest_label(&req.manifest, working_dir), + blocker_version: None, + req: req.req.clone(), + }); + } + false + } + } else { + // Git and alternate-registry requirements cannot be read here. + true + }; + if unverified && !unverified_dependents.contains(&dependent.name) { + unverified_dependents.push(dependent.name.clone()); } + } - /// Builds a client with retry/pacing delays zeroed out, so the test - /// suite doesn't sleep. - fn fast_client(transport: FakeTransport) -> CratesIoClient { - CratesIoClient::with_transport( - transport, - None, - 24, - RetryPolicy { - retry_count: NonZeroU32::new(1).unwrap(), - retry_delay: Duration::from_millis(0), - pacing_delay: Duration::from_millis(0), - }, - ) - } + GatheredConstraints { + constraints, + unverified_dependents, + } +} - fn too_new(package: &str) -> Violation { - Violation { - package: package.to_string(), - version: "1.0.0".to_string(), - kind: ViolationKind::TooNew(Aged { - published: now(), - age_days: 1, - }), - } +/// Requirements a registry dependent's crates.io index record places on +/// `name` at `locked_version`, plus whether an uncertain declaration remains. +struct RegistryConstraints { + constraints: Vec, + unverified: bool, +} + +impl RegistryConstraints { + /// The index record for the dependent (or the matching version within + /// it) couldn't be read at all, so nothing can be enforced. + fn unreadable() -> Self { + RegistryConstraints { + constraints: Vec::new(), + unverified: true, } + } +} - fn too_old(package: &str) -> Violation { - Violation { - package: package.to_string(), - version: "1.0.0".to_string(), - kind: ViolationKind::TooOld(Aged { - published: now(), - age_days: 1000, - }), +fn registry_dependent_constraints( + client: &mut CratesIoClient, + dependent: &Package, + name: &str, + locked_version: &str, + source: Option<&str>, +) -> RegistryConstraints { + let Ok(records) = client.fetch_index_record(&dependent.name, &dependent.version) else { + return RegistryConstraints::unreadable(); + }; + let Some(record) = records.iter().find(|r| r.vers == dependent.version) else { + return RegistryConstraints::unreadable(); + }; + + let mut declarations = Vec::new(); + let mut unverified = false; + for dep in &record.deps { + if dep.kind.as_deref() == Some("dev") { + continue; + } + if dep.registry.is_some() { + continue; + } + let real_name = dep.package.as_deref().unwrap_or(&dep.name); + if real_name != name { + continue; + } + match VersionReq::parse(&dep.req) { + Ok(req) if Version::parse(locked_version).is_ok_and(|v| req.matches(&v)) => { + let mandatory = dep.optional != Some(true); + declarations.push(NormalizedDeclaration { req, mandatory }); } + Ok(_) => {} + Err(_) => unverified = true, } + } - #[test] - fn only_too_new_violations_are_fetched() { - let transport = FakeTransport::default(); - transport.ok("serde", &versions_body(&[("1.0.0", 50, false)], now())); - // "syn" has no scripted response: if it were fetched, the - // transport would panic. - let mut client = fast_client(transport); + let locked_versions = eligible_locked_versions(dependent, name, source); + let result = attribute_requirements(&declarations, locked_versions); - let violations = vec![too_new("serde"), too_old("syn")]; - let suggestions = generate_suggestions(&mut client, &violations, 30, now()).unwrap(); + let constraints = result + .enforced + .into_iter() + .map(|req| Constraint { + blocker_name: dependent.name.clone(), + blocker_version: Some(dependent.version.clone()), + req, + }) + .collect(); - assert_eq!(suggestions.len(), 1); - assert_eq!(suggestions[0].package, "serde"); - } + RegistryConstraints { + unverified: unverified || result.unverified, + constraints, + } +} - #[test] - fn a_failed_fetch_does_not_abort_the_others() { - let transport = FakeTransport::default(); - transport.error("serde"); - transport.ok("syn", &versions_body(&[("1.0.0", 50, false)], now())); - let mut client = fast_client(transport); +fn manifest_label(path: &Path, working_dir: &Path) -> String { + path.strip_prefix(working_dir) + .unwrap_or(path) + .display() + .to_string() +} - let violations = vec![too_new("serde"), too_new("syn")]; - let suggestions = generate_suggestions(&mut client, &violations, 30, now()).unwrap(); +/// Uses a source-qualified pkgid when a shared name and version is ambiguous. +fn build_package_spec(name: &str, target_source: Option<&str>, is_ambiguous: bool) -> String { + match (is_ambiguous, target_source) { + (true, Some(source)) => format!("{source}#{name}"), + _ => name.to_string(), + } +} - assert_eq!(suggestions.len(), 1); - assert_eq!(suggestions[0].package, "syn"); - } +/// Generates outcomes for "too new" violations, or `None` when there are none. +/// Ignores packages whose version list cannot be fetched. +#[allow(clippy::too_many_arguments)] +pub fn generate_suggestions( + client: &mut CratesIoClient, + violations: &[Violation], + all_packages: &[Package], + direct_requirements: &[DirectRequirement], + working_dir: &Path, + min_age_days: u64, + allow_prerelease: bool, + now: DateTime, +) -> Option> { + let too_new: Vec<&Violation> = violations + .iter() + .filter(|v| matches!(v.kind, ViolationKind::TooNew(_))) + .collect(); - #[test] - fn no_compliant_version_produces_no_suggestion() { - let transport = FakeTransport::default(); - transport.ok("serde", &versions_body(&[("1.0.0", 5, false)], now())); - let mut client = fast_client(transport); + if too_new.is_empty() { + return None; + } - let violations = vec![too_new("serde")]; - let suggestions = generate_suggestions(&mut client, &violations, 30, now()).unwrap(); + let (dependents_index, name_version_index) = build_indexes(all_packages); - assert!(suggestions.is_empty()); - } + let mut outcomes = Vec::new(); + eprintln!("\nFetching version suggestions..."); + for (i, violation) in too_new.iter().enumerate() { + eprintln!(" [{}/{}] {}", i + 1, too_new.len(), violation.package); - #[test] - fn no_too_new_violations_yields_none() { - let transport = FakeTransport::default(); - let mut client = fast_client(transport); + let Ok(locked) = Version::parse(&violation.version) else { + eprintln!( + "\n Warning: failed to parse locked version for {}: {}", + violation.package, violation.version + ); + continue; + }; - let violations = vec![too_old("syn")]; - let suggestions = generate_suggestions(&mut client, &violations, 30, now()); + let versions = match client.fetch_all_versions(&violation.package) { + Ok(versions) => versions, + Err(e) => { + eprintln!( + "\n Warning: failed to fetch versions for {}: {e}", + violation.package + ); + continue; + } + }; + + let same_name_version = name_version_index + .get(&(violation.package.as_str(), violation.version.as_str())) + .map(Vec::as_slice) + .unwrap_or_default(); + + // This registry package is the violation target, even if another + // source shares its name and version. + let target_source = same_name_version + .iter() + .find(|p| p.is_registry) + .and_then(|p| p.source.as_deref()); + + let package_spec = build_package_spec( + &violation.package, + target_source, + same_name_version.len() > 1, + ); + + let gathered = gather_constraints( + client, + &dependents_index, + direct_requirements, + working_dir, + &violation.package, + &violation.version, + target_source, + ); + let candidates = filter_candidates(&versions, &locked, min_age_days, now, allow_prerelease); + + let outcome = match walk(candidates, gathered.constraints) { + WalkResult::Suggest(version, age_days) => Outcome::Suggest { + package: violation.package.clone(), + package_spec, + locked_version: violation.version.clone(), + suggested_version: version.to_string(), + suggested_age_days: age_days, + unverified_dependents: gathered.unverified_dependents, + }, + WalkResult::Blocked { + newest_compliant, + blocker, + } => Outcome::Blocked { + package: violation.package.clone(), + locked_version: violation.version.clone(), + newest_compliant: newest_compliant.to_string(), + blocker: Blocker { + // Whether this blocker's own locked version resolved to a + // suggestion is only known once every violation has been + // walked, so this starts false and is patched below. + also_suggested: false, + name: blocker.blocker_name, + version: blocker.blocker_version, + req: blocker.req.to_string(), + }, + }, + WalkResult::NoCompliantVersion => Outcome::NoCompliantVersion { + package: violation.package.clone(), + locked_version: violation.version.clone(), + }, + }; + outcomes.push(outcome); + } - assert!(suggestions.is_none()); + // Keyed by locked version as well as name. A suggestion for one locked + // version of a package says nothing about another version of it, which + // may have no compliant version at all. + let suggested: HashSet<(String, String)> = outcomes + .iter() + .filter_map(|o| match o { + Outcome::Suggest { + package, + locked_version, + .. + } => Some((package.clone(), locked_version.clone())), + _ => None, + }) + .collect(); + for outcome in &mut outcomes { + if let Outcome::Blocked { blocker, .. } = outcome + && let Some(version) = &blocker.version + { + blocker.also_suggested = suggested.contains(&(blocker.name.clone(), version.clone())); } } + + Some(outcomes) } + +#[cfg(test)] +mod tests; diff --git a/src/suggest/tests.rs b/src/suggest/tests.rs new file mode 100644 index 0000000..61963bb --- /dev/null +++ b/src/suggest/tests.rs @@ -0,0 +1,59 @@ +use super::*; +use crate::lockfile::PackageRef; +use chrono::TimeZone; + +fn now() -> DateTime { + Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap() +} + +/// Calls production `generate_suggestions` with the fixed 30-day +/// minimum age, no prerelease admission, and `now()` that almost every +/// call site in this module shares, unwrapping the `Some` result. +fn suggestions( + client: &mut CratesIoClient, + violations: &[Violation], + packages: &[Package], + direct_requirements: &[DirectRequirement], + working_dir: &Path, +) -> Vec { + generate_suggestions( + client, + violations, + packages, + direct_requirements, + working_dir, + 30, + false, + now(), + ) + .unwrap() +} + +fn v(s: &str) -> Version { + Version::parse(s).unwrap() +} + +fn make_version(version: &str, days_ago: i64, yanked: bool) -> CrateVersionInfo { + let created_at = now() - chrono::Duration::days(days_ago); + CrateVersionInfo { + num: version.to_string(), + created_at, + yanked, + } +} + +fn constraint(req: &str) -> Constraint { + Constraint { + blocker_name: "dep".to_string(), + blocker_version: Some("1.0.0".to_string()), + req: VersionReq::parse(req).unwrap(), + } +} + +mod end_to_end_tests; +mod filter_candidates_tests; +mod generate_suggestions_tests; +mod manifest_dependent_identity_tests; +mod manifest_registry_identity_tests; +mod source_collision_cargo_tests; +mod walk_tests; diff --git a/src/suggest/tests/end_to_end_tests.rs b/src/suggest/tests/end_to_end_tests.rs new file mode 100644 index 0000000..1111cf9 --- /dev/null +++ b/src/suggest/tests/end_to_end_tests.rs @@ -0,0 +1,169 @@ +/// Drives the whole pipeline — lockfile intake, manifest reading, and +/// `generate_suggestions` — over the committed fixture at +/// `tests/fixtures/suggest_fix_e2e/`, a two-member workspace-ish layout +/// (a real workspace with one member) plus a hand-written lockfile. +/// Covers all four outcome kinds at once: a suggestion, a package +/// blocked by a manifest requirement, one blocked by a transitive +/// dependent, and one with nothing old enough in range. +use super::*; +use crate::api::RetryPolicy; +use crate::api::test_support::{FakeTransport, ScriptedResponse, index_url, versions_url}; +use crate::manifest::load_direct_requirements; +use crate::report::Aged; +use std::num::NonZeroU32; +use std::path::PathBuf; +use std::time::Duration; + +fn fixture_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/suggest_fix_e2e") +} + +fn versions_body(entries: &[(&str, i64, bool)], now: DateTime) -> String { + let versions: Vec = entries + .iter() + .map(|(num, days_ago, yanked)| { + let created_at = now - chrono::Duration::days(*days_ago); + format!( + r#"{{"num":"{num}","created_at":"{}","yanked":{yanked}}}"#, + created_at.to_rfc3339() + ) + }) + .collect(); + format!(r#"{{"versions":[{}]}}"#, versions.join(",")) +} + +fn too_new(package: &str, locked_version: &str) -> Violation { + Violation { + package: package.to_string(), + version: locked_version.to_string(), + kind: ViolationKind::TooNew(Aged { + published: now() - chrono::Duration::days(5), + age_days: 5, + }), + } +} + +#[test] +fn covers_a_suggestion_two_blocked_kinds_and_no_compliant_version() { + let dir = fixture_dir(); + let packages = crate::lockfile::load(Path::new("Cargo.lock"), &dir) + .unwrap() + .packages; + let (direct_requirements, warnings) = load_direct_requirements(&dir); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("alpha"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ), + ); + transport.push( + &versions_url("beta"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ), + ); + transport.push( + &versions_url("gamma"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ), + ); + transport.push( + &versions_url("delta"), + ScriptedResponse::Http(200, versions_body(&[("1.5.0", 5, false)], now())), + ); + transport.push( + &index_url("consumer"), + ScriptedResponse::Http( + 200, + r#"{"vers":"2.0.0","deps":[{"name":"gamma","req":"^1.5"}]}"#.to_string(), + ), + ); + + let mut client = CratesIoClient::with_transport( + transport, + None, + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ); + + let violations = vec![ + too_new("alpha", "1.5.0"), + too_new("beta", "1.5.0"), + too_new("gamma", "1.5.0"), + too_new("delta", "1.5.0"), + ]; + + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + &dir, + ); + + assert_eq!(outcomes.len(), 4); + + match &outcomes[0] { + Outcome::Suggest { + package, + suggested_version, + suggested_age_days, + unverified_dependents, + .. + } => { + assert_eq!(package, "alpha"); + assert_eq!(suggested_version, "1.4.0"); + assert_eq!(*suggested_age_days, 50); + assert!(unverified_dependents.is_empty()); + } + _ => panic!("expected alpha to be Suggest"), + } + + match &outcomes[1] { + Outcome::Blocked { + package, + newest_compliant, + blocker, + .. + } => { + assert_eq!(package, "beta"); + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app/Cargo.toml"); + assert_eq!(blocker.version, None); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected beta to be Blocked"), + } + + match &outcomes[2] { + Outcome::Blocked { + package, + newest_compliant, + blocker, + .. + } => { + assert_eq!(package, "gamma"); + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "consumer"); + assert_eq!(blocker.version, Some("2.0.0".to_string())); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected gamma to be Blocked"), + } + + assert!(matches!( + &outcomes[3], + Outcome::NoCompliantVersion { package, .. } if package == "delta" + )); +} diff --git a/src/suggest/tests/filter_candidates_tests.rs b/src/suggest/tests/filter_candidates_tests.rs new file mode 100644 index 0000000..9b3e4e2 --- /dev/null +++ b/src/suggest/tests/filter_candidates_tests.rs @@ -0,0 +1,136 @@ +use super::*; + +#[test] +fn excludes_too_new_yanked_and_out_of_range() { + let versions = vec![ + make_version("1.0.0", 100, false), + make_version("1.1.0", 50, true), // yanked + make_version("1.2.0", 40, false), // compliant, same major as locked + make_version("0.9.0", 200, false), // different major: out of range + make_version("1.3.0", 5, false), // too new (min age 30) + ]; + + let result = filter_candidates(&versions, &v("1.5.0"), 30, now(), false); + let nums: Vec = result.iter().map(|(ver, _)| ver.to_string()).collect(); + // Newest-first by semver precedence among the two survivors. + assert_eq!(nums, vec!["1.2.0".to_string(), "1.0.0".to_string()]); +} + +#[test] +fn sorted_by_semver_precedence_descending() { + let versions = vec![ + make_version("1.0.0", 100, false), + make_version("1.1.0", 200, false), + make_version("1.2.0", 50, false), + ]; + + let result = filter_candidates(&versions, &v("1.5.0"), 30, now(), false); + let nums: Vec = result.iter().map(|(ver, _)| ver.to_string()).collect(); + assert_eq!(nums, vec!["1.2.0", "1.1.0", "1.0.0"]); +} + +#[test] +fn higher_semver_precedence_wins_over_more_recent_publish_date() { + // 1.4.0 outranks 1.3.9 by semver even though 1.3.9 was published + // more recently: ordering by precedence must win over publish date. + let versions = vec![ + make_version("1.4.0", 100, false), + make_version("1.3.9", 40, false), + ]; + + let result = filter_candidates(&versions, &v("1.5.0"), 30, now(), false); + let nums: Vec = result.iter().map(|(ver, _)| ver.to_string()).collect(); + assert_eq!(nums, vec!["1.4.0", "1.3.9"]); +} + +#[test] +fn publish_date_breaks_ties_in_equal_semver_precedence() { + // Build metadata doesn't affect precedence, so these two versions tie + // under `cmp_precedence`; publish date must decide the order, with + // the more recently published one (build.2, 50 days ago) first. + let versions = vec![ + make_version("1.3.0+build.1", 100, false), + make_version("1.3.0+build.2", 50, false), + ]; + + let result = filter_candidates(&versions, &v("1.5.0"), 30, now(), false); + let nums: Vec = result.iter().map(|(ver, _)| ver.to_string()).collect(); + assert_eq!(nums, vec!["1.3.0+build.2", "1.3.0+build.1"]); +} + +#[test] +fn prerelease_excluded_by_default() { + let versions = vec![make_version("1.1.0-beta.1", 100, false)]; + let result = filter_candidates(&versions, &v("1.2.0"), 30, now(), false); + assert!(result.is_empty()); +} + +#[test] +fn prerelease_included_with_flag_when_range_matches() { + // Same compatible zone (1.0.0), prerelease allowed by the flag. + let versions = vec![make_version("1.0.0-beta.1", 100, false)]; + let result = filter_candidates(&versions, &v("1.0.0"), 30, now(), true); + assert_eq!(result.len(), 1); + assert_eq!(result[0].0.to_string(), "1.0.0-beta.1"); +} + +#[test] +fn prerelease_allowed_when_locked_is_itself_a_prerelease() { + let versions = vec![make_version("1.0.0-beta.1", 100, false)]; + let result = filter_candidates(&versions, &v("1.0.0-beta.2"), 30, now(), false); + assert_eq!(result.len(), 1); +} + +#[test] +fn excludes_a_higher_version_published_earlier_than_locked() { + // "1.4.0" was published before "1.3.0" but is a higher semantic + // version, so it must never be offered as a downgrade even + // though it's older on the publish timeline. + let versions = vec![ + make_version("1.4.0", 100, false), + make_version("1.3.0", 50, false), + ]; + let result = filter_candidates(&versions, &v("1.3.0"), 30, now(), false); + let nums: Vec = result.iter().map(|(ver, _)| ver.to_string()).collect(); + assert!(nums.is_empty(), "expected no candidates, got {nums:?}"); +} + +#[test] +fn excludes_a_version_equal_in_precedence_including_build_metadata_only_differences() { + let versions = vec![ + make_version("1.3.0", 100, false), + make_version("1.3.0+build.1", 100, false), + ]; + let result = filter_candidates(&versions, &v("1.3.0"), 30, now(), false); + assert!(result.is_empty()); +} + +#[test] +fn excludes_versions_equal_in_precedence_to_a_locked_version_with_build_metadata() { + // Locked itself carries build metadata this time: candidates + // differing only in build metadata (or lacking it) still have + // equal semantic precedence and must not be offered. + let versions = vec![ + make_version("1.3.0", 100, false), + make_version("1.3.0+build.1", 100, false), + ]; + let result = filter_candidates(&versions, &v("1.3.0+build.2"), 30, now(), false); + assert!(result.is_empty()); +} + +#[test] +fn excludes_stable_release_above_a_locked_prerelease() { + // A stable release outranks any prerelease of the same + // major.minor.patch, so it must not be offered as a "downgrade" + // from a locked prerelease. + let versions = vec![make_version("1.0.0", 100, false)]; + let result = filter_candidates(&versions, &v("1.0.0-beta.1"), 30, now(), false); + assert!(result.is_empty()); +} + +#[test] +fn excludes_a_later_prerelease_above_a_locked_prerelease() { + let versions = vec![make_version("1.0.0-beta.2", 100, false)]; + let result = filter_candidates(&versions, &v("1.0.0-beta.1"), 30, now(), false); + assert!(result.is_empty()); +} diff --git a/src/suggest/tests/generate_suggestions_tests.rs b/src/suggest/tests/generate_suggestions_tests.rs new file mode 100644 index 0000000..a53aac1 --- /dev/null +++ b/src/suggest/tests/generate_suggestions_tests.rs @@ -0,0 +1,1567 @@ +use super::*; +use crate::api::RetryPolicy; +use crate::api::test_support::{FakeTransport, ScriptedResponse, index_url, versions_url}; +use crate::lockfile::PackageRef; +use crate::report::Aged; +use std::num::NonZeroU32; +use std::path::PathBuf; +use std::time::Duration; + +trait FakeTransportExt { + fn ok(&self, name: &str, versions_json: &str); + fn error(&self, name: &str); + fn index_ok(&self, name: &str, records_ndjson: &str); + fn index_error(&self, name: &str); +} + +impl FakeTransportExt for FakeTransport { + fn ok(&self, name: &str, versions_json: &str) { + self.push( + &versions_url(name), + ScriptedResponse::Http(200, versions_json.to_string()), + ); + } + + fn error(&self, name: &str) { + self.push(&versions_url(name), ScriptedResponse::Error); + } + + fn index_ok(&self, name: &str, records_ndjson: &str) { + self.push( + &index_url(name), + ScriptedResponse::Http(200, records_ndjson.to_string()), + ); + } + + fn index_error(&self, name: &str) { + self.push(&index_url(name), ScriptedResponse::Error); + } +} + +fn versions_body(entries: &[(&str, i64, bool)], now: DateTime) -> String { + let versions: Vec = entries + .iter() + .map(|(num, days_ago, yanked)| { + let created_at = now - chrono::Duration::days(*days_ago); + format!( + r#"{{"num":"{num}","created_at":"{}","yanked":{yanked}}}"#, + created_at.to_rfc3339() + ) + }) + .collect(); + format!(r#"{{"versions":[{}]}}"#, versions.join(",")) +} + +/// Builds a client with retry/pacing delays zeroed out, so the test +/// suite doesn't sleep. +fn fast_client(transport: FakeTransport) -> CratesIoClient { + CratesIoClient::with_transport( + transport, + None, + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ) +} + +fn too_new(package: &str, locked_version: &str) -> Violation { + Violation { + package: package.to_string(), + version: locked_version.to_string(), + kind: ViolationKind::TooNew(Aged { + published: now(), + age_days: 1, + }), + } +} + +fn too_old(package: &str) -> Violation { + Violation { + package: package.to_string(), + version: "1.0.0".to_string(), + kind: ViolationKind::TooOld(Aged { + published: now(), + age_days: 1000, + }), + } +} + +const CRATES_IO_SOURCE: &str = "registry+https://github.com/rust-lang/crates.io-index"; + +/// A registry package. Its dependency edges default to the same +/// crates.io source as the loader resolves an unsourced edge to, +/// when — as here — the only matching name is a registry package. +fn pkg(name: &str, version: &str, deps: &[(&str, &str)]) -> Package { + Package { + name: name.to_string(), + version: version.to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: deps + .iter() + .map(|(n, v)| PackageRef { + name: n.to_string(), + version: v.to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }) + .collect(), + } +} + +fn non_registry_pkg(name: &str, version: &str, deps: &[(&str, &str)]) -> Package { + Package { + is_registry: false, + source: None, + ..pkg(name, version, deps) + } +} + +/// A dependent whose lockfile source is present but isn't crates.io +/// — a git or alternate-registry package. Unlike `non_registry_pkg` +/// (a local/path package, `source: None`), nothing here can read its +/// requirements: not the crates.io index (it's not on crates.io), +/// and not a local manifest (it has no manifest this crate can find). +fn external_pkg(name: &str, version: &str, source: &str, deps: &[(&str, &str)]) -> Package { + Package { + is_registry: false, + source: Some(source.to_string()), + ..pkg(name, version, deps) + } +} + +const GIT_SOURCE: &str = + "git+https://github.com/example/app#0000000000000000000000000000000000000000"; +const ALT_REGISTRY_SOURCE: &str = "registry+https://example.com/priv-index"; + +/// A `DirectRequirement` naming `declaring_package`/`declaring_version` +/// as the identity of the manifest that placed it — the shape +/// `load_direct_requirements` produces for a real local manifest. +fn local_requirement( + declaring_package: &str, + declaring_version: &str, + crate_name: &str, + req: &str, +) -> DirectRequirement { + DirectRequirement { + manifest: "/work/Cargo.toml".into(), + declaring_package: declaring_package.to_string(), + declaring_version: Some(declaring_version.to_string()), + crate_name: crate_name.to_string(), + req: VersionReq::parse(req).unwrap(), + source: RequirementSource::CratesIo, + } +} + +/// Builds a client and dependents index from `transport`/`packages` +/// and calls production `gather_constraints` with a fixed +/// `/work` working dir — the shared shape of most of this module's +/// `gather_constraints` call sites. +fn gather( + transport: FakeTransport, + packages: &[Package], + requirements: &[DirectRequirement], + name: &str, + locked_version: &str, + source: Option<&str>, +) -> GatheredConstraints { + let mut client = fast_client(transport); + let index = build_indexes(packages).0; + gather_constraints( + &mut client, + &index, + requirements, + Path::new("/work"), + name, + locked_version, + source, + ) +} + +#[test] +fn aliased_registry_requirements_follow_the_locked_version() { + let transport = FakeTransport::default(); + transport.index_ok("app", r#"{"vers":"1.0.0","deps":[{"name":"foo_old","package":"foo","req":"^1"},{"name":"foo_new","package":"foo","req":"^2"}]}"#); + let mut client = fast_client(transport); + let packages = vec![pkg("app", "1.0.0", &[("foo", "1.5.0"), ("foo", "2.5.0")])]; + let index = build_indexes(&packages).0; + for (locked, candidate) in [("1.5.0", "1.4.0"), ("2.5.0", "2.4.0")] { + let gathered = gather_constraints( + &mut client, + &index, + &[], + Path::new("/work"), + "foo", + locked, + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.constraints.len(), 1); + assert!(gathered.unverified_dependents.is_empty()); + assert!(matches!( + walk( + vec![(Version::parse(candidate).unwrap(), 50)], + gathered.constraints + ), + WalkResult::Suggest(_, _) + )); + } +} + +#[test] +fn unmatched_registry_requirements_are_unverified() { + let transport = FakeTransport::default(); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"foo","req":"^1.5"}]}"#, + ); + let packages = vec![pkg("app", "1.0.0", &[("foo", "1.4.0")])]; + let gathered = gather( + transport, + &packages, + &[], + "foo", + "1.4.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["app"]); +} + +#[test] +fn missing_non_registry_requirements_are_unverified() { + let packages = vec![non_registry_pkg("git-app", "1.0.0", &[("foo", "1.5.0")])]; + let gathered = gather( + FakeTransport::default(), + &packages, + &[], + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["git-app"]); +} + +#[test] +fn same_named_git_dependents_report_one_unverified_label() { + let packages = vec![ + external_pkg("parent", "1.0.0", GIT_SOURCE, &[("foo", "1.9.0")]), + external_pkg("parent", "2.0.0", GIT_SOURCE, &[("foo", "1.9.0")]), + ]; + let gathered = gather( + FakeTransport::default(), + &packages, + &[], + "foo", + "1.9.0", + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.unverified_dependents, ["parent"]); +} + +#[test] +fn distinct_git_dependents_are_each_reported() { + let packages = vec![ + external_pkg("parent-a", "1.0.0", GIT_SOURCE, &[("foo", "1.9.0")]), + external_pkg("parent-b", "1.0.0", GIT_SOURCE, &[("foo", "1.9.0")]), + ]; + let gathered = gather( + FakeTransport::default(), + &packages, + &[], + "foo", + "1.9.0", + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.unverified_dependents, ["parent-a", "parent-b"]); +} + +#[test] +fn aliased_manifest_requirements_follow_the_locked_version() { + let mut client = fast_client(FakeTransport::default()); + let packages = vec![non_registry_pkg( + "app", + "1.0.0", + &[("foo", "1.5.0"), ("foo", "2.5.0")], + )]; + let requirements: Vec<_> = ["^1", "^2"] + .into_iter() + .map(|req| DirectRequirement { + manifest: "/work/Cargo.toml".into(), + declaring_package: "app".to_string(), + declaring_version: Some("1.0.0".to_string()), + crate_name: "foo".to_string(), + req: VersionReq::parse(req).unwrap(), + source: RequirementSource::CratesIo, + }) + .collect(); + let index = build_indexes(&packages).0; + for (locked, candidate) in [("1.5.0", "1.4.0"), ("2.5.0", "2.4.0")] { + let gathered = gather_constraints( + &mut client, + &index, + &requirements, + Path::new("/work"), + "foo", + locked, + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.constraints.len(), 1); + assert!(gathered.unverified_dependents.is_empty()); + assert!(matches!( + walk( + vec![(Version::parse(candidate).unwrap(), 50)], + gathered.constraints + ), + WalkResult::Suggest(_, _) + )); + } +} + +#[test] +fn matching_local_identity_with_a_permissive_requirement_allows_the_downgrade() { + // Positive control: a genuine local dependent, matched by both + // name and version, whose manifest requirement is loose enough + // to permit the downgrade — the ordinary case this whole path + // exists for. + let packages = vec![non_registry_pkg("app", "1.0.0", &[("foo", "1.5.0")])]; + let requirements = vec![local_requirement("app", "1.0.0", "foo", "^1")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.constraints.len(), 1); + assert!(gathered.unverified_dependents.is_empty()); + assert!(matches!( + walk( + vec![(Version::parse("1.4.0").unwrap(), 50)], + gathered.constraints + ), + WalkResult::Suggest(_, _) + )); +} + +#[test] +fn matching_local_identity_with_a_restrictive_requirement_blocks_the_downgrade() { + // Positive control, the other direction: the same identity + // match, but the requirement is restrictive enough to reject + // the downgrade candidate. + let packages = vec![non_registry_pkg("app", "1.0.0", &[("foo", "1.5.0")])]; + let requirements = vec![local_requirement("app", "1.0.0", "foo", "^1.5")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert_eq!(gathered.constraints.len(), 1); + assert!(gathered.unverified_dependents.is_empty()); + assert!(matches!( + walk( + vec![(Version::parse("1.4.0").unwrap(), 50)], + gathered.constraints + ), + WalkResult::Blocked { .. } + )); +} + +#[test] +fn git_dependent_sharing_a_local_packages_name_and_version_stays_unverified() { + // A git "app" and a local "app" happen to share a name and + // version. The local one's manifest permits the downgrade, but + // that manifest was never the git dependent's own — it must not + // be credited with verifying it. + let packages = vec![ + external_pkg("app", "1.0.0", GIT_SOURCE, &[("foo", "1.5.0")]), + non_registry_pkg("app", "1.0.0", &[]), + ]; + let requirements = vec![local_requirement("app", "1.0.0", "foo", "^1")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["app"]); +} + +#[test] +fn alternate_registry_dependent_sharing_a_local_packages_name_and_version_stays_unverified() { + // Same shape as the git case, but the external dependent is on + // an alternate registry instead. + let packages = vec![ + external_pkg("app", "1.0.0", ALT_REGISTRY_SOURCE, &[("foo", "1.5.0")]), + non_registry_pkg("app", "1.0.0", &[]), + ]; + let requirements = vec![local_requirement("app", "1.0.0", "foo", "^1")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["app"]); +} + +#[test] +fn restrictive_unrelated_local_requirement_does_not_block_the_external_dependents_target() { + // The local "app" shares the git dependent's name and version, + // but has no dependency edge of its own onto "foo" at all — its + // manifest requirement is unrelated noise. The requirement + // matches the locked version but would block candidate 1.4.0; + // even so, it must not block foo's downgrade for the git + // dependent, whose own requirement can't be read at all. + let packages = vec![ + external_pkg("app", "1.0.0", GIT_SOURCE, &[("foo", "1.5.0")]), + non_registry_pkg("app", "1.0.0", &[]), + ]; + let requirements = vec![local_requirement("app", "1.0.0", "foo", "^1.5")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["app"]); + assert!(matches!( + walk( + vec![(Version::parse("1.4.0").unwrap(), 50)], + gathered.constraints + ), + WalkResult::Suggest(_, _) + )); +} + +#[test] +fn local_declaring_version_mismatch_neither_verifies_nor_constrains() { + // A local "app" 2.0.0 declares a restrictive requirement on + // "foo", but the actual lockfile dependent is a *different* + // "app" — 1.0.0 — with an identical name. Declaring package + // name alone must not be enough to apply this requirement. + let packages = vec![non_registry_pkg("app", "1.0.0", &[("foo", "1.5.0")])]; + let requirements = vec![local_requirement("app", "2.0.0", "foo", "^1.5")]; + + let gathered = gather( + FakeTransport::default(), + &packages, + &requirements, + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + assert!(gathered.constraints.is_empty()); + assert_eq!(gathered.unverified_dependents, ["app"]); +} + +#[test] +fn only_too_new_violations_are_fetched() { + let transport = FakeTransport::default(); + transport.ok("serde", &versions_body(&[("1.0.0", 50, false)], now())); + // "syn" has no scripted response: if it were fetched, the + // transport would panic. + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.0.0"), too_old("syn")]; + let packages = vec![pkg("serde", "1.0.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert_eq!(outcomes.len(), 1); +} + +#[test] +fn an_unparsable_locked_version_does_not_abort_the_others() { + let transport = FakeTransport::default(); + // "serde" has no scripted response: if its versions were fetched, + // the transport would panic — its unparsable locked version must + // be skipped before that. + transport.ok( + "syn", + &versions_body(&[("1.0.0", 50, false), ("1.1.0", 5, false)], now()), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "not-a-version"), too_new("syn", "1.1.0")]; + let packages = vec![pkg("serde", "not-a-version", &[]), pkg("syn", "1.1.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert_eq!(outcomes.len(), 1); + assert!(matches!(&outcomes[0], Outcome::Suggest { package, .. } if package == "syn")); +} + +#[test] +fn a_failed_fetch_does_not_abort_the_others() { + let transport = FakeTransport::default(); + transport.error("serde"); + transport.ok( + "syn", + &versions_body(&[("1.0.0", 50, false), ("1.1.0", 5, false)], now()), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.0.0"), too_new("syn", "1.1.0")]; + let packages = vec![pkg("serde", "1.0.0", &[]), pkg("syn", "1.1.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert_eq!(outcomes.len(), 1); + assert!(matches!(&outcomes[0], Outcome::Suggest { package, .. } if package == "syn")); +} + +#[test] +fn no_compliant_version_reports_the_no_candidate_outcome() { + let transport = FakeTransport::default(); + transport.ok("serde", &versions_body(&[("1.0.0", 5, false)], now())); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.0.0")]; + let packages = vec![pkg("serde", "1.0.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!(matches!(outcomes[0], Outcome::NoCompliantVersion { .. })); +} + +#[test] +fn no_too_new_violations_yields_none() { + let transport = FakeTransport::default(); + let mut client = fast_client(transport); + + let violations = vec![too_old("syn")]; + let outcomes = generate_suggestions( + &mut client, + &violations, + &[], + &[], + Path::new("/work"), + 30, + false, + now(), + ); + + assert!(outcomes.is_none()); +} + +#[test] +fn transitive_dependent_requirement_blocks_the_newest_candidate() { + // "app" depends on serde 1.5.0; serde's index says app requires ^1.5. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected Blocked"), + } +} + +#[test] +fn suggests_the_semver_highest_compliant_version_over_a_later_backport() { + // 1.4.0 outranks 1.3.9 by semver even though 1.3.9 was published + // more recently (40 days ago vs. 100), so it must be the suggestion. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.4.0", 100, false), ("1.3.9", 40, false)], now()), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![pkg("serde", "1.5.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + suggested_age_days, + .. + } => { + assert_eq!(suggested_version, "1.4.0"); + assert_eq!(*suggested_age_days, 100); + } + _ => panic!("expected Suggest"), + } +} + +#[test] +fn blocked_names_the_semver_highest_compliant_version_as_newest_compliant() { + // 1.4.0 and 1.3.9 both satisfy age, but the manifest's `^1.4.1` + // requirement rejects both. "newest_compliant" in the Blocked + // outcome must be the semver-highest one, 1.4.0, not the more + // recently published 1.3.9. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.4.0", 100, false), ("1.3.9", 40, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.4.1"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.4.1"); + } + _ => panic!("expected Blocked"), + } +} + +#[test] +fn same_name_version_collision_across_sources_does_not_leak_dependents() { + // Two packages both named "serde" locked at 1.5.0: one from + // crates.io, one from git. "consumer" depends on the git one + // specifically. The crates.io serde must not inherit consumer's + // requirement just because the name and version happen to match. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + // If "consumer" were (wrongly) treated as a dependent of the + // crates.io serde, this scripted index response would be + // fetched and its ^1.5 requirement would block the downgrade. + transport.index_ok( + "consumer", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5"}]}"#, + ); + let mut client = fast_client(transport); + + let registry_source = "registry+https://github.com/rust-lang/crates.io-index"; + let git_source = + "git+https://github.com/example/serde#0000000000000000000000000000000000000000"; + + let packages = vec![ + Package { + name: "serde".to_string(), + version: "1.5.0".to_string(), + is_registry: true, + source: Some(registry_source.to_string()), + dependencies: vec![], + }, + Package { + name: "serde".to_string(), + version: "1.5.0".to_string(), + is_registry: false, + source: Some(git_source.to_string()), + dependencies: vec![], + }, + Package { + name: "consumer".to_string(), + version: "1.0.0".to_string(), + is_registry: true, + source: Some(registry_source.to_string()), + dependencies: vec![PackageRef { + name: "serde".to_string(), + version: "1.5.0".to_string(), + source: Some(git_source.to_string()), + }], + }, + ]; + + let violations = vec![too_new("serde", "1.5.0")]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!( + matches!(&outcomes[0], Outcome::Suggest { suggested_version, .. } if suggested_version == "1.4.0"), + "the crates.io serde must not be blocked by consumer's requirement on the git serde: {:?}", + match &outcomes[0] { + Outcome::Blocked { blocker, .. } => format!("Blocked by {}", blocker.name), + _ => "other".to_string(), + } + ); +} + +#[test] +fn source_collision_yields_a_source_qualified_package_spec() { + // Same fixture as above: a crates.io "serde" and a git "serde" + // both locked at 1.5.0. Cargo would reject the abbreviated + // `serde@1.5.0` spec as ambiguous, so the suggestion for the + // registry package must qualify it with the registry source. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + let mut client = fast_client(transport); + + let registry_source = "registry+https://github.com/rust-lang/crates.io-index"; + let git_source = + "git+https://github.com/example/serde#0000000000000000000000000000000000000000"; + + let packages = vec![ + Package { + name: "serde".to_string(), + version: "1.5.0".to_string(), + is_registry: true, + source: Some(registry_source.to_string()), + dependencies: vec![], + }, + Package { + name: "serde".to_string(), + version: "1.5.0".to_string(), + is_registry: false, + source: Some(git_source.to_string()), + dependencies: vec![], + }, + ]; + + let violations = vec![too_new("serde", "1.5.0")]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { package_spec, .. } => { + assert_eq!(package_spec, &format!("{registry_source}#serde")); + } + _ => panic!("expected serde to be Suggest"), + } +} + +#[test] +fn no_collision_keeps_the_abbreviated_package_spec() { + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![pkg("serde", "1.5.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { package_spec, .. } => assert_eq!(package_spec, "serde"), + _ => panic!("expected serde to be Suggest"), + } +} + +#[test] +fn path_package_sharing_a_name_and_version_does_not_leak_dependents_to_the_registry_package() { + // A path package "local-crate" 0.1.0 and a crates.io package of + // the same name and version both exist. "consumer" depends on + // the path one via an edge that omits the source, as cargo does + // for any edge whose true target has none. Since a path + // package's own source is always omitted too, the crates.io + // package must not inherit consumer's requirement just because + // the name and version happen to collide. + let transport = FakeTransport::default(); + transport.ok( + "local-crate", + &versions_body(&[("1.1.0", 5, false), ("1.0.0", 50, false)], now()), + ); + // If "consumer" were (wrongly) treated as a dependent of the + // crates.io local-crate, this scripted index response would be + // fetched and its ^1.1 requirement would block the downgrade. + transport.index_ok( + "consumer", + r#"{"vers":"1.0.0","deps":[{"name":"local-crate","req":"^1.1"}]}"#, + ); + let mut client = fast_client(transport); + + let registry_source = "registry+https://github.com/rust-lang/crates.io-index"; + + let packages = vec![ + Package { + name: "local-crate".to_string(), + version: "1.1.0".to_string(), + is_registry: true, + source: Some(registry_source.to_string()), + dependencies: vec![], + }, + Package { + name: "local-crate".to_string(), + version: "1.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![], + }, + Package { + name: "consumer".to_string(), + version: "1.0.0".to_string(), + is_registry: true, + source: Some(registry_source.to_string()), + dependencies: vec![PackageRef { + name: "local-crate".to_string(), + version: "1.1.0".to_string(), + source: None, + }], + }, + ]; + + let violations = vec![too_new("local-crate", "1.1.0")]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!( + matches!(&outcomes[0], Outcome::Suggest { suggested_version, .. } if suggested_version == "1.0.0"), + "the crates.io local-crate must not be blocked by consumer's requirement on the path local-crate: {:?}", + match &outcomes[0] { + Outcome::Blocked { blocker, .. } => format!("Blocked by {}", blocker.name), + _ => "other".to_string(), + } + ); +} + +#[test] +fn dev_kind_edge_from_a_transitive_dependent_is_ignored() { + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5","kind":"dev"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!(matches!(outcomes[0], Outcome::Suggest { .. })); +} + +#[test] +fn renamed_dependency_is_matched_by_its_real_name() { + let transport = FakeTransport::default(); + transport.ok("serde", &versions_body(&[("1.4.0", 50, false)], now())); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"my_serde","package":"serde","req":"^1.5"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!(matches!(outcomes[0], Outcome::Blocked { .. })); +} + +#[test] +fn alternate_registry_index_declaration_is_not_enforced() { + // "app" declares a normal `foo = "^1"` from crates.io and a + // same-crate alias `foo_alt = { package = "foo", version = "^1.3", + // registry = "alt" }` from an alternate registry. The alternate + // registry declaration cannot be resolved against the locked + // crates.io package, so only `^1` may be enforced. + let transport = FakeTransport::default(); + transport.ok( + "foo", + &versions_body(&[("1.5.0", 5, false), ("1.2.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"foo","req":"^1"},{"name":"foo_alt","package":"foo","req":"^1.3","registry":"alt"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.5.0")]; + let packages = vec![ + pkg("foo", "1.5.0", &[]), + pkg("app", "1.0.0", &[("foo", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, .. + } => assert_eq!(suggested_version, "1.2.0"), + _ => panic!("expected foo to be Suggest, with the alternate registry declaration ignored"), + } +} + +#[test] +fn ambiguous_optional_declaration_does_not_block_the_downgrade() { + // "app" declares both a normal `serde = "^1"` and a disabled, + // renamed optional `serde_new = { package = "serde", version = + // "^1.5", optional = true }`. Both match locked serde@1.5.0, but + // since the optional one may not even be activated, neither can + // be enforced as a definite blocker. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1"},{"name":"serde_new","package":"serde","req":"^1.5","optional":true}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.4.0"); + assert_eq!(unverified_dependents, &["app".to_string()]); + } + _ => panic!("expected serde to be Suggest, with app marked unverified"), + } +} + +#[test] +fn unique_optional_declaration_still_blocks() { + // Only the optional, renamed declaration matches — no ambiguity, + // so it's the unique explanation for the lockfile edge and must + // still be enforced. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde_new","package":"serde","req":"^1.5","optional":true}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected serde to be Blocked by app's unique optional declaration"), + } +} + +#[test] +fn mandatory_requirements_from_different_kinds_both_block() { + // "app" declares an unconditional, nonoptional normal + // requirement of `^1.5` and an unconditional, nonoptional build + // requirement of `^1` on serde. Both are always active, so both + // are enforced — the more restrictive one (`^1.5`) rejects the + // downgrade to 1.4.0. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5"},{"name":"serde","req":"^1","kind":"build"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected serde to be Blocked by app's mandatory requirement"), + } +} + +#[test] +fn mandatory_requirement_still_blocks_alongside_uncertain_declaration() { + // "app" declares an unconditional, nonoptional normal + // requirement of `^1.5`, and a separate disabled, renamed + // optional declaration matching a looser `^1`. The mandatory + // requirement is enforced regardless of the uncertain one, even + // though the uncertain one alone would have permitted the + // downgrade. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5"},{"name":"serde_new","package":"serde","req":"^1","optional":true}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected serde to be Blocked by app's mandatory requirement"), + } +} + +#[test] +fn target_specific_requirement_is_always_enforced() { + // "app" declares an unconditional `serde = "^1"` and a stricter + // `serde = "^1.4"` under a target-specific table. Cargo's resolver + // evaluates every target table regardless of the host platform, so + // the target-specific requirement is just as mandatory as the + // unconditional one and must be enforced alongside it. + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.3.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1"},{"name":"serde","req":"^1.4","target":"cfg(unix)"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.3.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.4"); + } + _ => panic!("expected serde to be Blocked by app's target-specific requirement"), + } +} + +#[test] +fn identical_requirement_repeated_across_targets_still_blocks() { + // "app" declares the same `serde = "^1.5"` requirement under two + // target-specific tables (e.g. cfg(unix) and cfg(windows)), which + // the index lists as two separate `deps` entries with identical + // `req` strings. This is not the same situation as two distinct + // declarations that might not both be active — the requirement + // is identical either way, so it must still be enforced as a + // definite blocker rather than merely "unverified". + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_ok( + "app", + r#"{"vers":"1.0.0","deps":[{"name":"serde","req":"^1.5","target":"cfg(unix)"},{"name":"serde","req":"^1.5","target":"cfg(windows)"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.4.0"); + assert_eq!(blocker.name, "app"); + assert_eq!(blocker.req, "^1.5"); + } + _ => panic!("expected serde to be Blocked by app's requirement, not merely unverified"), + } +} + +#[test] +fn requirement_attribution_acceptance_cases() { + // name, second version, requirements, unverified, blocks 1.7 + let cases: &[(&str, bool, &[&str], bool, bool)] = &[ + ("overlap", true, &["^1", ">=1.8,<3"], true, false), + ("disjoint", true, &["^1", "^2"], false, false), + ( + "identical aliases", + true, + &[">=1.8,<3", ">=1.8, <3"], + false, + true, + ), + ("single version", false, &["^1", ">=1.8,<2"], false, true), + ("optional", true, &["^1", ">=1.8,<3"], true, false), + ("target", true, &["^1", ">=1.8,<3"], true, false), + ("other blocker", true, &["^1", ">=1.8,<3"], true, true), + ("other parent", false, &["^1", ">=1.8,<3"], false, true), + ("other source", true, &["^1", ">=1.8,<3"], false, true), + ("path sibling", false, &["^1", ">=1.8,<3"], false, true), + ("duplicate edge", false, &["^1", ">=1.8,<3"], false, true), + ("unreadable", false, &[], true, false), + ]; + for registry in [true, false] { + for &(case, second_version, reqs, unverified, blocked) in cases { + if !registry && matches!(case, "optional" | "target") { + continue; + } + let transport = FakeTransport::default(); + let mut requirements = Vec::new(); + let mut app = if registry { + pkg("app", "1.0.0", &[("foo", "1.9.0")]) + } else { + non_registry_pkg("app", "1.0.0", &[("foo", "1.9.0")]) + }; + let source = pkg("foo", "1.9.0", &[]).source.unwrap(); + app.dependencies[0].source = Some(source.clone()); + let mut packages = vec![pkg("foo", "1.9.0", &[])]; + if second_version { + let mut sibling = pkg("foo", "2.0.0", &[]); + if case == "other source" { + sibling = external_pkg("foo", "2.0.0", ALT_REGISTRY_SOURCE, &[]); + } + app.dependencies.push(PackageRef { + name: "foo".into(), + version: sibling.version.clone(), + source: sibling.source.clone(), + }); + packages.push(sibling); + } + match case { + "path sibling" => { + packages.push(non_registry_pkg("foo", "1.9.0", &[])); + app.dependencies.push(PackageRef { + name: "foo".into(), + version: "1.9.0".into(), + source: None, + }); + } + "duplicate edge" => app.dependencies.push(PackageRef { + name: "foo".into(), + version: "1.9.0".into(), + source: Some(source.clone()), + }), + "other parent" => { + packages.push(pkg("foo", "2.0.0", &[])); + packages.push(pkg("other", "1.0.0", &[("foo", "2.0.0")])); + } + "other blocker" => { + if registry { + packages.push(pkg("other", "1.0.0", &[("foo", "1.9.0")])); + transport.index_ok( + "other", + r#"{"vers":"1.0.0","deps":[{"name":"foo","req":">=1.8"}]}"#, + ); + } else { + packages.push(non_registry_pkg("other", "1.0.0", &[("foo", "1.9.0")])); + requirements.push(local_requirement("other", "1.0.0", "foo", ">=1.8")); + } + } + _ => {} + } + packages.push(app); + if registry && case != "unreadable" { + let deps: Vec<_> = reqs.iter().enumerate().map(|(i, req)| { + serde_json::json!({ + "name": format!("alias_{i}"), "package": "foo", "req": req, + "optional": case == "optional" && i == 0, + "target": if case == "target" && i == 0 { Some("cfg(unix)") } else { None }, + }) + }).collect(); + transport.index_ok( + "app", + &serde_json::json!({ + "vers": "1.0.0", "deps": deps, + }) + .to_string(), + ); + } else if registry { + transport.index_error("app"); + } else { + requirements.extend( + reqs.iter() + .map(|req| local_requirement("app", "1.0.0", "foo", req)), + ); + } + let mut client = fast_client(transport); + let index = build_indexes(&packages).0; + let gathered = gather_constraints( + &mut client, + &index, + &requirements, + Path::new("/work"), + "foo", + "1.9.0", + Some(&source), + ); + let expected_unverified = if unverified { vec!["app"] } else { vec![] }; + assert_eq!( + gathered.unverified_dependents, expected_unverified, + "{case}, registry={registry}" + ); + if case == "disjoint" { + assert!( + gathered + .constraints + .iter() + .any(|c| !c.req.matches(&v("0.9.0"))) + ); + } + let result = walk(vec![(v("1.7.0"), 80)], gathered.constraints); + assert_eq!( + matches!(result, WalkResult::Blocked { .. }), + blocked, + "{case}, registry={registry}" + ); + if case == "other blocker" { + let WalkResult::Blocked { blocker, .. } = result else { + unreachable!() + }; + assert_eq!( + blocker.blocker_name, + if registry { "other" } else { "Cargo.toml" } + ); + } else if !blocked { + assert!( + matches!(result, WalkResult::Suggest(version, 80) if version == v("1.7.0")) + ); + } + } + } +} + +#[test] +fn failed_index_fetch_yields_suggestion_with_unverified_annotation() { + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.index_error("app"); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.5.0")]; + let packages = vec![ + pkg("serde", "1.5.0", &[]), + pkg("app", "1.0.0", &[("serde", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Suggest { + unverified_dependents, + .. + } => assert_eq!(unverified_dependents, &["app".to_string()]), + _ => panic!("expected Suggest"), + } +} + +#[test] +fn a_package_locked_at_two_versions_produces_two_outcomes() { + let transport = FakeTransport::default(); + transport.ok( + "serde", + &versions_body(&[("1.0.0", 50, false), ("2.0.0", 50, false)], now()), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("serde", "1.0.0"), too_new("serde", "2.0.0")]; + let packages = vec![pkg("serde", "1.0.0", &[]), pkg("serde", "2.0.0", &[])]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert_eq!(outcomes.len(), 2); +} + +#[test] +fn also_suggested_is_false_when_the_blocker_itself_has_no_suggestion() { + // "y" blocks "target", and "y" is itself in the too-new set — + // but y's own walk resolves to NoCompliantVersion, not Suggest, + // so the blocked message must not claim a fix for y exists. + let transport = FakeTransport::default(); + transport.ok( + "target", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.ok("y", &versions_body(&[("1.5.0", 5, false)], now())); + transport.index_ok( + "y", + r#"{"vers":"1.5.0","deps":[{"name":"target","req":"^1.5"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("target", "1.5.0"), too_new("y", "1.5.0")]; + let packages = vec![ + pkg("target", "1.5.0", &[]), + pkg("y", "1.5.0", &[("target", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + match &outcomes[0] { + Outcome::Blocked { blocker, .. } => { + assert_eq!(blocker.name, "y"); + assert!( + !blocker.also_suggested, + "y has no Suggest outcome of its own" + ); + } + _ => panic!("expected target to be Blocked"), + } + assert!(matches!( + &outcomes[1], + Outcome::NoCompliantVersion { package, .. } if package == "y" + )); +} + +#[test] +fn also_suggested_is_false_when_only_another_version_of_the_blocker_is_suggested() { + // "foo" is locked at both 1.5.0 and 2.5.0. Only 1.5.0 resolves to + // a Suggest; the 2.5.0 that blocks "target" has no compliant + // version, so the blocked message must not point at the unrelated + // 1.5.0 downgrade. + let transport = FakeTransport::default(); + transport.ok( + "target", + &versions_body(&[("1.5.0", 5, false), ("1.4.0", 50, false)], now()), + ); + transport.ok( + "foo", + &versions_body( + &[ + ("1.5.0", 5, false), + ("1.4.0", 50, false), + ("2.5.0", 5, false), + ], + now(), + ), + ); + transport.index_ok( + "foo", + r#"{"vers":"2.5.0","deps":[{"name":"target","req":"^1.5"}]}"#, + ); + let mut client = fast_client(transport); + + let violations = vec![ + too_new("target", "1.5.0"), + too_new("foo", "1.5.0"), + too_new("foo", "2.5.0"), + ]; + let packages = vec![ + pkg("target", "1.5.0", &[]), + pkg("foo", "1.5.0", &[]), + pkg("foo", "2.5.0", &[("target", "1.5.0")]), + ]; + let outcomes = suggestions(&mut client, &violations, &packages, &[], Path::new("/work")); + + assert!( + matches!(&outcomes[1], Outcome::Suggest { package, locked_version, .. } + if package == "foo" && locked_version == "1.5.0"), + "foo 1.5.0 should be suggested, otherwise the test proves nothing" + ); + match &outcomes[0] { + Outcome::Blocked { blocker, .. } => { + assert_eq!(blocker.name, "foo"); + assert_eq!(blocker.version.as_deref(), Some("2.5.0")); + assert!( + !blocker.also_suggested, + "the suggestion is for foo 1.5.0, which does not unblock foo 2.5.0" + ); + } + _ => panic!("expected target to be Blocked"), + } +} + +#[test] +fn manifest_constraint_is_scoped_to_the_declaring_dependent() { + // member_a locks clap@2.5.0 and requires ^2; member_b locks a + // different clap version and requires ^3. member_b's unrelated + // requirement must not leak into member_a's constraint set. + let transport = FakeTransport::default(); + transport.ok( + "clap", + &versions_body(&[("2.5.0", 5, false), ("2.0.0", 50, false)], now()), + ); + let mut client = fast_client(transport); + + let direct_requirements = vec![ + crate::manifest::DirectRequirement { + manifest: PathBuf::from("/work/member_a/Cargo.toml"), + declaring_package: "member_a".to_string(), + declaring_version: Some("0.1.0".to_string()), + crate_name: "clap".to_string(), + req: VersionReq::parse("^2").unwrap(), + source: RequirementSource::CratesIo, + }, + crate::manifest::DirectRequirement { + manifest: PathBuf::from("/work/member_b/Cargo.toml"), + declaring_package: "member_b".to_string(), + declaring_version: Some("0.1.0".to_string()), + crate_name: "clap".to_string(), + req: VersionReq::parse("^3").unwrap(), + source: RequirementSource::CratesIo, + }, + ]; + let packages = vec![ + pkg("clap", "2.5.0", &[]), + non_registry_pkg("member_a", "0.1.0", &[("clap", "2.5.0")]), + non_registry_pkg("member_b", "0.1.0", &[("clap", "3.1.0")]), + ]; + + let violations = vec![too_new("clap", "2.5.0")]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + Path::new("/work"), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, .. + } => assert_eq!(suggested_version, "2.0.0"), + _ => panic!("expected clap to be Suggest: member_b's ^3 must not apply"), + } +} + +#[test] +fn stale_cached_index_records_missing_dependent_version_are_refetched() { + // The cache holds "app"'s index records as of an earlier, older + // release ("1.0.0"). The lockfile has since moved to "1.0.1", + // which the cache entry (still within its max age) doesn't list. + // Looking up "1.0.1" must fall back to a live fetch rather than + // silently treating the requirement as unreadable. + let dir = tempfile::tempdir().unwrap(); + let cache_path = dir.path().join("cache.json"); + let mut cache = crate::cache::ResponseCache::load(Some(&cache_path)); + cache.set_index_records( + "app", + vec![crate::api::IndexRecord { + vers: "1.0.0".to_string(), + yanked: false, + deps: vec![], + }], + ); + cache.save().unwrap(); + + let transport = FakeTransport::default(); + transport.index_ok( + "app", + r#"{"vers":"1.0.1","deps":[{"name":"foo","req":"^1.5"}]}"#, + ); + let mut client = CratesIoClient::with_transport( + transport, + Some(&cache_path), + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ); + + let packages = vec![pkg("app", "1.0.1", &[("foo", "1.5.0")])]; + let index = build_indexes(&packages).0; + let gathered = gather_constraints( + &mut client, + &index, + &[], + Path::new("/work"), + "foo", + "1.5.0", + Some(CRATES_IO_SOURCE), + ); + + assert!(gathered.unverified_dependents.is_empty()); + assert_eq!(gathered.constraints.len(), 1); + assert!(!gathered.constraints[0].req.matches(&v("1.4.0"))); +} diff --git a/src/suggest/tests/manifest_dependent_identity_tests.rs b/src/suggest/tests/manifest_dependent_identity_tests.rs new file mode 100644 index 0000000..059c36b --- /dev/null +++ b/src/suggest/tests/manifest_dependent_identity_tests.rs @@ -0,0 +1,487 @@ +/// Covers matching a real local dependent's manifest requirement against +/// its own locked identity (name and version), including workspace +/// version inheritance, rather than name alone. +use super::*; +use crate::api::RetryPolicy; +use crate::api::test_support::{FakeTransport, ScriptedResponse, versions_url}; +use crate::manifest::load_direct_requirements; +use crate::report::Aged; +use std::num::NonZeroU32; +use std::time::Duration; +use tempfile::tempdir; + +const CRATES_IO_SOURCE: &str = "registry+https://github.com/rust-lang/crates.io-index"; +const GIT_SOURCE: &str = + "git+https://github.com/example/vendor#0000000000000000000000000000000000000000"; +const ALT_REGISTRY_SOURCE: &str = "registry+https://example.com/priv-index"; + +fn versions_body(entries: &[(&str, i64, bool)]) -> String { + let versions: Vec = entries + .iter() + .map(|(num, days_ago, yanked)| { + let created_at = now() - chrono::Duration::days(*days_ago); + format!( + r#"{{"num":"{num}","created_at":"{}","yanked":{yanked}}}"#, + created_at.to_rfc3339() + ) + }) + .collect(); + format!(r#"{{"versions":[{}]}}"#, versions.join(",")) +} + +fn fast_client(transport: FakeTransport) -> CratesIoClient { + CratesIoClient::with_transport( + transport, + None, + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ) +} + +fn too_new(package: &str, locked_version: &str) -> Violation { + Violation { + package: package.to_string(), + version: locked_version.to_string(), + kind: ViolationKind::TooNew(Aged { + published: now() - chrono::Duration::days(5), + age_days: 5, + }), + } +} + +fn write_manifest(dir: &Path, rel: &str, contents: &str) { + let path = dir.join(rel); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, contents).unwrap(); +} + +#[test] +fn workspace_inherited_declaring_version_matches_the_locked_local_dependent() { + // "app"'s own version is inherited from the workspace root + // rather than written directly. Its requirement on foo must + // still be recognized as its own — matched by the resolved + // version, not skipped for lack of one. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["app"] + +[workspace.package] +version = "0.1.0" +"#, + ); + write_manifest( + dir.path(), + "app/Cargo.toml", + r#" +[package] +name = "app" +version.workspace = true + +[dependencies] +foo = "^1.0" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.8.0"); + assert!( + unverified_dependents.is_empty(), + "app's workspace-inherited version should have matched: {unverified_dependents:?}" + ); + } + _ => panic!("expected foo to be Suggest"), + } +} + +#[test] +fn unresolvable_declaring_version_does_not_falsely_verify_the_dependent() { + // "app" inherits its version from the workspace, but the + // workspace root supplies none — the member's manifest fails to + // load entirely, so no requirement is ever collected for it. + // "app" must come back unverified, not silently treated as + // matching by name alone. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + "Cargo.toml", + r#" +[workspace] +members = ["app"] +"#, + ); + write_manifest( + dir.path(), + "app/Cargo.toml", + r#" +[package] +name = "app" +version.workspace = true + +[dependencies] +foo = "^1.0" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!( + !warnings.is_empty(), + "expected a warning about app's unresolved workspace version" + ); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + unverified_dependents, + .. + } => assert_eq!(unverified_dependents, &["app".to_string()]), + _ => panic!("expected foo to be Suggest, with app marked unverified"), + } +} + +#[test] +fn real_local_constraint_is_enforced_while_an_external_dependent_stays_unverified() { + // "app" is a real local dependent whose manifest permits the + // downgrade; "vendor" is a git dependent that also locks foo at + // the same version, but nothing here can read its requirement. + // 1.8.0 is the newer, otherwise-preferred candidate, but app's + // manifest rejects it, so enforcement must fall back to 1.8.5 + // while still flagging vendor as unverified. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +foo = "^1.8.5" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[ + ("1.9.0", 5, false), + ("1.8.0", 40, false), + ("1.8.5", 50, false), + ]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + Package { + name: "vendor".to_string(), + version: "1.0.0".to_string(), + is_registry: false, + source: Some(GIT_SOURCE.to_string()), + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.8.5"); + assert_eq!(unverified_dependents, &["vendor".to_string()]); + } + _ => panic!("expected foo to be Suggest, with vendor marked unverified"), + } +} + +#[test] +fn git_dependent_sharing_a_local_packages_identity_is_not_verified_by_its_manifest() { + // A git "app" and the local "app" share name and version; the + // local manifest permits the downgrade but was never the git + // dependent's own, so it must not verify it. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "app" +version = "1.0.0" + +[dependencies] +foo = "^1.0" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "1.0.0".to_string(), + is_registry: false, + source: Some(GIT_SOURCE.to_string()), + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + Package { + name: "app".to_string(), + version: "1.0.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.8.0"); + assert_eq!(unverified_dependents, &["app".to_string()]); + } + _ => panic!("expected foo to be Suggest, with the git app marked unverified"), + } +} + +#[test] +fn alternate_registry_dependent_sharing_a_local_packages_identity_is_not_verified_by_its_manifest() +{ + // An alternate-registry "app" and the local "app" share name and + // version; the local manifest permits the downgrade but was + // never the alternate-registry dependent's own, so it must not + // verify it. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + "Cargo.toml", + r#" +[package] +name = "app" +version = "1.0.0" + +[dependencies] +foo = "^1.0" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "1.0.0".to_string(), + is_registry: false, + source: Some(ALT_REGISTRY_SOURCE.to_string()), + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + Package { + name: "app".to_string(), + version: "1.0.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.8.0"); + assert_eq!(unverified_dependents, &["app".to_string()]); + } + _ => { + panic!("expected foo to be Suggest, with the alternate-registry app marked unverified") + } + } +} diff --git a/src/suggest/tests/manifest_registry_identity_tests.rs b/src/suggest/tests/manifest_registry_identity_tests.rs new file mode 100644 index 0000000..f636bd4 --- /dev/null +++ b/src/suggest/tests/manifest_registry_identity_tests.rs @@ -0,0 +1,473 @@ +/// Covers a manifest crate name declared against two different +/// registries at once: an ordinary crates.io requirement and one +/// explicitly pinned to a renamed private registry. The renamed +/// declaration must never be treated as if it constrained the crates.io +/// package this flow actually suggests a downgrade for, nor may it +/// silently mark the declaring dependent as unverified when the +/// crates.io declaration alone already verifies it. +use super::*; +use crate::api::RetryPolicy; +use crate::api::test_support::{FakeTransport, ScriptedResponse, versions_url}; +use crate::manifest::load_direct_requirements; +use crate::report::Aged; +use std::num::NonZeroU32; +use std::time::Duration; +use tempfile::tempdir; + +const CRATES_IO_SOURCE: &str = "registry+https://github.com/rust-lang/crates.io-index"; +const PRIVATE_SOURCE: &str = "registry+https://example.com/priv-index"; + +fn versions_body(entries: &[(&str, i64, bool)]) -> String { + let versions: Vec = entries + .iter() + .map(|(num, days_ago, yanked)| { + let created_at = now() - chrono::Duration::days(*days_ago); + format!( + r#"{{"num":"{num}","created_at":"{}","yanked":{yanked}}}"#, + created_at.to_rfc3339() + ) + }) + .collect(); + format!(r#"{{"versions":[{}]}}"#, versions.join(",")) +} + +fn fast_client(transport: FakeTransport) -> CratesIoClient { + CratesIoClient::with_transport( + transport, + None, + 24, + RetryPolicy { + retry_count: NonZeroU32::new(1).unwrap(), + retry_delay: Duration::from_millis(0), + pacing_delay: Duration::from_millis(0), + }, + ) +} + +fn too_new(package: &str, locked_version: &str) -> Violation { + Violation { + package: package.to_string(), + version: locked_version.to_string(), + kind: ViolationKind::TooNew(Aged { + published: now(), + age_days: 1, + }), + } +} + +fn write_manifest(dir: &Path, contents: &str) { + std::fs::write(dir.join("Cargo.toml"), contents).unwrap(); +} + +/// A "foo" package locked at 1.9.0 on each of `CRATES_IO_SOURCE` and +/// `PRIVATE_SOURCE`, both depended on by workspace member "app" via +/// source-qualified lockfile dependency edges — the shape a real +/// lockfile resolves to when a same-name/same-version package exists +/// on more than one registry. +fn packages_with_dual_source_foo() -> Vec { + vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: false, + source: Some(PRIVATE_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![ + PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }, + PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(PRIVATE_SOURCE.to_string()), + }, + ], + }, + ] +} + +#[test] +fn crates_io_declaration_is_enforced_while_the_renamed_registry_declaration_is_excluded() { + // "app" depends on crates.io foo ^1.0 and a renamed + // private-registry foo pinned to =1.9.0; both resolve to foo + // 1.9.0 in the lockfile. Only the crates.io declaration should + // count toward foo's suggestion: it doesn't block 1.8.0, so foo + // should suggest it, and the =1.9.0 renamed declaration must not + // spuriously block a package it was never written for. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +foo = "^1.0" +foo_priv = { package = "foo", version = "=1.9.0", registry = "priv" } +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = packages_with_dual_source_foo(); + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + package_spec, + suggested_version, + unverified_dependents, + .. + } => { + assert_eq!(suggested_version, "1.8.0"); + assert_eq!(package_spec, &format!("{CRATES_IO_SOURCE}#foo")); + assert!( + unverified_dependents.is_empty(), + "app is verified by its crates.io ^1.0 declaration: {unverified_dependents:?}" + ); + } + Outcome::Blocked { blocker, .. } => panic!( + "expected foo to be Suggest, but was Blocked by {} \ + (the renamed-registry declaration must have leaked in)", + blocker.name + ), + _ => panic!("expected foo to be Suggest"), + } +} + +#[test] +fn reverse_roles_still_block_via_the_crates_io_declaration() { + // Same fixture, roles swapped: crates.io foo is now pinned to + // =1.9.0 and the renamed private-registry foo carries the + // lenient ^1.0. The crates.io pin must still block the + // downgrade, reported as the blocker. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +foo = "=1.9.0" +foo_priv = { package = "foo", version = "^1.0", registry = "priv" } +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = packages_with_dual_source_foo(); + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.8.0"); + assert_eq!(blocker.name, "Cargo.toml"); + assert_eq!(blocker.version, None); + assert_eq!(blocker.req, "=1.9.0"); + } + _ => panic!("expected foo to be Blocked by the crates.io declaration"), + } +} + +#[test] +fn explicit_crates_io_registry_name_still_blocks() { + // Same fixture as `reverse_roles_still_block_via_the_crates_io_declaration`, + // but the crates.io declaration names its registry explicitly + // via the reserved `crates-io` alias instead of omitting + // `registry` altogether. It must still be recognized as + // crates.io and enforced, not excluded as an unrecognized + // alternate registry. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +foo = { version = "=1.9.0", registry = "crates-io" } +foo_priv = { package = "foo", version = "^1.0", registry = "priv" } +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = packages_with_dual_source_foo(); + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.8.0"); + assert_eq!(blocker.name, "Cargo.toml"); + assert_eq!(blocker.version, None); + assert_eq!(blocker.req, "=1.9.0"); + } + _ => panic!( + "expected foo to be Blocked by the explicit crates-io declaration, \ + not excluded as an unrecognized alternate registry" + ), + } +} + +#[test] +fn a_second_crates_io_declaration_for_the_same_identity_still_blocks() { + // Both declarations are ordinary crates.io dependencies — no + // registry collision at all — one lenient, one restrictive. + // Guards against the crates.io source filter collapsing + // enforcement down to a single matching declaration. + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +foo = "^1.0" +foo_pinned = { package = "foo", version = "=1.9.0" } +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + assert_eq!(direct_requirements.len(), 2); + assert!( + direct_requirements + .iter() + .all(|r| r.source == RequirementSource::CratesIo) + ); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Blocked { + newest_compliant, + blocker, + .. + } => { + assert_eq!(newest_compliant, "1.8.0"); + assert_eq!(blocker.req, "=1.9.0"); + } + _ => panic!( + "expected foo to be Blocked by the restrictive =1.9.0 declaration \ + alongside the lenient ^1.0 one" + ), + } +} + +/// "helper" is a path dependency of root "app", not a workspace +/// member. Cargo ignores the dev-dependencies of a non-member path +/// dependency, so helper's `[dev-dependencies] foo = "=1.9.0"` must +/// not block a downgrade that only helper's own `[dependencies] foo +/// = "1"` would allow. +#[test] +fn dev_dependency_of_a_non_member_path_dependency_does_not_block() { + let dir = tempdir().unwrap(); + write_manifest( + dir.path(), + r#" +[package] +name = "app" +version = "0.1.0" + +[dependencies] +helper = { path = "helper" } +"#, + ); + std::fs::create_dir_all(dir.path().join("helper")).unwrap(); + write_manifest( + &dir.path().join("helper"), + r#" +[package] +name = "helper" +version = "0.1.0" + +[dependencies] +foo = "1" + +[dev-dependencies] +foo = "=1.9.0" +"#, + ); + let (direct_requirements, warnings) = load_direct_requirements(dir.path()); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let transport = FakeTransport::default(); + transport.push( + &versions_url("foo"), + ScriptedResponse::Http( + 200, + versions_body(&[("1.9.0", 5, false), ("1.8.0", 50, false)]), + ), + ); + let mut client = fast_client(transport); + + let violations = vec![too_new("foo", "1.9.0")]; + let packages = vec![ + Package { + name: "foo".to_string(), + version: "1.9.0".to_string(), + is_registry: true, + source: Some(CRATES_IO_SOURCE.to_string()), + dependencies: vec![], + }, + Package { + name: "helper".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "foo".to_string(), + version: "1.9.0".to_string(), + source: Some(CRATES_IO_SOURCE.to_string()), + }], + }, + Package { + name: "app".to_string(), + version: "0.1.0".to_string(), + is_registry: false, + source: None, + dependencies: vec![PackageRef { + name: "helper".to_string(), + version: "0.1.0".to_string(), + source: None, + }], + }, + ]; + let outcomes = suggestions( + &mut client, + &violations, + &packages, + &direct_requirements, + dir.path(), + ); + + match &outcomes[0] { + Outcome::Suggest { + suggested_version, .. + } => { + assert_eq!(suggested_version, "1.8.0"); + } + Outcome::Blocked { blocker, .. } => panic!( + "expected foo to be Suggest, but was Blocked by {} \ + (helper's dev-dependency must be ignored: it isn't a workspace member)", + blocker.name + ), + _ => panic!("expected foo to be Suggest"), + } +} diff --git a/src/suggest/tests/source_collision_cargo_tests.rs b/src/suggest/tests/source_collision_cargo_tests.rs new file mode 100644 index 0000000..3b1c216 --- /dev/null +++ b/src/suggest/tests/source_collision_cargo_tests.rs @@ -0,0 +1,176 @@ +/// Builds a real Cargo project with a source collision — a crates.io +/// package and a path package sharing a name and locked version — and +/// runs a real `cargo` against the spec `build_package_spec` produces, +/// to verify it's the source-qualified pkgid Cargo itself expects, +/// rather than merely a string this crate assumes is valid. +use super::*; +use sha2::{Digest, Sha256}; +use std::process::Command; + +const CRATE_NAME: &str = "semver"; +const CRATE_VERSION: &str = "1.0.28"; + +/// Writes a minimal crate (`Cargo.toml` + `src/lib.rs`) at `dir`. +fn write_crate_source(dir: &Path, name: &str, version: &str) { + std::fs::create_dir_all(dir.join("src")).unwrap(); + std::fs::write( + dir.join("Cargo.toml"), + format!("[package]\nname = \"{name}\"\nversion = \"{version}\"\nedition = \"2021\"\n"), + ) + .unwrap(); + std::fs::write(dir.join("src/lib.rs"), "").unwrap(); +} + +/// Packs `crate_dir` (already containing a `{name}-{version}` +/// top-level directory) into a `.crate` tarball, Cargo's own +/// publish format. +fn pack_crate_tarball(crate_dir: &Path, name: &str, version: &str) -> Vec { + let mut bytes = Vec::new(); + { + let encoder = flate2::write::GzEncoder::new(&mut bytes, flate2::Compression::default()); + let mut builder = tar::Builder::new(encoder); + builder + .append_dir_all(format!("{name}-{version}"), crate_dir) + .unwrap(); + builder.finish().unwrap(); + } + bytes +} + +/// Assembles a local-registry source (see Cargo's source-replacement +/// docs) at `registry_dir`, containing one crate. Local-registry +/// index entries are sharded by name length: a 4+ character name +/// shards under its first two, then next two, characters. +fn write_local_registry(registry_dir: &Path, name: &str, version: &str) { + let build_dir = registry_dir + .join(".build") + .join(format!("{name}-{version}")); + write_crate_source(&build_dir, name, version); + let tarball = pack_crate_tarball(&build_dir, name, version); + + std::fs::write( + registry_dir.join(format!("{name}-{version}.crate")), + &tarball, + ) + .unwrap(); + + let cksum = Sha256::digest(&tarball) + .iter() + .map(|b| format!("{b:02x}")) + .collect::(); + let shard = registry_dir + .join("index") + .join(&name[0..2]) + .join(&name[2..4]); + std::fs::create_dir_all(&shard).unwrap(); + std::fs::write( + shard.join(name), + format!( + r#"{{"name":"{name}","vers":"{version}","deps":[],"cksum":"{cksum}","features":{{}},"yanked":false}}"# + ), + ) + .unwrap(); +} + +fn run_cargo(args: &[&str], cwd: &Path) -> std::process::Output { + Command::new("cargo") + .args(args) + .current_dir(cwd) + .output() + .expect("failed to run cargo") +} + +#[test] +fn qualified_spec_resolves_where_the_abbreviated_spec_is_ambiguous() { + let root = tempfile::tempdir().unwrap(); + let registry_dir = root.path().join("registry"); + let workspace_dir = root.path().join("workspace"); + + write_local_registry(®istry_dir, CRATE_NAME, CRATE_VERSION); + write_crate_source( + &workspace_dir.join("vendor-semver"), + CRATE_NAME, + CRATE_VERSION, + ); + + std::fs::create_dir_all(workspace_dir.join(".cargo")).unwrap(); + std::fs::write( + workspace_dir.join(".cargo/config.toml"), + format!( + "[source.local-vendor]\nlocal-registry = \"{}\"\n\n[source.crates-io]\nreplace-with = \"local-vendor\"\n", + registry_dir.display() + ), + ) + .unwrap(); + std::fs::create_dir_all(workspace_dir.join("src")).unwrap(); + std::fs::write(workspace_dir.join("src/main.rs"), "fn main() {}\n").unwrap(); + std::fs::write( + workspace_dir.join("Cargo.toml"), + format!( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[dependencies]\n{CRATE_NAME} = \"{CRATE_VERSION}\"\n{CRATE_NAME}-path = {{ package = \"{CRATE_NAME}\", path = \"vendor-semver\" }}\n" + ), + ) + .unwrap(); + + let lock = run_cargo(&["generate-lockfile", "--offline"], &workspace_dir); + assert!( + lock.status.success(), + "generate-lockfile failed: {}", + String::from_utf8_lossy(&lock.stderr) + ); + + let packages = crate::lockfile::load(Path::new("Cargo.lock"), &workspace_dir) + .unwrap() + .packages; + let target_source = packages + .iter() + .find(|p| p.name == CRATE_NAME && p.is_registry) + .and_then(|p| p.source.as_deref()); + let is_ambiguous = packages + .iter() + .filter(|p| p.name == CRATE_NAME && p.version == CRATE_VERSION) + .count() + > 1; + let spec = build_package_spec(CRATE_NAME, target_source, is_ambiguous); + assert!( + spec.contains('#'), + "expected a source-qualified spec for a name/version collision, got {spec}" + ); + + // The abbreviated spec really is ambiguous in this fixture — + // otherwise the qualified spec above proves nothing. + let abbreviated = run_cargo( + &[ + "update", + "--offline", + "-p", + &format!("{CRATE_NAME}@{CRATE_VERSION}"), + "--precise", + CRATE_VERSION, + ], + &workspace_dir, + ); + assert!( + !abbreviated.status.success() + && String::from_utf8_lossy(&abbreviated.stderr).contains("ambiguous"), + "expected the abbreviated spec to be ambiguous in this fixture: {}", + String::from_utf8_lossy(&abbreviated.stderr) + ); + + let qualified = run_cargo( + &[ + "update", + "--offline", + "-p", + &format!("{spec}@{CRATE_VERSION}"), + "--precise", + CRATE_VERSION, + ], + &workspace_dir, + ); + assert!( + qualified.status.success(), + "expected the source-qualified spec to resolve without an ambiguous-specification error: {}", + String::from_utf8_lossy(&qualified.stderr) + ); +} diff --git a/src/suggest/tests/walk_tests.rs b/src/suggest/tests/walk_tests.rs new file mode 100644 index 0000000..0a4396a --- /dev/null +++ b/src/suggest/tests/walk_tests.rs @@ -0,0 +1,96 @@ +use super::*; + +#[test] +fn newest_accepted_when_every_constraint_matches() { + let candidates = vec![(v("1.3.0"), 5), (v("1.2.0"), 20)]; + let constraints = vec![constraint("^1.2")]; + + match walk(candidates, constraints) { + WalkResult::Suggest(version, age) => { + assert_eq!(version.to_string(), "1.3.0"); + assert_eq!(age, 5); + } + _ => panic!("expected Suggest"), + } +} + +#[test] +fn walk_continues_to_older_version_when_newest_is_rejected() { + let candidates = vec![(v("1.3.0"), 5), (v("1.2.0"), 20), (v("1.1.0"), 40)]; + // `~1.1` narrows to the 1.1.x line, so only the oldest candidate + // satisfies it — the walk must skip past the two newer ones. + let constraints = vec![constraint("~1.1")]; + match walk(candidates, constraints) { + WalkResult::Suggest(version, _) => assert_eq!(version.to_string(), "1.1.0"), + _ => panic!("expected Suggest"), + } +} + +#[test] +fn blocked_when_no_candidate_satisfies_every_constraint() { + let candidates = vec![(v("1.3.0"), 5), (v("1.2.0"), 20)]; + let constraints = vec![constraint("^2.0")]; + + match walk(candidates, constraints) { + WalkResult::Blocked { + newest_compliant, + blocker, + } => { + assert_eq!(newest_compliant.to_string(), "1.3.0"); + assert_eq!(blocker.blocker_name, "dep"); + assert_eq!(blocker.req.to_string(), "^2.0"); + } + _ => panic!("expected Blocked"), + } +} + +#[test] +fn blocker_is_the_constraint_that_rejects_every_candidate() { + // `<=1.3` comes first and rejects the newest candidate, but 1.2.0 + // satisfies it — only `>=1.4.5` makes every downgrade impossible. + let candidates = vec![(v("1.4.0"), 5), (v("1.2.0"), 20)]; + let constraints = vec![constraint("<=1.3"), constraint(">=1.4.5")]; + + match walk(candidates, constraints) { + WalkResult::Blocked { blocker, .. } => { + assert_eq!(blocker.req.to_string(), ">=1.4.5"); + } + _ => panic!("expected Blocked"), + } +} + +#[test] +fn blocker_falls_back_when_no_single_constraint_blocks_all_candidates() { + // When no single constraint rejects every candidate, the block is + // a genuine combination, so we fall back to the first constraint + // that rejects the newest candidate. + let candidates = vec![(v("1.4.0"), 5), (v("1.2.0"), 20)]; + let constraints = vec![constraint("<=1.3"), constraint(">=1.4")]; + + match walk(candidates, constraints) { + WalkResult::Blocked { blocker, .. } => { + assert_eq!(blocker.req.to_string(), "<=1.3"); + } + _ => panic!("expected Blocked"), + } +} + +#[test] +fn no_compliant_version_when_candidates_empty() { + match walk(vec![], vec![constraint("^1.0")]) { + WalkResult::NoCompliantVersion => {} + _ => panic!("expected NoCompliantVersion"), + } +} + +#[test] +fn no_constraints_picks_newest_by_age() { + let candidates = vec![(v("1.3.0"), 5), (v("1.2.0"), 20)]; + match walk(candidates, vec![]) { + WalkResult::Suggest(version, age) => { + assert_eq!(version.to_string(), "1.3.0"); + assert_eq!(age, 5); + } + _ => panic!("expected Suggest"), + } +} diff --git a/tests/fixtures/suggest_fix_e2e/Cargo.lock b/tests/fixtures/suggest_fix_e2e/Cargo.lock new file mode 100644 index 0000000..5bc11b8 --- /dev/null +++ b/tests/fixtures/suggest_fix_e2e/Cargo.lock @@ -0,0 +1,42 @@ +version = 4 + +[[package]] +name = "app" +version = "0.1.0" +dependencies = [ + "alpha", + "beta", +] + +[[package]] +name = "alpha" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" + +[[package]] +name = "beta" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" + +[[package]] +name = "gamma" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" + +[[package]] +name = "delta" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" + +[[package]] +name = "consumer" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" +dependencies = [ + "gamma", +] diff --git a/tests/fixtures/suggest_fix_e2e/Cargo.toml b/tests/fixtures/suggest_fix_e2e/Cargo.toml new file mode 100644 index 0000000..cfb525e --- /dev/null +++ b/tests/fixtures/suggest_fix_e2e/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = ["app"] diff --git a/tests/fixtures/suggest_fix_e2e/app/Cargo.toml b/tests/fixtures/suggest_fix_e2e/app/Cargo.toml new file mode 100644 index 0000000..bfee6e9 --- /dev/null +++ b/tests/fixtures/suggest_fix_e2e/app/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "app" +version = "0.1.0" +edition = "2021" + +[dependencies] +alpha = "1.0" +beta = "^1.5" diff --git a/tests/suggest_fix_cli.rs b/tests/suggest_fix_cli.rs new file mode 100644 index 0000000..0957e69 --- /dev/null +++ b/tests/suggest_fix_cli.rs @@ -0,0 +1,541 @@ +use chrono::{Duration, Utc}; +use flate2::write::GzEncoder; +use sha2::{Digest, Sha256}; +use std::fs; +use std::path::Path; +use std::process::{Command, Output}; +use tar::Builder; +use tempfile::{TempDir, tempdir}; + +const BIN: &str = env!("CARGO_BIN_EXE_cargo-oxidate"); +type CacheEntry<'a> = (&'a str, &'a str, i64, Vec<(&'a str, i64)>); + +fn run_oxidate(cwd: &Path, args: &[&str]) -> Output { + Command::new(BIN) + .args(args) + .current_dir(cwd) + .output() + .expect("cargo-oxidate should run") +} + +fn write_cache(dir: &Path, entries: &[CacheEntry<'_>]) -> std::path::PathBuf { + let now = Utc::now(); + let mut publish_dates = serde_json::Map::new(); + let mut all_versions = serde_json::Map::new(); + let mut index_records = serde_json::Map::new(); + + for (name, locked, locked_age, versions) in entries { + publish_dates.insert( + format!("{name}/{locked}"), + serde_json::Value::String((now - Duration::days(*locked_age)).to_rfc3339()), + ); + all_versions.insert( + (*name).to_string(), + serde_json::json!({ + "fetched_at": now, + "versions": versions.iter().map(|(version, age)| serde_json::json!({ + "num": version, + "created_at": now - Duration::days(*age), + "yanked": false, + })).collect::>(), + }), + ); + index_records.insert( + (*name).to_string(), + serde_json::json!({ + "fetched_at": now, + "records": [{ "vers": locked, "yanked": false, "deps": [] }], + }), + ); + } + + let path = dir.join("responses.json"); + fs::write( + &path, + serde_json::to_vec(&serde_json::json!({ + "version": 1, + "publish_dates": publish_dates, + "all_versions": all_versions, + "index_records": index_records, + })) + .unwrap(), + ) + .unwrap(); + path +} + +fn copied_fixture() -> TempDir { + let temp = tempdir().unwrap(); + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/suggest_fix_e2e"); + fs::copy( + fixture.join("app/Cargo.toml"), + temp.path().join("Cargo.toml"), + ) + .unwrap(); + fs::copy(fixture.join("Cargo.lock"), temp.path().join("Cargo.lock")).unwrap(); + temp +} + +#[test] +fn suggest_fix_cli_reports_mixed_outcomes_and_preserves_project_files() { + let project = copied_fixture(); + let cache = write_cache( + project.path(), + &[ + ("alpha", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("beta", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("gamma", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("delta", "1.5.0", 2, vec![("1.5.0", 2)]), + ("consumer", "2.0.0", 100, vec![("2.0.0", 100)]), + ], + ); + let mut cache_json: serde_json::Value = + serde_json::from_slice(&fs::read(&cache).unwrap()).unwrap(); + cache_json["index_records"]["consumer"]["records"] = serde_json::json!([{ + "vers": "2.0.0", + "yanked": false, + "deps": [ + { "name": "gamma", "req": "^1.5", "kind": null, "target": null, "optional": false, "package": null } + ] + }]); + fs::write(&cache, serde_json::to_vec(&cache_json).unwrap()).unwrap(); + + let manifest_before = fs::read(project.path().join("Cargo.toml")).unwrap(); + let lock_before = fs::read(project.path().join("Cargo.lock")).unwrap(); + let output = run_oxidate( + project.path(), + &[ + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("cargo update -p alpha@1.5.0 --precise 1.4.0"), + "stdout was:\n{stdout}" + ); + assert!( + stdout.contains("beta 1.5.0") && stdout.contains("Cargo.toml") && stdout.contains("^1.5"), + "stdout was:\n{stdout}" + ); + assert!( + stdout.contains("gamma 1.5.0") + && stdout.contains("consumer 2.0.0") + && stdout.contains("^1.5") + ); + assert!( + stdout.contains( + "delta 1.5.0: no eligible downgrade at least the minimum age old within its compatible range" + ), + "stdout was:\n{stdout}" + ); + assert!(stdout.contains("best-effort")); + assert_eq!( + fs::read(project.path().join("Cargo.toml")).unwrap(), + manifest_before + ); + assert_eq!( + fs::read(project.path().join("Cargo.lock")).unwrap(), + lock_before + ); +} + +#[test] +fn suggest_fix_cli_reports_excluded_lower_versions_as_no_eligible_downgrade() { + for (candidate, yanked) in [("1.4.0", true), ("1.4.0-beta.1", false)] { + let project = copied_fixture(); + let cache = write_cache( + project.path(), + &[ + ("alpha", "1.5.0", 100, vec![]), + ("beta", "1.5.0", 100, vec![]), + ("gamma", "1.5.0", 100, vec![]), + ("delta", "1.5.0", 2, vec![("1.5.0", 2), (candidate, 80)]), + ("consumer", "2.0.0", 100, vec![]), + ], + ); + let mut cache_json: serde_json::Value = + serde_json::from_slice(&fs::read(&cache).unwrap()).unwrap(); + cache_json["all_versions"]["delta"]["versions"][1]["yanked"] = serde_json::json!(yanked); + fs::write(&cache, serde_json::to_vec(&cache_json).unwrap()).unwrap(); + + let output = run_oxidate( + project.path(), + &[ + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.lines().any(|line| { + line + == " delta 1.5.0: no eligible downgrade at least the minimum age old within its compatible range" + }), + "stdout was:\n{stdout}" + ); + assert!( + !stdout + .lines() + .any(|line| line.trim_start().starts_with("cargo update -p delta@")), + "stdout was:\n{stdout}" + ); + } +} + +#[test] +fn suggest_fix_cli_retains_best_effort_qualification() { + let project = tempdir().unwrap(); + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/suggest_fix_e2e"); + fs::copy( + fixture.join("app/Cargo.toml"), + project.path().join("Cargo.toml"), + ) + .unwrap(); + let lock = fs::read_to_string(fixture.join("Cargo.lock")).unwrap(); + let epsilon = "\n[[package]]\nname = \"epsilon\"\nversion = \"1.5.0\"\nsource = \"registry+https://github.com/rust-lang/crates.io-index\"\nchecksum = \"0000000000000000000000000000000000000000000000000000000000000000\"\n"; + fs::write( + project.path().join("Cargo.lock"), + lock.replace( + " \"gamma\",\n]", + " \"gamma\",\n \"epsilon 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)\",\n]", + ) + epsilon, + ) + .unwrap(); + let cache = write_cache( + project.path(), + &[ + ("alpha", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("beta", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("gamma", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("delta", "1.5.0", 2, vec![("1.5.0", 2)]), + ("epsilon", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)]), + ("consumer", "2.0.0", 100, vec![("2.0.0", 100)]), + ], + ); + let mut cache_json: serde_json::Value = + serde_json::from_slice(&fs::read(&cache).unwrap()).unwrap(); + cache_json["index_records"]["consumer"]["records"] = serde_json::json!([{ + "vers": "1.0.0", "yanked": false, "deps": [] + }]); + fs::write(&cache, serde_json::to_vec(&cache_json).unwrap()).unwrap(); + let output = run_oxidate( + project.path(), + &[ + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("requirement of consumer unverified"), + "stdout was:\n{stdout}" + ); + assert!(stdout.contains("best-effort"), "stdout was:\n{stdout}"); + assert!( + stdout.contains("apply top to bottom, then re-run"), + "stdout was:\n{stdout}" + ); + assert!( + stdout.contains("or test after applying"), + "stdout was:\n{stdout}" + ); +} + +#[test] +fn suggest_fix_cli_reports_same_named_git_parent_once() { + let project = tempdir().unwrap(); + fs::write( + project.path().join("Cargo.toml"), + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + fs::write( + project.path().join("Cargo.lock"), + r#"version = 4 + +[[package]] +name = "app" +version = "0.1.0" + +[[package]] +name = "parent" +version = "1.0.0" +source = "git+https://github.com/example/parent?tag=v1#1111111111111111111111111111111111111111" +dependencies = [ + "foo", +] + +[[package]] +name = "parent" +version = "2.0.0" +source = "git+https://github.com/example/parent?tag=v2#2222222222222222222222222222222222222222" +dependencies = [ + "foo", +] + +[[package]] +name = "foo" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0000000000000000000000000000000000000000000000000000000000000000" +"#, + ) + .unwrap(); + let cache = write_cache( + project.path(), + &[("foo", "1.9.0", 2, vec![("1.9.0", 2), ("1.7.0", 80)])], + ); + let output = run_oxidate( + project.path(), + &[ + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("cargo update -p foo@1.9.0 --precise 1.7.0"), + "stdout was:\n{stdout}" + ); + assert!( + stdout.contains("(requirement of parent unverified)"), + "stdout was:\n{stdout}" + ); + assert!(!stdout.contains("parent, parent"), "stdout was:\n{stdout}"); +} + +#[cfg(unix)] +#[test] +fn suggest_fix_uses_manifest_beside_the_symlinks_real_lockfile() { + // `decoy/Cargo.lock` is a symlink to `real/Cargo.lock`. Manifest discovery + // must follow the canonical path, or the decoy's permissive manifest + // (which doesn't declare `alpha`) would let the downgrade through. + let project = tempdir().unwrap(); + let real_dir = project.path().join("real"); + let decoy_dir = project.path().join("decoy"); + fs::create_dir_all(&real_dir).unwrap(); + fs::create_dir_all(&decoy_dir).unwrap(); + + fs::write( + real_dir.join("Cargo.toml"), + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[dependencies]\nalpha = \"^1.5\"\n", + ) + .unwrap(); + fs::write( + real_dir.join("Cargo.lock"), + "version = 4\n\n[[package]]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\n \"alpha\",\n]\n\n[[package]]\nname = \"alpha\"\nversion = \"1.5.0\"\nsource = \"registry+https://github.com/rust-lang/crates.io-index\"\nchecksum = \"0000000000000000000000000000000000000000000000000000000000000000\"\n", + ) + .unwrap(); + + fs::write( + decoy_dir.join("Cargo.toml"), + "[package]\nname = \"decoy\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + std::os::unix::fs::symlink(real_dir.join("Cargo.lock"), decoy_dir.join("Cargo.lock")).unwrap(); + + let cache = write_cache( + project.path(), + &[("alpha", "1.5.0", 2, vec![("1.5.0", 2), ("1.4.0", 80)])], + ); + let output = run_oxidate( + project.path(), + &[ + "decoy/Cargo.lock", + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + !stdout.contains("cargo update -p alpha@1.5.0 --precise 1.4.0"), + "forbidden downgrade command was suggested; stdout was:\n{stdout}" + ); + assert!( + stdout.contains("alpha 1.5.0") && stdout.contains("Cargo.toml") && stdout.contains("^1.5"), + "expected the real manifest's restriction on alpha to be reported; stdout was:\n{stdout}" + ); +} + +#[test] +fn ordinary_and_invalid_cli_runs_keep_their_exit_contract() { + let project = tempdir().unwrap(); + fs::write( + project.path().join("Cargo.toml"), + "[package]\nname = \"clean\"\nversion = \"0.1.0\"\nedition = \"2024\"\n", + ) + .unwrap(); + fs::write( + project.path().join("Cargo.lock"), + "version = 4\n\n[[package]]\nname = \"clean\"\nversion = \"0.1.0\"\n", + ) + .unwrap(); + + let clean = run_oxidate(project.path(), &["--min-age-days", "30"]); + assert!(clean.status.success()); + assert!( + !String::from_utf8(clean.stdout) + .unwrap() + .contains("Suggested fixes") + ); + + let invalid = run_oxidate(project.path(), &["--suggest-fix"]); + assert_eq!(invalid.status.code(), Some(2)); +} + +#[test] +fn suggest_fix_with_no_too_new_violations_does_not_warn_about_a_missing_manifest() { + // No Cargo.toml beside the lockfile, and no registry packages to + // check, so there is nothing to suggest a fix for. `--suggest-fix` + // must not still load (and warn about) direct requirements when + // there are no "too new" violations to act on. + let project = tempdir().unwrap(); + fs::write( + project.path().join("Cargo.lock"), + "version = 4\n\n[[package]]\nname = \"clean\"\nversion = \"0.1.0\"\n", + ) + .unwrap(); + + let output = run_oxidate(project.path(), &["--suggest-fix", "--min-age-days", "30"]); + assert!(output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!(!stderr.contains("No Cargo.toml"), "stderr was:\n{stderr}"); +} + +fn write_crate_source(dir: &Path, name: &str, version: &str) { + fs::create_dir_all(dir.join("src")).unwrap(); + fs::write( + dir.join("Cargo.toml"), + format!("[package]\nname = \"{name}\"\nversion = \"{version}\"\nedition = \"2021\"\n"), + ) + .unwrap(); + fs::write(dir.join("src/lib.rs"), "").unwrap(); +} + +fn write_local_registry(registry: &Path, name: &str, version: &str) { + let source = registry.join(".build").join(format!("{name}-{version}")); + write_crate_source(&source, name, version); + let mut bytes = Vec::new(); + let encoder = GzEncoder::new(&mut bytes, flate2::Compression::default()); + let mut tar = Builder::new(encoder); + tar.append_dir_all(format!("{name}-{version}"), &source) + .unwrap(); + tar.into_inner().unwrap().finish().unwrap(); + fs::write(registry.join(format!("{name}-{version}.crate")), &bytes).unwrap(); + let checksum = Sha256::digest(&bytes) + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + let shard = registry.join("index").join(&name[..2]).join(&name[2..4]); + fs::create_dir_all(&shard).unwrap(); + let entry = serde_json::json!({"name": name, "vers": version, "deps": [], "cksum": checksum, "features": {}, "yanked": false}); + use std::io::Write; + fs::OpenOptions::new() + .create(true) + .append(true) + .open(shard.join(name)) + .unwrap() + .write_all(format!("{entry}\n").as_bytes()) + .unwrap(); +} + +#[test] +fn printed_source_qualified_command_updates_only_the_registry_package() { + let root = tempdir().unwrap(); + let registry = root.path().join("registry"); + let project = root.path().join("project"); + write_local_registry(®istry, "semver", "1.0.28"); + write_local_registry(®istry, "semver", "1.0.27"); + write_crate_source(&project.join("vendor-semver"), "semver", "1.0.28"); + fs::create_dir_all(project.join(".cargo")).unwrap(); + fs::create_dir_all(project.join("src")).unwrap(); + fs::write(project.join("src/main.rs"), "fn main() {}\n").unwrap(); + fs::write(project.join(".cargo/config.toml"), format!("[source.local-vendor]\nlocal-registry = \"{}\"\n\n[source.crates-io]\nreplace-with = \"local-vendor\"\n", registry.display())).unwrap(); + fs::write(project.join("Cargo.toml"), "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[dependencies]\nsemver = \"1\"\nsemver-path = { package = \"semver\", path = \"vendor-semver\" }\n").unwrap(); + let generated = Command::new("cargo") + .args(["generate-lockfile", "--offline"]) + .current_dir(&project) + .output() + .unwrap(); + assert!( + generated.status.success(), + "{}", + String::from_utf8_lossy(&generated.stderr) + ); + + let cache = write_cache( + &project, + &[("semver", "1.0.28", 2, vec![("1.0.28", 2), ("1.0.27", 80)])], + ); + let output = run_oxidate( + &project, + &[ + "--min-age-days", + "30", + "--suggest-fix", + "--cache-path", + cache.to_str().unwrap(), + ], + ); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8(output.stdout).unwrap(); + let line = stdout + .lines() + .find(|line| line.trim_start().starts_with("cargo update ")) + .unwrap_or_else(|| panic!("a rendered cargo update command; stdout was:\n{stdout}")); + let command = line.split(" #").next().unwrap().trim(); + let args: Vec<_> = command.split_whitespace().skip(1).collect(); + assert!( + args.iter().any(|arg| arg.contains('#')), + "expected source-qualified package spec: {command}" + ); + let applied = Command::new("cargo") + .args(args) + .arg("--offline") + .current_dir(&project) + .output() + .unwrap(); + assert!( + applied.status.success(), + "{}", + String::from_utf8_lossy(&applied.stderr) + ); + + let lock = cargo_lock::Lockfile::load(project.join("Cargo.lock")).unwrap(); + assert!( + lock.packages + .iter() + .any(|package| package.name.as_str() == "semver" + && package.version.to_string() == "1.0.27" + && package.source.is_some()) + ); + assert!( + lock.packages + .iter() + .any(|package| package.name.as_str() == "semver" + && package.version.to_string() == "1.0.28" + && package.source.is_none()) + ); +}