diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index e5338e833d6..2d7456ab8cc 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -113,12 +113,11 @@ fn find_closest(config: &Config, cmd: &str) -> Option { let cmds = list_commands(config); // Only consider candidates with a lev_distance of 3 or less so we don't // suggest out-of-the-blue options. - let mut filtered = cmds.iter() - .map(|&(ref c, _)| (lev_distance(c, cmd), c)) + cmds.into_iter() + .map(|(c, _)| (lev_distance(&c, cmd), c)) .filter(|&(d, _)| d < 4) - .collect::>(); - filtered.sort_by(|a, b| a.0.cmp(&b.0)); - filtered.get(0).map(|slot| slot.1.clone()) + .min_by_key(|a| a.0) + .map(|slot| slot.1) } fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> CliResult { diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index aebf315cb94..451efcb57e6 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -14,11 +14,11 @@ use sources::config::SourceConfigMap; /// See also `core::Source`. pub trait Registry { /// Attempt to find the packages that match a dependency request. - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()>; - fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { + fn query_vec(&mut self, dep: &Dependency, fuzzy: bool) -> CargoResult> { let mut ret = Vec::new(); - self.query(dep, &mut |s| ret.push(s))?; + self.query(dep, &mut |s| ret.push(s), fuzzy)?; Ok(ret) } } @@ -395,7 +395,7 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies } impl<'cfg> Registry for PackageRegistry<'cfg> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()> { assert!(self.patches_locked); let (override_summary, n, to_warn) = { // Look for an override and get ready to query the real source. @@ -476,7 +476,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { // already selected, then we skip this `summary`. let locked = &self.locked; let all_patches = &self.patches_available; - return source.query(dep, &mut |summary| { + let callback = &mut |summary: Summary| { for patch in patches.iter() { let patch = patch.package_id().version(); if summary.package_id().version() == patch { @@ -484,7 +484,12 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { } } f(lock(locked, all_patches, summary)) - }); + }; + return if fuzzy { + source.fuzzy_query(dep, callback) + } else { + source.query(dep, callback) + }; } // If we have an override summary then we query the source @@ -496,10 +501,17 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { } let mut n = 0; let mut to_warn = None; - source.query(dep, &mut |summary| { - n += 1; - to_warn = Some(summary); - })?; + { + let callback = &mut |summary| { + n += 1; + to_warn = Some(summary); + }; + if fuzzy { + source.fuzzy_query(dep, callback)?; + } else { + source.query(dep, callback)?; + } + } (override_summary, n, to_warn) } } diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 37ddf9691ef..5de359717fa 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -59,6 +59,7 @@ use core::PackageIdSpec; use core::{Dependency, PackageId, Registry, Summary}; use util::config::Config; use util::errors::{CargoError, CargoResult}; +use util::lev_distance::lev_distance; use util::profile; use self::context::{Activations, Context}; @@ -230,7 +231,9 @@ fn activate_deps_loop( // to amortize the cost of the current time lookup. ticks += 1; if let Some(config) = config { - if config.shell().is_err_tty() && !printed && ticks % 1000 == 0 + if config.shell().is_err_tty() + && !printed + && ticks % 1000 == 0 && start.elapsed() - deps_time > time_to_print { printed = true; @@ -857,12 +860,14 @@ fn activation_error( msg.push_str("\nversions that meet the requirements `"); msg.push_str(&dep.version_req().to_string()); msg.push_str("` are: "); - msg.push_str(&candidates - .iter() - .map(|v| v.summary.version()) - .map(|v| v.to_string()) - .collect::>() - .join(", ")); + msg.push_str( + &candidates + .iter() + .map(|v| v.summary.version()) + .map(|v| v.to_string()) + .collect::>() + .join(", "), + ); let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect(); conflicting_activations.sort_unstable(); @@ -922,17 +927,16 @@ fn activation_error( return format_err!("{}", msg); } - // Once we're all the way down here, we're definitely lost in the - // weeds! We didn't actually find any candidates, so we need to + // We didn't actually find any candidates, so we need to // give an error message that nothing was found. // - // Note that we re-query the registry with a new dependency that - // allows any version so we can give some nicer error reporting - // which indicates a few versions that were actually found. + // Maybe the user mistyped the ver_req? Like `dep="2"` when `dep="0.2"` + // was meant. So we re-query the registry with `deb="*"` so we can + // list a few versions that were actually found. let all_req = semver::VersionReq::parse("*").unwrap(); let mut new_dep = dep.clone(); new_dep.set_version_req(all_req); - let mut candidates = match registry.query_vec(&new_dep) { + let mut candidates = match registry.query_vec(&new_dep, false) { Ok(candidates) => candidates, Err(e) => return e, }; @@ -977,12 +981,41 @@ fn activation_error( msg } else { + // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing` + // was meant. So we try asking the registry for a `fuzzy` search for suggestions. + let mut candidates = Vec::new(); + if let Err(e) = registry.query(&new_dep, &mut |s| candidates.push(s.name()), true) { + return e; + }; + candidates.sort_unstable(); + candidates.dedup(); + let mut candidates: Vec<_> = candidates + .iter() + .map(|n| (lev_distance(&*new_dep.name(), &*n), n)) + .filter(|&(d, _)| d < 4) + .collect(); + candidates.sort_by_key(|o| o.0); let mut msg = format!( "no matching package named `{}` found\n\ location searched: {}\n", dep.name(), dep.source_id() ); + if !candidates.is_empty() { + let mut names = candidates + .iter() + .take(3) + .map(|c| c.1.as_str()) + .collect::>(); + + if candidates.len() > 3 { + names.push("..."); + } + + msg.push_str("did you mean: "); + msg.push_str(&names.join(", ")); + msg.push_str("\n"); + } msg.push_str("required by "); msg.push_str(&describe_path(&graph.path_to_top(parent.package_id()))); diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 502d4a0c313..8017cd063a6 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -52,7 +52,7 @@ impl<'a> RegistryQueryer<'a> { summary: s, replace: None, }); - })?; + }, false)?; for candidate in ret.iter_mut() { let summary = &candidate.summary; @@ -66,7 +66,7 @@ impl<'a> RegistryQueryer<'a> { }; debug!("found an override for {} {}", dep.name(), dep.version_req()); - let mut summaries = self.registry.query_vec(dep)?.into_iter(); + let mut summaries = self.registry.query_vec(dep, false)?.into_iter(); let s = summaries.next().ok_or_else(|| { format_err!( "no matching package for override `{}` found\n\ diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index c36480aab27..4f0b0a4f962 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -25,6 +25,12 @@ pub trait Source { /// Attempt to find the packages that match a dependency request. fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + /// Attempt to find the packages that are close to a dependency request. + /// Each source gets to define what `close` means for it. + /// path/git sources may return all dependencies that are at that uri. + /// where as an Index source may return dependencies that have the same canonicalization. + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()>; + fn query_vec(&mut self, dep: &Dependency) -> CargoResult> { let mut ret = Vec::new(); self.query(dep, &mut |s| ret.push(s))?; @@ -79,6 +85,11 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).query(dep, f) } + /// Forwards to `Source::query` + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + (**self).fuzzy_query(dep, f) + } + /// Forwards to `Source::source_id` fn source_id(&self) -> &SourceId { (**self).source_id() diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index bf20a270d2e..91c3e50bc7a 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -54,6 +54,14 @@ impl<'cfg> Source for DirectorySource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let packages = self.packages.values().map(|p| &p.0); + for summary in packages.map(|pkg| pkg.summary().clone()) { + f(summary); + } + Ok(()) + } + fn supports_checksums(&self) -> bool { true } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index fdf3e6082e6..6d5d2cfb5f8 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -130,6 +130,13 @@ impl<'cfg> Source for GitSource<'cfg> { src.query(dep, f) } + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let src = self.path_source + .as_mut() + .expect("BUG: update() must be called before query()"); + src.fuzzy_query(dep, f) + } + fn supports_checksums(&self) -> bool { false } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index f058b25700f..a5d2e9591d2 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -508,6 +508,13 @@ impl<'cfg> Source for PathSource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, _dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + for s in self.packages.iter().map(|p| p.summary()) { + f(s.clone()) + } + Ok(()) + } + fn supports_checksums(&self) -> bool { false } diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 3f349af0b2a..7541dd70c50 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -11,6 +11,85 @@ use sources::registry::RegistryData; use sources::registry::{RegistryPackage, INDEX_LOCK}; use util::{internal, CargoResult, Config, Filesystem}; +/// Crates.io treats hyphen and underscores as interchangeable +/// but, the index and old cargo do not. So the index must store uncanonicalized version +/// of the name so old cargos can find it. +/// This loop tries all possible combinations of switching +/// hyphen and underscores to find the uncanonicalized one. +/// As all stored inputs have the correct spelling, we start with the spelling as provided. +struct UncanonicalizedIter<'s> { + input: &'s str, + num_hyphen_underscore: u32, + hyphen_combination_num: u16, +} + +impl<'s> UncanonicalizedIter<'s> { + fn new(input: &'s str) -> Self { + let num_hyphen_underscore = input.chars().filter(|&c| c == '_' || c == '-').count() as u32; + UncanonicalizedIter { + input, + num_hyphen_underscore, + hyphen_combination_num: 0, + } + } +} + +impl<'s> Iterator for UncanonicalizedIter<'s> { + type Item = String; + + fn next(&mut self) -> Option { + if self.hyphen_combination_num > 0 && self.hyphen_combination_num.trailing_zeros() >= self.num_hyphen_underscore { + return None; + } + + let ret = Some(self.input + .chars() + .scan(0u16, |s, c| { + // the check against 15 here's to prevent + // shift overflow on inputs with more then 15 hyphens + if (c == '_' || c == '-') && *s <= 15 { + let switch = (self.hyphen_combination_num & (1u16 << *s)) > 0; + let out = if (c == '_') ^ switch { + '_' + } else { + '-' + }; + *s += 1; + Some(out) + } else { + Some(c) + } + }) + .collect()); + self.hyphen_combination_num += 1; + ret + } +} + +#[test] +fn no_hyphen() { + assert_eq!( + UncanonicalizedIter::new("test").collect::>(), + vec!["test".to_string()] + ) +} + +#[test] +fn two_hyphen() { + assert_eq!( + UncanonicalizedIter::new("te-_st").collect::>(), + vec!["te-_st".to_string(), "te__st".to_string(), "te--st".to_string(), "te_-st".to_string()] + ) +} + +#[test] +fn overflow_hyphen() { + assert_eq!( + UncanonicalizedIter::new("te-_-_-_-_-_-_-_-_-st").take(100).count(), + 100 + ) +} + pub struct RegistryIndex<'cfg> { source_id: SourceId, path: Filesystem, @@ -99,51 +178,56 @@ impl<'cfg> RegistryIndex<'cfg> { .collect::(); // see module comment for why this is structured the way it is - let path = match fs_name.len() { + let raw_path = match fs_name.len() { 1 => format!("1/{}", fs_name), 2 => format!("2/{}", fs_name), 3 => format!("3/{}/{}", &fs_name[..1], fs_name), _ => format!("{}/{}/{}", &fs_name[0..2], &fs_name[2..4], fs_name), }; let mut ret = Vec::new(); - let mut hit_closure = false; - let err = load.load(&root, Path::new(&path), &mut |contents| { - hit_closure = true; - let contents = str::from_utf8(contents) - .map_err(|_| format_err!("registry index file was not valid utf-8"))?; - ret.reserve(contents.lines().count()); - let lines = contents.lines().map(|s| s.trim()).filter(|l| !l.is_empty()); - - let online = !self.config.cli_unstable().offline; - // Attempt forwards-compatibility on the index by ignoring - // everything that we ourselves don't understand, that should - // allow future cargo implementations to break the - // interpretation of each line here and older cargo will simply - // ignore the new lines. - ret.extend(lines.filter_map(|line| { - let (summary, locked) = match self.parse_registry_package(line) { - Ok(p) => p, - Err(e) => { - info!("failed to parse `{}` registry package: {}", name, e); - trace!("line: {}", line); - return None; + for path in UncanonicalizedIter::new(&raw_path).take(1024) { + let mut hit_closure = false; + let err = load.load(&root, Path::new(&path), &mut |contents| { + hit_closure = true; + let contents = str::from_utf8(contents) + .map_err(|_| format_err!("registry index file was not valid utf-8"))?; + ret.reserve(contents.lines().count()); + let lines = contents.lines().map(|s| s.trim()).filter(|l| !l.is_empty()); + + let online = !self.config.cli_unstable().offline; + // Attempt forwards-compatibility on the index by ignoring + // everything that we ourselves don't understand, that should + // allow future cargo implementations to break the + // interpretation of each line here and older cargo will simply + // ignore the new lines. + ret.extend(lines.filter_map(|line| { + let (summary, locked) = match self.parse_registry_package(line) { + Ok(p) => p, + Err(e) => { + info!("failed to parse `{}` registry package: {}", name, e); + trace!("line: {}", line); + return None; + } + }; + if online || load.is_crate_downloaded(summary.package_id()) { + Some((summary, locked)) + } else { + None } - }; - if online || load.is_crate_downloaded(summary.package_id()) { - Some((summary, locked)) - } else { - None - } - })); + })); - Ok(()) - }); + Ok(()) + }); - // We ignore lookup failures as those are just crates which don't exist - // or we haven't updated the registry yet. If we actually ran the - // closure though then we care about those errors. - if hit_closure { - err?; + // We ignore lookup failures as those are just crates which don't exist + // or we haven't updated the registry yet. If we actually ran the + // closure though then we care about those errors. + if hit_closure { + err?; + // Crates.io ensures that there is only one hyphen and underscore equivalent + // result in the index so return when we find it. + return Ok(ret); + } } Ok(ret) @@ -178,7 +262,7 @@ impl<'cfg> RegistryIndex<'cfg> { Ok((summary, yanked.unwrap_or(false))) } - pub fn query( + pub fn query_inner( &mut self, dep: &Dependency, load: &mut RegistryData, @@ -212,9 +296,7 @@ impl<'cfg> RegistryIndex<'cfg> { }); for summary in summaries { - if dep.matches(&summary) { - f(summary); - } + f(summary); } Ok(()) } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 0d72ae74659..7dc1ea2813f 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -463,9 +463,11 @@ impl<'cfg> Source for RegistrySource<'cfg> { if dep.source_id().precise().is_some() && !self.updated { debug!("attempting query without update"); let mut called = false; - self.index.query(dep, &mut *self.ops, &mut |s| { - called = true; - f(s); + self.index.query_inner(dep, &mut *self.ops, &mut |s| { + if dep.matches(&s) { + called = true; + f(s); + } })?; if called { return Ok(()); @@ -475,7 +477,15 @@ impl<'cfg> Source for RegistrySource<'cfg> { } } - self.index.query(dep, &mut *self.ops, f) + self.index.query_inner(dep, &mut *self.ops, &mut |s| { + if dep.matches(&s) { + f(s); + } + }) + } + + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + self.index.query_inner(dep, &mut *self.ops, f) } fn supports_checksums(&self) -> bool { diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 16d867c17bd..997731aff52 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -35,6 +35,19 @@ impl<'cfg> Source for ReplacedSource<'cfg> { Ok(()) } + fn fuzzy_query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + let (replace_with, to_replace) = (&self.replace_with, &self.to_replace); + let dep = dep.clone().map_source(to_replace, replace_with); + + self.inner + .fuzzy_query( + &dep, + &mut |summary| f(summary.map_source(replace_with, to_replace)), + ) + .chain_err(|| format!("failed to query replaced source {}", self.to_replace))?; + Ok(()) + } + fn supports_checksums(&self) -> bool { self.inner.supports_checksums() } diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index db4d3057f96..4434abb8108 100644 --- a/tests/testsuite/directory.rs +++ b/tests/testsuite/directory.rs @@ -230,6 +230,7 @@ error: failed to compile `bar v0.1.0`, intermediate artifacts can be found at `[ Caused by: no matching package named `baz` found location searched: registry `https://github.com/rust-lang/crates.io-index` +did you mean: bar, foo required by package `bar v0.1.0` ", ), diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index 98fb7c5e9dd..e7ea0f024b0 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -1239,6 +1239,7 @@ fn invalid_path_dep_in_workspace_with_lockfile() { "\ error: no matching package named `bar` found location searched: [..] +did you mean: foo required by package `foo v0.5.0 ([..])` ", ), diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 4d632a7b925..89a60aa0f72 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -141,6 +141,76 @@ required by package `foo v0.0.1 ([..])` ); } +#[test] +fn wrong_case() { + Package::new("init", "0.0.1").publish(); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + Init = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + // #5678 to make this work + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[UPDATING] registry [..] +error: no matching package named `Init` found +location searched: registry [..] +did you mean: init +required by package `foo v0.0.1 ([..])` +", + ), + ); +} + +#[test] +fn mis_hyphenated() { + Package::new("mis-hyphenated", "0.0.1").publish(); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + mis_hyphenated = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + // #2775 to make this work + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[UPDATING] registry [..] +error: no matching package named `mis_hyphenated` found +location searched: registry [..] +did you mean: mis-hyphenated +required by package `foo v0.0.1 ([..])` +", + ), + ); +} + #[test] fn wrong_version() { let p = project("foo") diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 24d5ff9efb0..3634cef7cb4 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -28,9 +28,9 @@ fn resolve_with_config( ) -> CargoResult> { struct MyRegistry<'a>(&'a [Summary]); impl<'a> Registry for MyRegistry<'a> { - fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> { + fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary), fuzzy: bool) -> CargoResult<()> { for summary in self.0.iter() { - if dep.matches(summary) { + if fuzzy || dep.matches(summary) { f(summary.clone()); } } @@ -440,6 +440,46 @@ fn resolving_incompat_versions() { ); } +#[test] +fn resolving_wrong_case_from_registry() { + // In the future we may #5678 allow this to happen. + // For back compatibility reasons, we probably won't. + // But we may want to future prove ourselves by understanding it. + // This test documents the current behavior. + let reg = registry(vec![ + pkg!(("foo", "1.0.0")), + pkg!("bar" => ["Foo"]), + ]); + + assert!( + resolve( + &pkg_id("root"), + vec![dep("bar")], + ® + ).is_err() + ); +} + +#[test] +fn resolving_mis_hyphenated_from_registry() { + // In the future we may #2775 allow this to happen. + // For back compatibility reasons, we probably won't. + // But we may want to future prove ourselves by understanding it. + // This test documents the current behavior. + let reg = registry(vec![ + pkg!(("fo-o", "1.0.0")), + pkg!("bar" => ["fo_o"]), + ]); + + assert!( + resolve( + &pkg_id("root"), + vec![dep("bar")], + ® + ).is_err() + ); +} + #[test] fn resolving_backtrack() { let reg = registry(vec![