Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add suggestion for bad package id. #9095

Merged
merged 1 commit into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use std::collections::HashMap;
use std::fmt;

use anyhow::bail;
use semver::Version;
use serde::{de, ser};
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:
Expand Down Expand Up @@ -96,7 +98,7 @@ impl PackageIdSpec {
/// Tries to convert a valid `Url` to a `PackageIdSpec`.
fn from_url(mut url: Url) -> CargoResult<PackageIdSpec> {
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);
Expand Down Expand Up @@ -180,14 +182,57 @@ impl PackageIdSpec {
where
I: IntoIterator<Item = PackageId>,
{
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) => {
Expand Down
13 changes: 12 additions & 1 deletion src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
13 changes: 13 additions & 0 deletions tests/testsuite/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

<tab>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, \
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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
",
Expand Down
65 changes: 65 additions & 0 deletions tests/testsuite/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,68 @@ fn simple() {
.with_stdout("https:/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

<tab>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();
}