From 8c5dd7b1745e710ace3718624855eaa71bf9801b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 14:33:02 -0600 Subject: [PATCH 01/12] refactor(add): Pull out summary lookup logic --- src/cargo/ops/cargo_add/mod.rs | 67 +++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index b36e4e72955..a04bfbf39f2 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -615,39 +615,26 @@ fn get_latest_dependency( })?; if gctx.cli_unstable().msrv_policy && honor_rust_version { - fn parse_msrv(comp: &RustVersion) -> (u64, u64, u64) { - (comp.major, comp.minor.unwrap_or(0), comp.patch.unwrap_or(0)) - } - - if let Some(req_msrv) = spec.rust_version().map(parse_msrv) { + if let Some(req_msrv) = spec.rust_version() { let msrvs = possibilities .iter() - .map(|s| (s, s.rust_version().map(parse_msrv))) + .map(|s| (s, s.rust_version())) .collect::>(); // Find the latest version of the dep which has a compatible rust-version. To // determine whether or not one rust-version is compatible with another, we // compare the lowest possible versions they could represent, and treat // candidates without a rust-version as compatible by default. - let (latest_msrv, _) = msrvs - .iter() - .filter(|(_, v)| v.map(|msrv| req_msrv >= msrv).unwrap_or(true)) - .last() - .ok_or_else(|| { - // Failing that, try to find the highest version with the lowest - // rust-version to report to the user. - let lowest_candidate = msrvs - .iter() - .min_set_by_key(|(_, v)| v) - .iter() - .map(|(s, _)| s) - .max_by_key(|s| s.version()); - rust_version_incompat_error( - &dependency.name, - spec.rust_version().unwrap(), - lowest_candidate.copied(), - ) - })?; + let latest_msrv = latest_compatible(&msrvs, req_msrv).ok_or_else(|| { + // Failing that, try to find the highest version with the lowest + // rust-version to report to the user. + let lowest_candidate = lowest_msrv(&msrvs); + rust_version_incompat_error( + &dependency.name, + spec.rust_version().unwrap(), + lowest_candidate, + ) + })?; if latest_msrv.version() < latest.version() { gctx.shell().warn(format_args!( @@ -673,6 +660,36 @@ fn get_latest_dependency( } } +/// Of MSRV-compatible summaries, find the highest version +/// +/// Assumptions: +/// - `msrvs` is sorted by version +fn latest_compatible<'s>( + msrvs: &[(&'s Summary, Option<&RustVersion>)], + req_msrv: &RustVersion, +) -> Option<&'s Summary> { + msrvs + .iter() + .filter(|(_, v)| v.as_ref().map(|msrv| req_msrv >= *msrv).unwrap_or(true)) + .map(|(s, _)| s) + .last() + .copied() +} + +/// Find the lowest MSRV summaries and pick the highest version +/// +/// Assumptions: +/// - `msrvs` is sorted by version +fn lowest_msrv<'s>(msrvs: &[(&'s Summary, Option<&RustVersion>)]) -> Option<&'s Summary> { + msrvs + .iter() + .min_set_by_key(|(_, v)| v) + .iter() + .map(|(s, _)| s) + .last() + .copied() +} + fn rust_version_incompat_error( dep: &str, rust_version: &RustVersion, From 1de98370214f7feee187f89ec05924ee808ff552 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 14:43:12 -0600 Subject: [PATCH 02/12] refactor(add): Simplify code based on assumptions --- src/cargo/ops/cargo_add/mod.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index a04bfbf39f2..5d0adb79b07 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -628,7 +628,8 @@ fn get_latest_dependency( let latest_msrv = latest_compatible(&msrvs, req_msrv).ok_or_else(|| { // Failing that, try to find the highest version with the lowest // rust-version to report to the user. - let lowest_candidate = lowest_msrv(&msrvs); + let lowest_candidate = lowest_msrv(&msrvs) + .expect("already checked that at least one is present"); rust_version_incompat_error( &dependency.name, spec.rust_version().unwrap(), @@ -693,26 +694,19 @@ fn lowest_msrv<'s>(msrvs: &[(&'s Summary, Option<&RustVersion>)]) -> Option<&'s fn rust_version_incompat_error( dep: &str, rust_version: &RustVersion, - lowest_rust_version: Option<&Summary>, + suggested_summary: &Summary, ) -> anyhow::Error { - let mut error_msg = format!( - "could not find version of crate `{dep}` that satisfies this package's rust-version of \ - {rust_version}\n\ - help: use `--ignore-rust-version` to override this behavior" + let suggested_version = suggested_summary.version(); + let suggested_rust_version = suggested_summary.rust_version().unwrap(); + let error_msg = format!( + "\ +could not find version of crate `{dep}` that satisfies this package's rust-version of {rust_version} +help: use `--ignore-rust-version` to override this behavior +note: the lowest rust-version available for `{dep}` is {suggested_rust_version}, used in version {suggested_version} +" ); - if let Some(lowest) = lowest_rust_version { - // rust-version must be present for this candidate since it would have been selected as - // compatible previously if it weren't. - let version = lowest.version(); - let rust_version = lowest.rust_version().unwrap(); - error_msg.push_str(&format!( - "\nnote: the lowest rust-version available for `{dep}` is {rust_version}, used in \ - version {version}" - )); - } - - anyhow::format_err!(error_msg) + anyhow::Error::msg(error_msg) } fn select_package( From a0e02cc5caa3798c47db9c126b65ad7772bdef0b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 14:45:49 -0600 Subject: [PATCH 03/12] refactor(add): Clarify why an unwrap is safe --- src/cargo/ops/cargo_add/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 5d0adb79b07..016540d849b 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -697,13 +697,14 @@ fn rust_version_incompat_error( suggested_summary: &Summary, ) -> anyhow::Error { let suggested_version = suggested_summary.version(); - let suggested_rust_version = suggested_summary.rust_version().unwrap(); + let suggested_rust_version = suggested_summary + .rust_version() + .expect("`latest_compatible should pick `None` cases, so we shouldn't get in here"); let error_msg = format!( "\ could not find version of crate `{dep}` that satisfies this package's rust-version of {rust_version} help: use `--ignore-rust-version` to override this behavior -note: the lowest rust-version available for `{dep}` is {suggested_rust_version}, used in version {suggested_version} -" +note: the lowest rust-version available for `{dep}` is {suggested_rust_version}, used in version {suggested_version}" ); anyhow::Error::msg(error_msg) From 64c7fbf44d8ba3570291bc68708697c82c6cd143 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 14:55:00 -0600 Subject: [PATCH 04/12] refactor(add): Clarify formatting of warning --- src/cargo/ops/cargo_add/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 016540d849b..a8e27b377e2 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -639,9 +639,8 @@ fn get_latest_dependency( if latest_msrv.version() < latest.version() { gctx.shell().warn(format_args!( - "ignoring `{dependency}@{latest_version}` (which has a rust-version of \ - {latest_rust_version}) to satisfy this package's rust-version of \ - {rust_version} (use `--ignore-rust-version` to override)", + "\ +ignoring `{dependency}@{latest_version}` (which has a rust-version of {latest_rust_version}) to satisfy this package's rust-version of {rust_version} (use `--ignore-rust-version` to override)", latest_version = latest.version(), latest_rust_version = latest.rust_version().unwrap(), rust_version = spec.rust_version().unwrap(), From 5476e544a755be25d9925c9f8b11bafc97d3657a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 15:12:10 -0600 Subject: [PATCH 05/12] fix(add): Clarify adding-of-old warning The `note:` was removed because `--ignore-rust-version` is a bit too late (dependency was already added). --- src/cargo/ops/cargo_add/mod.rs | 8 ++++---- .../cargo_add/rust_version_older/stderr.term.svg | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index a8e27b377e2..573cb0dd143 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -638,12 +638,12 @@ fn get_latest_dependency( })?; if latest_msrv.version() < latest.version() { + let latest_version = latest.version(); + let latest_rust_version = latest.rust_version().unwrap(); + let name = spec.name(); gctx.shell().warn(format_args!( "\ -ignoring `{dependency}@{latest_version}` (which has a rust-version of {latest_rust_version}) to satisfy this package's rust-version of {rust_version} (use `--ignore-rust-version` to override)", - latest_version = latest.version(), - latest_rust_version = latest.rust_version().unwrap(), - rust_version = spec.rust_version().unwrap(), +ignoring {dependency}@{latest_version} (which requires rustc {latest_rust_version}) to maintain {name}'s rust-version of {req_msrv}", ))?; latest = latest_msrv; diff --git a/tests/testsuite/cargo_add/rust_version_older/stderr.term.svg b/tests/testsuite/cargo_add/rust_version_older/stderr.term.svg index 904b2013f1c..bccac411676 100644 --- a/tests/testsuite/cargo_add/rust_version_older/stderr.term.svg +++ b/tests/testsuite/cargo_add/rust_version_older/stderr.term.svg @@ -1,4 +1,4 @@ - +