Make list_components safer to work with

PR 2116 introduced a regression in linked toolchain support. The
regression was introduced in part because there was no type information
signifying the difference between uninstalled toolchains and toolchains
that don't have components, thus the compiler didn't hint during
development that any discrimination was needed.

This patch introduces a type level discrimination to avoid future issues
of this type. The code is a little crufty due to error_chain glue
issues, but there is a longer term path to making that clean.

Author: rbtcollins

src/cli/common.rs

@@ -318,7 +318,17 @@ where
 
 pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    for component in toolchain.list_components()? {
+    let components = match toolchain.list_components()? {
+        // XXX: long term move this error to cli ? the normal .into doesn't work
+        // because Result here is the wrong sort and expression type ascription
+        // isn't a feature yet.
+        None => Err(rustup::Error(
+            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
+            error_chain::State::default(),
+        )),
+        Some(components) => Ok(components),
+    }?;
+    for component in components {
         if component.component.short_name_in_manifest() == "rust-std" {
             let target = component
                 .component
@@ -340,7 +350,17 @@ pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    for component in toolchain.list_components()? {
+    let components = match toolchain.list_components()? {
+        // XXX: long term move this error to cli ? the normal .into doesn't work
+        // because Result here is the wrong sort and expression type ascription
+        // isn't a feature yet.
+        None => Err(rustup::Error(
+            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
+            error_chain::State::default(),
+        )),
+        Some(components) => Ok(components),
+    }?;
+    for component in components {
         if component.component.short_name_in_manifest() == "rust-std" {
             let target = component
                 .component
@@ -357,7 +377,17 @@ pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    for component in toolchain.list_components()? {
+    let components = match toolchain.list_components()? {
+        // XXX: long term move this error to cli ? the normal .into doesn't work
+        // because Result here is the wrong sort and expression type ascription
+        // isn't a feature yet.
+        None => Err(rustup::Error(
+            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
+            error_chain::State::default(),
+        )),
+        Some(components) => Ok(components),
+    }?;
+    for component in components {
         let name = component.name;
         if component.installed {
             t.attr(term2::Attr::Bold)?;
@@ -373,7 +403,17 @@ pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
 
 pub fn list_installed_components(toolchain: &Toolchain<'_>) -> Result<()> {
     let mut t = term2::stdout();
-    for component in toolchain.list_components()? {
+    let components = match toolchain.list_components()? {
+        // XXX: long term move this error to cli ? the normal .into doesn't work
+        // because Result here is the wrong sort and expression type ascription
+        // isn't a feature yet.
+        None => Err(rustup::Error(
+            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
+            error_chain::State::default(),
+        )),
+        Some(components) => Ok(components),
+    }?;
+    for component in components {
         if component.installed {
             writeln!(t, "{}", component.name)?;
         }

src/cli/rustup_mode.rs

@@ -10,7 +10,7 @@ use rustup::dist::dist::{PartialTargetTriple, PartialToolchainDesc, Profile, Tar
 use rustup::dist::manifest::Component;
 use rustup::utils::utils::{self, ExitCode};
 use rustup::Notification;
-use rustup::{command, Cfg, Toolchain};
+use rustup::{command, Cfg, ComponentStatus, Toolchain};
 use std::error::Error;
 use std::fmt;
 use std::io::Write;
@@ -934,7 +934,8 @@ fn show(cfg: &Cfg) -> Result<()> {
     let active_targets = if let Ok(ref at) = active_toolchain {
         if let Some((ref t, _)) = *at {
             match t.list_components() {
-                Ok(cs_vec) => cs_vec
+                Ok(None) => vec![],
+                Ok(Some(cs_vec)) => cs_vec
                     .into_iter()
                     .filter(|c| c.component.short_name_in_manifest() == "rust-std")
                     .filter(|c| c.installed)
@@ -1079,6 +1080,18 @@ fn target_list(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
 
 fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     let toolchain = explicit_or_dir_toolchain(cfg, m)?;
+    // XXX: long term move this error to cli ? the normal .into doesn't work
+    // because Result here is the wrong sort and expression type ascription
+    // isn't a feature yet.
+    // list_components *and* add_component would both be inappropriate for
+    // custom toolchains.
+    if toolchain.is_custom() {
+        return Err(rustup::Error(
+            rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
+            error_chain::State::default(),
+        )
+        .into());
+    }
 
     let mut targets: Vec<String> = m
         .values_of("target")
@@ -1092,7 +1105,7 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
         }
 
         targets.clear();
-        for component in toolchain.list_components()? {
+        for component in toolchain.list_components()?.unwrap() {
             if component.component.short_name_in_manifest() == "rust-std"
                 && component.available
                 && !component.installed
@@ -1312,9 +1325,18 @@ const DOCS_DATA: &[(&str, &str, &str,)] = &[
 
 fn doc(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
     let toolchain = explicit_or_dir_toolchain(cfg, m)?;
-    if !toolchain.is_custom() {
-        for cstatus in &toolchain.list_components()? {
-            if cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed {
+    match toolchain.list_components()? {
+        None => { /* custom - no validation */ }
+        Some(components) => {
+            if let [_] = components
+                .into_iter()
+                .filter(|cstatus| {
+                    cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
+                })
+                .take(1)
+                .collect::<Vec<ComponentStatus>>()
+                .as_slice()
+            {
                 info!(
                     "`rust-docs` not installed in toolchain `{}`",
                     toolchain.name()

src/toolchain.rs

@@ -526,14 +526,16 @@ impl<'a> Toolchain<'a> {
         }
     }
 
-    pub fn list_components(&self) -> Result<Vec<ComponentStatus>> {
+    pub fn list_components(&self) -> Result<Option<Vec<ComponentStatus>>> {
         if !self.exists() {
             return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
         }
 
         let toolchain = &self.name;
-        let toolchain = ToolchainDesc::from_str(toolchain)
-            .chain_err(|| ErrorKind::ComponentsUnsupported(self.name.to_string()))?;
+        let toolchain = match ToolchainDesc::from_str(toolchain).ok() {
+            None => return Ok(None),
+            Some(toolchain) => toolchain,
+        };
 
         let prefix = InstallPrefix::from(self.path.to_owned());
         let manifestation = Manifestation::open(prefix, toolchain.target.clone())?;
@@ -586,7 +588,7 @@ impl<'a> Toolchain<'a> {
 
             res.sort_by(|a, b| a.component.cmp(&b.component));
 
-            Ok(res)
+            Ok(Some(res))
         } else {
             Err(ErrorKind::ComponentsUnsupported(self.name.to_string()).into())
         }
@@ -605,7 +607,7 @@ impl<'a> Toolchain<'a> {
         const MAX_DISTANCE: usize = 3;
 
         let components = self.list_components();
-        if let Ok(components) = components {
+        if let Ok(Some(components)) = components {
             let short_name_distance = components
                 .iter()
                 .filter(|c| !only_installed || c.installed)