From b04c7fb849de72f14cdddb5da7c123abfc5fa6e4 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 22 Jan 2021 13:37:52 -0800 Subject: [PATCH] Add suggestion for bad package id. --- src/cargo/core/package_id_spec.rs | 59 ++++++++++++++++++++++++---- src/cargo/ops/cargo_clean.rs | 13 ++++++- tests/testsuite/build.rs | 4 +- tests/testsuite/clean.rs | 13 +++++++ tests/testsuite/install.rs | 6 +-- tests/testsuite/pkgid.rs | 65 +++++++++++++++++++++++++++++++ 6 files changed, 147 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 57a2db61436..daa3c73ee74 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::fmt; +use anyhow::bail; use semver::Version; use serde::{de, ser}; use url::Url; @@ -8,6 +9,7 @@ use url::Url; use crate::core::PackageId; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::interning::InternedString; +use crate::util::lev_distance; use crate::util::{validate_package_name, IntoUrl, ToSemver}; /// Some or all of the data required to identify a package: @@ -96,7 +98,7 @@ impl PackageIdSpec { /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { if url.query().is_some() { - anyhow::bail!("cannot have a query string in a pkgid: {}", url) + bail!("cannot have a query string in a pkgid: {}", url) } let frag = url.fragment().map(|s| s.to_owned()); url.set_fragment(None); @@ -180,14 +182,57 @@ impl PackageIdSpec { where I: IntoIterator, { - let mut ids = i.into_iter().filter(|p| self.matches(*p)); + let all_ids: Vec<_> = i.into_iter().collect(); + let mut ids = all_ids.iter().copied().filter(|&id| self.matches(id)); let ret = match ids.next() { Some(id) => id, - None => anyhow::bail!( - "package ID specification `{}` \ - matched no packages", - self - ), + None => { + let mut suggestion = String::new(); + let try_spec = |spec: PackageIdSpec, suggestion: &mut String| { + let try_matches: Vec<_> = all_ids + .iter() + .copied() + .filter(|&id| spec.matches(id)) + .collect(); + if !try_matches.is_empty() { + suggestion.push_str("\nDid you mean one of these?\n"); + minimize(suggestion, &try_matches, self); + } + }; + if self.url.is_some() { + try_spec( + PackageIdSpec { + name: self.name, + version: self.version.clone(), + url: None, + }, + &mut suggestion, + ); + } + if suggestion.is_empty() && self.version.is_some() { + try_spec( + PackageIdSpec { + name: self.name, + version: None, + url: None, + }, + &mut suggestion, + ); + } + if suggestion.is_empty() { + suggestion.push_str(&lev_distance::closest_msg( + &self.name, + all_ids.iter(), + |id| id.name().as_str(), + )); + } + + bail!( + "package ID specification `{}` did not match any packages{}", + self, + suggestion + ); + } }; return match ids.next() { Some(other) => { diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 68f82b3c146..ba5daad5921 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -4,6 +4,7 @@ use crate::core::{PackageIdSpec, TargetKind, Workspace}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::interning::InternedString; +use crate::util::lev_distance; use crate::util::paths; use crate::util::Config; use std::fs; @@ -119,7 +120,17 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { } let matches: Vec<_> = resolve.iter().filter(|id| spec.matches(*id)).collect(); if matches.is_empty() { - anyhow::bail!("package ID specification `{}` matched no packages", spec); + let mut suggestion = String::new(); + suggestion.push_str(&lev_distance::closest_msg( + &spec.name(), + resolve.iter(), + |id| id.name().as_str(), + )); + anyhow::bail!( + "package ID specification `{}` did not match any packages{}", + spec, + suggestion + ); } pkg_ids.extend(matches); } diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index e468106aab7..b4e87aa81f3 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -3063,12 +3063,12 @@ fn invalid_spec() { p.cargo("build -p notAValidDep") .with_status(101) - .with_stderr("[ERROR] package ID specification `notAValidDep` matched no packages") + .with_stderr("[ERROR] package ID specification `notAValidDep` did not match any packages") .run(); p.cargo("build -p d1 -p notAValidDep") .with_status(101) - .with_stderr("[ERROR] package ID specification `notAValidDep` matched no packages") + .with_stderr("[ERROR] package ID specification `notAValidDep` did not match any packages") .run(); } diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index 49986070b85..f79619b15d2 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -466,6 +466,19 @@ fn clean_spec_multiple() { .build(); p.cargo("build").run(); + + // Check suggestion for bad pkgid. + p.cargo("clean -p baz") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `baz` did not match any packages + +Did you mean `bar`? +", + ) + .run(); + p.cargo("clean -p bar:1.0.0") .with_stderr( "warning: version qualifier in `-p bar:1.0.0` is ignored, \ diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 070c39008ae..5264bb11a40 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -757,7 +757,7 @@ Caused by: fn uninstall_pkg_does_not_exist() { cargo_process("uninstall foo") .with_status(101) - .with_stderr("[ERROR] package ID specification `foo` matched no packages") + .with_stderr("[ERROR] package ID specification `foo` did not match any packages") .run(); } @@ -797,7 +797,7 @@ fn uninstall_piecemeal() { cargo_process("uninstall foo") .with_status(101) - .with_stderr("[ERROR] package ID specification `foo` matched no packages") + .with_stderr("[ERROR] package ID specification `foo` did not match any packages") .run(); } @@ -1205,7 +1205,7 @@ fn uninstall_multiple_and_some_pkg_does_not_exist() { .with_stderr( "\ [REMOVING] [CWD]/home/.cargo/bin/foo[EXE] -error: package ID specification `bar` matched no packages +error: package ID specification `bar` did not match any packages [SUMMARY] Successfully uninstalled foo! Failed to uninstall bar (see error(s) above). error: some packages failed to uninstall ", diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 3ff564ed9af..93ee9eeddfb 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -32,3 +32,68 @@ fn simple() { .with_stdout("https://github.com/rust-lang/crates.io-index#bar:0.1.0") .run(); } + +#[cargo_test] +fn suggestion_bad_pkgid() { + Package::new("crates-io", "0.1.0").publish(); + Package::new("two-ver", "0.1.0").publish(); + Package::new("two-ver", "0.2.0").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2018" + + [dependencies] + crates-io = "0.1.0" + two-ver = "0.1.0" + two-ver2 = { package = "two-ver", version = "0.2.0" } + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile").run(); + + // Bad URL. + p.cargo("pkgid https://example.com/crates-io") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `https://example.com/crates-io` did not match any packages +Did you mean one of these? + + crates-io:0.1.0 +", + ) + .run(); + + // Bad name. + p.cargo("pkgid crates_io") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `crates_io` did not match any packages + +Did you mean `crates-io`? +", + ) + .run(); + + // Bad version. + p.cargo("pkgid two-ver:0.3.0") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `two-ver:0.3.0` did not match any packages +Did you mean one of these? + + two-ver:0.1.0 + two-ver:0.2.0 +", + ) + .run(); +}