refactor(toolchain/names)!: remove some conversion macros

src/cli/proxy_mode.rs

@@ -1,4 +1,4 @@
-use std::{path::PathBuf, process::ExitStatus};
+use std::{path::PathBuf, process::ExitStatus, str::FromStr};
 
 use anyhow::Result;
 
@@ -23,7 +23,7 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result
         .as_ref()
         .map(|arg| arg.to_string_lossy())
         .filter(|arg| arg.starts_with('+'))
-        .map(|name| ResolvableLocalToolchainName::try_from(&name.as_ref()[1..]))
+        .map(|name| ResolvableLocalToolchainName::from_str(&name[1..]))
         .transpose()?;
 
     // Build command args now while we know whether or not to skip arg 1.

src/cli/rustup_mode.rs

@@ -111,7 +111,7 @@ struct Rustup {
 fn plus_toolchain_value_parser(s: &str) -> clap::error::Result<ResolvableToolchainName> {
     use clap::{Error, error::ErrorKind};
     if let Some(stripped) = s.strip_prefix('+') {
-        ResolvableToolchainName::try_from(stripped)
+        ResolvableToolchainName::from_str(stripped)
             .map_err(|e| Error::raw(ErrorKind::InvalidValue, e))
     } else {
         Err(Error::raw(
@@ -1841,7 +1841,7 @@ async fn display_version(current_dir: PathBuf, process: &Process) -> Result<()>
     cfg.toolchain_override = cfg
         .process
         .args()
-        .find_map(|arg| arg.strip_prefix('+').map(ResolvableToolchainName::try_from))
+        .find_map(|arg| arg.strip_prefix('+').map(ResolvableToolchainName::from_str))
         .transpose()?;
 
     match cfg.maybe_ensure_active_toolchain(None).await {

src/cli/self_update.rs

@@ -382,14 +382,16 @@ impl InstallOpts<'_> {
             process,
         )?);
 
-        self.default_toolchain = Some(MaybeOfficialToolchainName::try_from(common::question_str(
-            "Default toolchain? (stable/beta/nightly/none)",
-            &match &self.default_toolchain {
-                Some(name) => name.to_string(),
-                None => "stable".to_owned(),
-            },
-            process,
-        )?)?);
+        self.default_toolchain = Some(MaybeOfficialToolchainName::from_str(
+            &common::question_str(
+                "Default toolchain? (stable/beta/nightly/none)",
+                &match &self.default_toolchain {
+                    Some(name) => name.to_string(),
+                    None => "stable".to_owned(),
+                },
+                process,
+            )?,
+        )?);
 
         self.profile = <Profile as FromStr>::from_str(&common::question_str(
             &format!(
@@ -416,7 +418,7 @@ impl InstallOpts<'_> {
             .unwrap_or_else(|| TargetTuple::from_host_or_build(process));
         let partial_channel = match &self.default_toolchain {
             None | Some(MaybeOfficialToolchainName::None) => {
-                ResolvableToolchainName::try_from("stable")?
+                ResolvableToolchainName::from_str("stable")?
             }
             Some(MaybeOfficialToolchainName::Some(s)) => s.into(),
         };

src/config.rs

@@ -187,7 +187,7 @@ impl OverrideCfg {
     fn from_file(cfg: &Cfg<'_>, file: OverrideFile) -> Result<Self> {
         let toolchain_name = match (file.toolchain.channel, file.toolchain.path) {
             (Some(name), None) => {
-                ResolvableToolchainName::try_from(name)?.resolve(&cfg.default_host_tuple()?)?
+                ResolvableToolchainName::from_str(&name)?.resolve(&cfg.default_host_tuple()?)?
             }
             (None, Some(path)) => {
                 if file.toolchain.targets.is_some()
@@ -346,8 +346,8 @@ impl<'a> Cfg<'a> {
         // Figure out default_host_tuple before Config is populated
         let default_host = settings_file.with(|s| Ok(default_host_tuple(s, process)))?;
         // Environment override
-        let env_override = match process.var_opt("RUSTUP_TOOLCHAIN")? {
-            Some(tc) => Some(ResolvableLocalToolchainName::try_from(tc)?.resolve(&default_host)?),
+        let env_override = match &process.var_opt("RUSTUP_TOOLCHAIN")? {
+            Some(tc) => Some(ResolvableLocalToolchainName::from_str(tc)?.resolve(&default_host)?),
             None => None,
         };
 
@@ -376,7 +376,7 @@ impl<'a> Cfg<'a> {
         // Run some basic checks against the constructed configuration
         // For now, that means simply checking that 'stable' can resolve
         // for the current configuration.
-        ResolvableToolchainName::try_from("stable")?.resolve(
+        ResolvableToolchainName::from_str("stable")?.resolve(
             &cfg.default_host_tuple()
                 .context("Unable parse configuration")?,
         )?;
@@ -644,7 +644,7 @@ impl<'a> Cfg<'a> {
                 // However, settings.toml could conceivably be hand edited to
                 // have an unresolved name. I'm just preserving pre-existing
                 // behaviour by choosing ResolvableToolchainName here.
-                let toolchain_name = ResolvableToolchainName::try_from(name)?
+                let toolchain_name = ResolvableToolchainName::from_str(&name)?
                     .resolve(&default_host_tuple(settings, self.process))?;
                 let override_cfg = toolchain_name.into();
                 return Ok(Some((override_cfg, source)));
@@ -696,7 +696,7 @@ impl<'a> Cfg<'a> {
                         }
                     })?;
                 if let Some(toolchain_name_str) = &override_file.toolchain.channel {
-                    let toolchain_name = ResolvableToolchainName::try_from(
+                    let toolchain_name = ResolvableToolchainName::from_str(
                         toolchain_name_str.as_str(),
                     )
                     .map_err(|_| {
@@ -714,7 +714,7 @@ impl<'a> Cfg<'a> {
                         // Permit fully qualified names IFF the toolchain is installed. TODO(robertc): consider
                         // disabling this and backing out https://github.com/rust-lang/rustup/pull/2141 (but provide
                         // the base name in the error to help users)
-                        let resolved_name = &ToolchainName::try_from(toolchain_name_str.as_str())?;
+                        let resolved_name = &ToolchainName::from_str(toolchain_name_str)?;
                         if !self.list_toolchains()?.iter().any(|s| s == resolved_name) {
                             return Err(anyhow!(format!("target tuple in channel name '{name}'")));
                         }
@@ -914,7 +914,7 @@ impl<'a> Cfg<'a> {
             user_opt
         }?;
         toolchain_maybe_str
-            .map(ResolvableToolchainName::try_from)
+            .map(|s| ResolvableToolchainName::from_str(&s))
             .transpose()?
             .map(|t| t.resolve(&self.default_host_tuple()?))
             .transpose()
@@ -935,7 +935,7 @@ impl<'a> Cfg<'a> {
                 .filter_map(io::Result::ok)
                 .filter(|e| e.file_type().map(|f| !f.is_file()).unwrap_or(false))
                 .filter_map(|e| e.file_name().into_string().ok())
-                .filter_map(|n| ToolchainName::try_from(n).ok())
+                .filter_map(|n| ToolchainName::from_str(&n).ok())
                 .collect();
 
             toolchains.sort();

src/toolchain/names.rs

@@ -71,47 +71,6 @@ pub enum InvalidName {
     PlusPrefix(String),
 }
 
-macro_rules! from_variant {
-    ($from:ident, $to:ident, $variant:expr) => {
-        impl From<$from> for $to {
-            fn from(value: $from) -> Self {
-                $variant(value)
-            }
-        }
-    };
-}
-
-macro_rules! try_from_str {
-    ($to:ident) => {
-        try_from_str!(&str, $to);
-
-        impl TryFrom<String> for $to {
-            type Error = InvalidName;
-
-            fn try_from(value: String) -> std::result::Result<Self, Self::Error> {
-                $to::validate(&value)
-            }
-        }
-
-        impl FromStr for $to {
-            type Err = InvalidName;
-
-            fn from_str(value: &str) -> std::result::Result<Self, Self::Err> {
-                $to::validate(value)
-            }
-        }
-    };
-    ($from:ty, $to:ident) => {
-        impl TryFrom<$from> for $to {
-            type Error = InvalidName;
-
-            fn try_from(value: $from) -> std::result::Result<Self, Self::Error> {
-                $to::validate(value)
-            }
-        }
-    };
-}
-
 /// Common validate rules for all sorts of toolchain names
 fn validate(candidate: &str) -> Result<&str, InvalidName> {
     if let Some(without_plus) = candidate.strip_prefix('+') {
@@ -149,14 +108,20 @@ impl ResolvableToolchainName {
             return Ok(Self::Official(desc));
         }
 
-        match CustomToolchainName::try_from(candidate) {
+        match CustomToolchainName::from_str(candidate) {
             Ok(custom) => Ok(Self::Custom(custom)),
             Err(_) => Err(InvalidName::ToolchainName(candidate.into())),
         }
     }
 }
 
-try_from_str!(ResolvableToolchainName);
+impl FromStr for ResolvableToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl From<&PartialToolchainDesc> for ResolvableToolchainName {
     fn from(value: &PartialToolchainDesc) -> Self {
@@ -193,7 +158,13 @@ impl MaybeResolvableToolchainName {
     }
 }
 
-try_from_str!(MaybeResolvableToolchainName);
+impl FromStr for MaybeResolvableToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl Display for MaybeResolvableToolchainName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -224,7 +195,13 @@ impl MaybeOfficialToolchainName {
     }
 }
 
-try_from_str!(MaybeOfficialToolchainName);
+impl FromStr for MaybeOfficialToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl Display for MaybeOfficialToolchainName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -252,17 +229,32 @@ impl ToolchainName {
             return Ok(Self::Official(desc));
         }
 
-        match CustomToolchainName::try_from(candidate) {
+        match CustomToolchainName::from_str(candidate) {
             Ok(custom) => Ok(Self::Custom(custom)),
             Err(_) => Err(InvalidName::ToolchainName(candidate.into())),
         }
     }
 }
 
-from_variant!(ToolchainDesc, ToolchainName, ToolchainName::Official);
-from_variant!(CustomToolchainName, ToolchainName, ToolchainName::Custom);
+impl From<ToolchainDesc> for ToolchainName {
+    fn from(value: ToolchainDesc) -> Self {
+        Self::Official(value)
+    }
+}
 
-try_from_str!(ToolchainName);
+impl From<CustomToolchainName> for ToolchainName {
+    fn from(value: CustomToolchainName) -> Self {
+        Self::Custom(value)
+    }
+}
+
+impl FromStr for ToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl Display for ToolchainName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -294,7 +286,7 @@ impl ResolvableLocalToolchainName {
     /// Validates if the string is a resolvable toolchain, or a path based toolchain.
     fn validate(candidate: &str) -> Result<Self, InvalidName> {
         let candidate = validate(candidate)?;
-        if let Ok(name) = ResolvableToolchainName::try_from(candidate) {
+        if let Ok(name) = ResolvableToolchainName::from_str(candidate) {
             return Ok(Self::Named(name));
         }
 
@@ -304,7 +296,13 @@ impl ResolvableLocalToolchainName {
     }
 }
 
-try_from_str!(ResolvableLocalToolchainName);
+impl FromStr for ResolvableLocalToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl Display for ResolvableLocalToolchainName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -326,6 +324,18 @@ pub(crate) enum LocalToolchainName {
     Path(PathBasedToolchainName),
 }
 
+impl From<ToolchainName> for LocalToolchainName {
+    fn from(value: ToolchainName) -> Self {
+        Self::Named(value)
+    }
+}
+
+impl From<PathBasedToolchainName> for LocalToolchainName {
+    fn from(value: PathBasedToolchainName) -> Self {
+        Self::Path(value)
+    }
+}
+
 impl From<ToolchainDesc> for LocalToolchainName {
     fn from(value: ToolchainDesc) -> Self {
         ToolchainName::Official(value).into()
@@ -338,13 +348,6 @@ impl From<CustomToolchainName> for LocalToolchainName {
     }
 }
 
-from_variant!(ToolchainName, LocalToolchainName, LocalToolchainName::Named);
-from_variant!(
-    PathBasedToolchainName,
-    LocalToolchainName,
-    LocalToolchainName::Path
-);
-
 impl PartialEq<ToolchainName> for LocalToolchainName {
     fn eq(&self, other: &ToolchainName) -> bool {
         match self {
@@ -391,7 +394,13 @@ impl Deref for CustomToolchainName {
     }
 }
 
-try_from_str!(CustomToolchainName);
+impl FromStr for CustomToolchainName {
+    type Err = InvalidName;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::validate(value)
+    }
+}
 
 impl Display for CustomToolchainName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -522,12 +531,12 @@ mod tests {
 
         #[test]
         fn test_parse_custom(name in arb_custom_name()) {
-            CustomToolchainName::try_from(name).unwrap();
+            CustomToolchainName::from_str(&name).unwrap();
         }
 
         #[test]
         fn test_parse_resolvable_name(name in arb_resolvable_name()) {
-            ResolvableToolchainName::try_from(name).unwrap();
+            ResolvableToolchainName::from_str(&name).unwrap();
         }
 
         // TODO: This needs some thought
@@ -564,7 +573,7 @@ mod tests {
             "this.is.not-a+semver",
         ]
         .into_iter()
-        .map(|s| ToolchainName::try_from(s).unwrap())
+        .map(|s| ToolchainName::from_str(s).unwrap())
         .collect::<Vec<_>>();
 
         let mut v = vec![
@@ -589,7 +598,7 @@ mod tests {
             "the cake is a lie",
         ]
         .into_iter()
-        .map(|s| ToolchainName::try_from(s).unwrap())
+        .map(|s| ToolchainName::from_str(s).unwrap())
         .collect::<Vec<_>>();
 
         v.sort();