Skip to content

Commit e334803

Browse files
kinnisonkinnison
authored
Merge pull request #2259 from rbtcollins/guard-components
Use dependent types to make programming errors harder
2 parents 1db9dc9 + f39749a commit e334803

File tree

9 files changed

+619
-594
lines changed

9 files changed

+619
-594
lines changed

‎src/cli/common.rs

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::term2;
66
use git_testament::{git_testament, render_testament};
77
use lazy_static::lazy_static;
88
use rustup::dist::notifications as dist_notifications;
9+
use rustup::toolchain::DistributableToolchain;
910
use rustup::utils::notifications as util_notifications;
1011
use rustup::utils::notify::NotificationLevel;
1112
use rustup::utils::utils;
@@ -346,16 +347,9 @@ where
346347

347348
pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
348349
let mut t = term2::stdout();
349-
let components = match toolchain.list_components()? {
350-
// XXX: long term move this error to cli ? the normal .into doesn't work
351-
// because Result here is the wrong sort and expression type ascription
352-
// isn't a feature yet.
353-
None => Err(rustup::Error(
354-
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
355-
error_chain::State::default(),
356-
)),
357-
Some(components) => Ok(components),
358-
}?;
350+
let distributable = DistributableToolchain::new(&toolchain)
351+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
352+
let components = distributable.list_components()?;
359353
for component in components {
360354
if component.component.short_name_in_manifest() == "rust-std" {
361355
let target = component
@@ -378,16 +372,9 @@ pub fn list_targets(toolchain: &Toolchain<'_>) -> Result<()> {
378372

379373
pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
380374
let mut t = term2::stdout();
381-
let components = match toolchain.list_components()? {
382-
// XXX: long term move this error to cli ? the normal .into doesn't work
383-
// because Result here is the wrong sort and expression type ascription
384-
// isn't a feature yet.
385-
None => Err(rustup::Error(
386-
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
387-
error_chain::State::default(),
388-
)),
389-
Some(components) => Ok(components),
390-
}?;
375+
let distributable = DistributableToolchain::new(&toolchain)
376+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
377+
let components = distributable.list_components()?;
391378
for component in components {
392379
if component.component.short_name_in_manifest() == "rust-std" {
393380
let target = component
@@ -405,16 +392,9 @@ pub fn list_installed_targets(toolchain: &Toolchain<'_>) -> Result<()> {
405392

406393
pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
407394
let mut t = term2::stdout();
408-
let components = match toolchain.list_components()? {
409-
// XXX: long term move this error to cli ? the normal .into doesn't work
410-
// because Result here is the wrong sort and expression type ascription
411-
// isn't a feature yet.
412-
None => Err(rustup::Error(
413-
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
414-
error_chain::State::default(),
415-
)),
416-
Some(components) => Ok(components),
417-
}?;
395+
let distributable = DistributableToolchain::new(&toolchain)
396+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
397+
let components = distributable.list_components()?;
418398
for component in components {
419399
let name = component.name;
420400
if component.installed {
@@ -431,16 +411,9 @@ pub fn list_components(toolchain: &Toolchain<'_>) -> Result<()> {
431411

432412
pub fn list_installed_components(toolchain: &Toolchain<'_>) -> Result<()> {
433413
let mut t = term2::stdout();
434-
let components = match toolchain.list_components()? {
435-
// XXX: long term move this error to cli ? the normal .into doesn't work
436-
// because Result here is the wrong sort and expression type ascription
437-
// isn't a feature yet.
438-
None => Err(rustup::Error(
439-
rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()),
440-
error_chain::State::default(),
441-
)),
442-
Some(components) => Ok(components),
443-
}?;
414+
let distributable = DistributableToolchain::new(&toolchain)
415+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
416+
let components = distributable.list_components()?;
444417
for component in components {
445418
if component.installed {
446419
writeln!(t, "{}", component.name)?;

‎src/cli/errors.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ error_chain! {
2626
}
2727

2828
errors {
29+
InvalidCustomToolchainName(t: String) {
30+
display("invalid custom toolchain name: '{}'", t)
31+
}
2932
PermissionDenied {
3033
description("permission denied")
3134
}

‎src/cli/rustup_mode.rs

Lines changed: 64 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ use crate::self_update;
55
use crate::term2;
66
use crate::term2::Terminal;
77
use crate::topical_doc;
8+
89
use clap::{App, AppSettings, Arg, ArgGroup, ArgMatches, Shell, SubCommand};
910
use rustup::dist::dist::{PartialTargetTriple, PartialToolchainDesc, Profile, TargetTriple};
1011
use rustup::dist::manifest::Component;
12+
use rustup::toolchain::{CustomToolchain, DistributableToolchain};
1113
use rustup::utils::utils::{self, ExitCode};
1214
use rustup::Notification;
1315
use rustup::{command, Cfg, ComponentStatus, Toolchain};
@@ -742,7 +744,8 @@ fn default_(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
742744
let toolchain = cfg.get_toolchain(toolchain, false)?;
743745

744746
let status = if !toolchain.is_custom() {
745-
Some(toolchain.install_from_dist_if_not_installed()?)
747+
let distributable = DistributableToolchain::new(&toolchain)?;
748+
Some(distributable.install_from_dist_if_not_installed()?)
746749
} else if !toolchain.exists() {
747750
return Err(ErrorKind::ToolchainNotInstalled(toolchain.name().to_string()).into());
748751
} else {
@@ -781,8 +784,9 @@ fn check_updates(cfg: &Cfg) -> Result<()> {
781784
for channel in channels {
782785
match channel {
783786
(ref name, Ok(ref toolchain)) => {
784-
let current_version = toolchain.show_version()?;
785-
let dist_version = toolchain.show_dist_version()?;
787+
let distributable = DistributableToolchain::new(&toolchain)?;
788+
let current_version = distributable.show_version()?;
789+
let dist_version = distributable.show_dist_version()?;
786790
let _ = t.attr(term2::Attr::Bold);
787791
write!(t, "{} - ", name)?;
788792
match (current_version, dist_version) {
@@ -840,7 +844,8 @@ fn update(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result<()> {
840844
.values_of("targets")
841845
.map(|v| v.collect())
842846
.unwrap_or_else(Vec::new);
843-
Some(toolchain.install_from_dist(
847+
let distributable = DistributableToolchain::new(&toolchain)?;
848+
Some(distributable.install_from_dist(
844849
m.is_present("force"),
845850
m.is_present("allow-downgrade"),
846851
&components,
@@ -933,14 +938,18 @@ fn show(cfg: &Cfg) -> Result<()> {
933938

934939
// active_toolchain will carry the reason we don't have one in its detail.
935940
let active_targets = if let Ok(ref at) = active_toolchain {
936-
match at.0.list_components() {
937-
Ok(None) => vec![],
938-
Ok(Some(cs_vec)) => cs_vec
939-
.into_iter()
940-
.filter(|c| c.component.short_name_in_manifest() == "rust-std")
941-
.filter(|c| c.installed)
942-
.collect(),
943-
Err(_) => vec![],
941+
if let Ok(distributable) = DistributableToolchain::new(&at.0) {
942+
match distributable.list_components() {
943+
Ok(cs_vec) => cs_vec
944+
.into_iter()
945+
.filter(|c| c.component.short_name_in_manifest() == "rust-std")
946+
.filter(|c| c.installed)
947+
.collect(),
948+
Err(_) => vec![],
949+
}
950+
} else {
951+
// These three vec![] could perhaps be reduced with and_then on active_toolchain.
952+
vec![]
944953
}
945954
} else {
946955
vec![]
@@ -1106,7 +1115,8 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11061115
}
11071116

11081117
targets.clear();
1109-
for component in toolchain.list_components()?.unwrap() {
1118+
let distributable = DistributableToolchain::new(&toolchain)?;
1119+
for component in distributable.list_components()? {
11101120
if component.component.short_name_in_manifest() == "rust-std"
11111121
&& component.available
11121122
&& !component.installed
@@ -1127,7 +1137,8 @@ fn target_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11271137
Some(TargetTriple::new(target)),
11281138
false,
11291139
);
1130-
toolchain.add_component(new_component)?;
1140+
let distributable = DistributableToolchain::new(&toolchain)?;
1141+
distributable.add_component(new_component)?;
11311142
}
11321143

11331144
Ok(())
@@ -1142,8 +1153,9 @@ fn target_remove(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11421153
Some(TargetTriple::new(target)),
11431154
false,
11441155
);
1145-
1146-
toolchain.remove_component(new_component)?;
1156+
let distributable = DistributableToolchain::new(&toolchain)
1157+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
1158+
distributable.remove_component(new_component)?;
11471159
}
11481160

11491161
Ok(())
@@ -1161,8 +1173,9 @@ fn component_list(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11611173

11621174
fn component_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11631175
let toolchain = explicit_or_dir_toolchain(cfg, m)?;
1176+
let distributable = DistributableToolchain::new(&toolchain)?;
11641177
let target = m.value_of("target").map(TargetTriple::new).or_else(|| {
1165-
toolchain
1178+
distributable
11661179
.desc()
11671180
.as_ref()
11681181
.ok()
@@ -1172,17 +1185,18 @@ fn component_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11721185
for component in m.values_of("component").unwrap() {
11731186
let new_component = Component::new_with_target(component, false)
11741187
.unwrap_or_else(|| Component::new(component.to_string(), target.clone(), true));
1175-
1176-
toolchain.add_component(new_component)?;
1188+
distributable.add_component(new_component)?;
11771189
}
11781190

11791191
Ok(())
11801192
}
11811193

11821194
fn component_remove(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11831195
let toolchain = explicit_or_dir_toolchain(cfg, m)?;
1196+
let distributable = DistributableToolchain::new(&toolchain)
1197+
.chain_err(|| rustup::ErrorKind::ComponentsUnsupported(toolchain.name().to_string()))?;
11841198
let target = m.value_of("target").map(TargetTriple::new).or_else(|| {
1185-
toolchain
1199+
distributable
11861200
.desc()
11871201
.as_ref()
11881202
.ok()
@@ -1192,8 +1206,7 @@ fn component_remove(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
11921206
for component in m.values_of("component").unwrap() {
11931207
let new_component = Component::new_with_target(component, false)
11941208
.unwrap_or_else(|| Component::new(component.to_string(), target.clone(), true));
1195-
1196-
toolchain.remove_component(new_component)?;
1209+
distributable.remove_component(new_component)?;
11971210
}
11981211

11991212
Ok(())
@@ -1221,9 +1234,13 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
12211234
let path = m.value_of("path").unwrap();
12221235
let toolchain = cfg.get_toolchain(toolchain, true)?;
12231236

1224-
toolchain
1225-
.install_from_dir(Path::new(path), true)
1226-
.map_err(Into::into)
1237+
if let Ok(custom) = CustomToolchain::new(&toolchain) {
1238+
custom
1239+
.install_from_dir(Path::new(path), true)
1240+
.map_err(Into::into)
1241+
} else {
1242+
Err(ErrorKind::InvalidCustomToolchainName(toolchain.name().to_string()).into())
1243+
}
12271244
}
12281245

12291246
fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result<()> {
@@ -1239,7 +1256,8 @@ fn override_add(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
12391256
let toolchain = cfg.get_toolchain(toolchain, false)?;
12401257

12411258
let status = if !toolchain.is_custom() {
1242-
Some(toolchain.install_from_dist_if_not_installed()?)
1259+
let distributable = DistributableToolchain::new(&toolchain)?;
1260+
Some(distributable.install_from_dist_if_not_installed()?)
12431261
} else if !toolchain.exists() {
12441262
return Err(ErrorKind::ToolchainNotInstalled(toolchain.name().to_string()).into());
12451263
} else {
@@ -1326,28 +1344,26 @@ const DOCS_DATA: &[(&str, &str, &str,)] = &[
13261344

13271345
fn doc(cfg: &Cfg, m: &ArgMatches<'_>) -> Result<()> {
13281346
let toolchain = explicit_or_dir_toolchain(cfg, m)?;
1329-
match toolchain.list_components()? {
1330-
None => { /* custom - no validation */ }
1331-
Some(components) => {
1332-
if let [_] = components
1333-
.into_iter()
1334-
.filter(|cstatus| {
1335-
cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
1336-
})
1337-
.take(1)
1338-
.collect::<Vec<ComponentStatus>>()
1339-
.as_slice()
1340-
{
1341-
info!(
1342-
"`rust-docs` not installed in toolchain `{}`",
1343-
toolchain.name()
1344-
);
1345-
info!(
1346-
"To install, try `rustup component add --toolchain {} rust-docs`",
1347-
toolchain.name()
1348-
);
1349-
return Err("unable to view documentation which is not installed".into());
1350-
}
1347+
if let Ok(distributable) = DistributableToolchain::new(&toolchain) {
1348+
let components = distributable.list_components()?;
1349+
if let [_] = components
1350+
.into_iter()
1351+
.filter(|cstatus| {
1352+
cstatus.component.short_name_in_manifest() == "rust-docs" && !cstatus.installed
1353+
})
1354+
.take(1)
1355+
.collect::<Vec<ComponentStatus>>()
1356+
.as_slice()
1357+
{
1358+
info!(
1359+
"`rust-docs` not installed in toolchain `{}`",
1360+
toolchain.name()
1361+
);
1362+
info!(
1363+
"To install, try `rustup component add --toolchain {} rust-docs`",
1364+
toolchain.name()
1365+
);
1366+
return Err("unable to view documentation which is not installed".into());
13511367
}
13521368
}
13531369
let topical_path: PathBuf;

‎src/cli/self_update.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ use crate::errors::*;
3535
use crate::markdown::md;
3636
use crate::term2;
3737
use rustup::dist::dist::{self, Profile, TargetTriple};
38+
use rustup::toolchain::DistributableToolchain;
3839
use rustup::utils::utils;
3940
use rustup::utils::Notification;
4041
use rustup::{Cfg, UpdateStatus};
@@ -787,7 +788,8 @@ fn maybe_install_rust(
787788
if toolchain.exists() {
788789
warn!("Updating existing toolchain, profile choice will be ignored");
789790
}
790-
let status = toolchain.install_from_dist(true, false, components, targets)?;
791+
let distributable = DistributableToolchain::new(&toolchain)?;
792+
let status = distributable.install_from_dist(true, false, components, targets)?;
791793
cfg.set_default(toolchain_str)?;
792794
println!();
793795
common::show_channel_update(&cfg, toolchain_str, Ok(status))?;

0 commit comments

Comments
 (0)