diff --git a/src/errors.rs b/src/errors.rs index 5dbf05e091..a8f17b4761 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -3,25 +3,151 @@ use std::{ borrow::Cow, ffi::OsString, - fmt::{Debug, Write as FmtWrite}, + fmt::{self, Debug, Write as FmtWrite}, io::{self, Write}, path::PathBuf, }; use platforms::Platform; +use strsim::damerau_levenshtein; use thiserror::Error as ThisError; use url::Url; use crate::{ + config::Cfg, dist::{ Channel, TargetTuple, ToolchainDesc, + config::Config as DistConfig, manifest::{Component, Manifest}, }, - toolchain::{PathBasedToolchainName, ToolchainName}, + toolchain::{PathBasedToolchainName, Toolchain, ToolchainName}, }; pub(crate) const DEFAULT_STABLE_HINT: &str = "help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain."; +#[derive(Debug, Clone)] +pub enum TargetSuggestion { + Toolchain { name: String, target: TargetTuple }, + Component(String), +} + +impl TargetSuggestion { + pub(crate) fn from_target( + desc: &ToolchainDesc, + target: &TargetTuple, + component: &Component, + config: &DistConfig, + manifest: &Manifest, + cfg: &Cfg<'_>, + ) -> Option { + let Ok(toolchains) = cfg.list_toolchains(true) else { + return component_suggestion(desc, component, config, manifest, true) + .map(Self::Component); + }; + + for toolchain_name in toolchains { + if let ToolchainName::Official(toolchain_desc) = &toolchain_name + && toolchain_desc == desc + { + continue; + } + + let Ok(toolchain) = Toolchain::new(cfg, toolchain_name.clone().into()) else { + continue; + }; + let Ok(installed_targets) = toolchain.installed_targets() else { + continue; + }; + + if installed_targets.contains(target) { + return Some(Self::Toolchain { + name: toolchain_name.to_string(), + target: target.clone(), + }); + } + } + + component_suggestion(desc, component, config, manifest, true).map(Self::Component) + } +} + +impl fmt::Display for TargetSuggestion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Toolchain { name, target } => { + write!(f, "\nhelp: try `rustup +{name} target remove {target}`") + } + Self::Component(name) => write!(f, "\nhelp: did you mean '{name}'?"), + } + } +} + +pub(crate) fn component_suggestion( + desc: &ToolchainDesc, + component: &Component, + config: &DistConfig, + manifest: &Manifest, + only_installed: bool, +) -> Option { + // Suggest only for very small differences + // High number can result in inaccurate suggestions for short queries e.g. `rls` + const MAX_DISTANCE: usize = 3; + let Ok(components) = manifest.query_components(desc, config) else { + return None; + }; + let components = components + .iter() + .filter(|c| !only_installed || c.installed) + .collect::>(); + let short_name_distance = components + .iter() + .map(|c| { + ( + damerau_levenshtein(&manifest.name(&c.component), &manifest.name(component)), + *c, + ) + }) + .min_by_key(|t| t.0) + .expect("There should be always at least one component"); + let long_name_distance = components + .iter() + .map(|c| { + ( + damerau_levenshtein(&c.component.name(), &manifest.name(component)), + *c, + ) + }) + .min_by_key(|t| t.0) + .expect("There should be always at least one component"); + + // Find closer suggestion + let (closest_distance, closest_match) = if short_name_distance.0 > long_name_distance.0 { + let closest = &long_name_distance.1.component; + // Check if only targets differ + let name = if closest.short_name() == component.short_name() { + closest.target() + } else { + closest.short_name().to_string() + }; + (long_name_distance.0, name) + } else { + // Check if only targets differ + let name = if manifest.short_name(&short_name_distance.1.component) + == manifest.short_name(component) + { + short_name_distance.1.component.target() + } else { + manifest + .short_name(&short_name_distance.1.component) + .to_string() + }; + (short_name_distance.0, name) + }; + + // If suggestion is too different don't suggest anything + (closest_distance <= MAX_DISTANCE).then_some(closest_match) +} + /// A type erasing thunk for the retry crate to permit use with anyhow. See #[derive(Debug, ThisError)] #[error(transparent)] @@ -161,12 +287,12 @@ pub enum RustupError { target: TargetTuple, suggestion: Option, }, - #[error("toolchain '{}' does not have target '{}' installed{}\n", .desc, .target, - suggest_message(.suggestion))] + #[error("toolchain '{}' does not have target '{}' installed{}", .desc, .target, + .suggestion.as_ref().map_or_else(String::new, ToString::to_string))] TargetNotInstalled { desc: Box, target: TargetTuple, - suggestion: Option, + suggestion: Option, }, #[error( "rustup executable proxies don't seem to work\n\ @@ -205,7 +331,7 @@ fn maybe_suggest_toolchain(bad_name: &str) -> Cow<'static, str> { let suggestion = ["stable", "beta", "nightly"] .into_iter() .filter_map(|s| { - let distance = strsim::damerau_levenshtein(bad_name, s); + let distance = damerau_levenshtein(bad_name, s); (distance <= MAX_DISTANCE).then_some((distance, s)) }) .max(); diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 1fdec3cc5d..56822dbf9e 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -16,13 +16,12 @@ use crate::{ config::{ActiveSource, Cfg, EnsureInstalled}, dist::{ DistOptions, PartialToolchainDesc, ToolchainDesc, - config::Config, download::DownloadCfg, manifest::{Component, ComponentStatus, Manifest, ManifestWithHash}, manifestation::{Changes, Manifestation}, prefix::InstallPrefix, }, - errors::UnknownComponentInfo, + errors::{TargetSuggestion, UnknownComponentInfo, component_suggestion}, install::InstallMethod, }; @@ -100,7 +99,8 @@ impl<'a> DistributableToolchain<'a> { } let config = manifestation.read_config()?.unwrap_or_default(); - let suggestion = self.get_component_suggestion(&component, &config, &manifest, false); + let suggestion = + component_suggestion(&self.desc, &component, &config, &manifest, false); let desc = self.desc.clone(); if targ_pkg @@ -246,83 +246,6 @@ impl<'a> DistributableToolchain<'a> { Ok(cmd) } - fn get_component_suggestion( - &self, - component: &Component, - config: &Config, - manifest: &Manifest, - only_installed: bool, - ) -> Option { - use strsim::damerau_levenshtein; - - // Suggest only for very small differences - // High number can result in inaccurate suggestions for short queries e.g. `rls` - const MAX_DISTANCE: usize = 3; - - let components = manifest.query_components(&self.desc, config); - if let Ok(components) = components { - let short_name_distance = components - .iter() - .filter(|c| !only_installed || c.installed) - .map(|c| { - ( - damerau_levenshtein( - &manifest.name(&c.component)[..], - &manifest.name(component)[..], - ), - c, - ) - }) - .min_by_key(|t| t.0) - .expect("There should be always at least one component"); - - let long_name_distance = components - .iter() - .filter(|c| !only_installed || c.installed) - .map(|c| { - ( - damerau_levenshtein(&c.component.name()[..], &manifest.name(component)[..]), - c, - ) - }) - .min_by_key(|t| t.0) - .expect("There should be always at least one component"); - - let mut closest_distance = short_name_distance; - let mut closest_match = manifest - .short_name(&short_name_distance.1.component) - .to_owned(); - - // Find closer suggestion - if short_name_distance.0 > long_name_distance.0 { - closest_distance = long_name_distance; - - // Check if only targets differ - if closest_distance.1.component.short_name() == component.short_name() { - closest_match = long_name_distance.1.component.target(); - } else { - closest_match = long_name_distance.1.component.short_name().to_string(); - } - } else { - // Check if only targets differ - if manifest.short_name(&closest_distance.1.component) - == manifest.short_name(component) - { - closest_match = short_name_distance.1.component.target(); - } - } - - // If suggestion is too different don't suggest anything - if closest_distance.0 > MAX_DISTANCE { - None - } else { - Some(closest_match) - } - } else { - None - } - } - #[tracing::instrument(level = "trace", skip_all)] pub(crate) fn get_manifestation(&self) -> anyhow::Result { let prefix = InstallPrefix::from(self.toolchain.path()); @@ -412,20 +335,34 @@ impl<'a> DistributableToolchain<'a> { continue; } - let suggestion = self.get_component_suggestion(&component, &config, &manifest, true); // Check if the target is installed. if !config .components .iter() .any(|c| c.target() == component.target()) { + let target = component + .target + .as_ref() + .expect("component target should be known"); + let suggestion = TargetSuggestion::from_target( + &self.desc, + target, + &component, + &config, + &manifest, + self.toolchain.cfg, + ); return Err(RustupError::TargetNotInstalled { desc: Box::new(self.desc.clone()), - target: component.target.expect("component target should be known"), + target: target.clone(), suggestion, } .into()); } + + let suggestion = component_suggestion(&self.desc, &component, &config, &manifest, true); + unknown_components.push(UnknownComponentInfo { name: manifest.short_name(&component).to_string(), description: manifest.description(&component), diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 6d8ae541a7..4d24c98dea 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1795,6 +1795,73 @@ error: toolchain 'nightly-[HOST_TUPLE]' does not have target '[CROSS_ARCH_I]' in .is_err(); } +#[tokio::test] +async fn remove_target_not_installed_with_suggestion() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + cx.config + .expect(["rustup", "toolchain", "install", "stable"]) + .await + .is_ok(); + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_ok(); + cx.config + .expect(["rustup", "default", "stable"]) + .await + .is_ok(); + cx.config + .expect([ + "rustup", + "target", + "add", + CROSS_ARCH1, + "--toolchain=nightly", + ]) + .await + .is_ok(); + cx.config + .expect(["rustup", "target", "remove", CROSS_ARCH1]) + .await + .extend_redactions([ + ("[HOST_TUPLE]", this_host_tuple()), + ("[CROSS_ARCH_I]", CROSS_ARCH1.to_string()), + ]) + .with_stderr(snapbox::str![[r#" +... +error: toolchain 'stable-[HOST_TUPLE]' does not have target '[CROSS_ARCH_I]' installed +help: try `rustup +nightly-[HOST_TUPLE] target remove [CROSS_ARCH_I]` +... +"#]]) + .is_err(); +} + +#[tokio::test] +async fn remove_target_not_installed_no_alternative() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + cx.config + .expect(["rustup", "toolchain", "install", "stable"]) + .await + .is_ok(); + cx.config + .expect(["rustup", "default", "stable"]) + .await + .is_ok(); + cx.config + .expect(["rustup", "target", "remove", CROSS_ARCH1]) + .await + .extend_redactions([ + ("[HOST_TUPLE]", this_host_tuple()), + ("[CROSS_ARCH_I]", CROSS_ARCH1.to_string()), + ]) + .with_stderr(snapbox::str![[r#" +... +error: toolchain 'stable-[HOST_TUPLE]' does not have target '[CROSS_ARCH_I]' installed +... +"#]]) + .is_err(); +} + #[tokio::test] async fn remove_target_no_toolchain() { let cx = CliTestContext::new(Scenario::SimpleV2).await; @@ -2337,8 +2404,8 @@ async fn remove_target_suggest_best_match() { .expect(["rustup", "target", "remove", &format!("{CROSS_ARCH1}a")[..]]) .await .with_stderr(snapbox::str![[r#" -error: toolchain 'nightly-[HOST_TUPLE]' does not have target '[CROSS_ARCH_I]a' installed; did you mean '[CROSS_ARCH_I]'? - +error: toolchain 'nightly-[HOST_TUPLE]' does not have target '[CROSS_ARCH_I]a' installed +help: did you mean '[CROSS_ARCH_I]'? "#]]) .is_err();