Skip to content

Commit

Permalink
Auto merge of #9095 - ehuss:spec-suggestion, r=alexcrichton
Browse files Browse the repository at this point in the history
Add suggestion for bad package id.

This adds some suggestions to the error message if a pkgid spec does not match any packages.
  • Loading branch information
bors committed Jan 22, 2021
2 parents 24cf9c5 + b04c7fb commit b52fc0a
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 13 deletions.
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://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
<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();
}

0 comments on commit b52fc0a

Please sign in to comment.