Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error-chain errors. #4090

Merged
merged 12 commits into from
May 31, 2017
52 changes: 52 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ crossbeam = "0.2"
curl = "0.4.6"
docopt = "0.7"
env_logger = "0.4"
error-chain = "0.10.0"
filetime = "0.1"
flate2 = "0.2"
fs2 = "0.4"
Expand Down
6 changes: 3 additions & 3 deletions src/bin/bench.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, CliError, Human, Config, human};
use cargo::util::{CliResult, CliError, Config, CargoErrorKind};
use cargo::util::important_paths::{find_root_manifest_for_wd};

#[derive(RustcDecodable)]
Expand Down Expand Up @@ -128,8 +128,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
None => Ok(()),
Some(err) => {
Err(match err.exit.as_ref().and_then(|e| e.code()) {
Some(i) => CliError::new(human("bench failed"), i),
None => CliError::new(Box::new(Human(err)), 101)
Some(i) => CliError::new("bench failed".into(), i),
None => CliError::new(CargoErrorKind::CargoTestErrorKind(err).into(), 101)
})
}
}
Expand Down
19 changes: 11 additions & 8 deletions src/bin/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::fs;
use std::path::{Path, PathBuf};

use cargo::core::shell::{Verbosity, ColorConfig};
use cargo::util::{self, CliResult, lev_distance, Config, human, CargoResult};
use cargo::util::{self, CliResult, lev_distance, Config, CargoResult, CargoError, CargoErrorKind};
use cargo::util::CliError;

#[derive(RustcDecodable)]
Expand Down Expand Up @@ -84,7 +84,9 @@ fn main() {
let result = (|| {
let args: Vec<_> = try!(env::args_os()
.map(|s| {
s.into_string().map_err(|s| human(format!("invalid unicode in argument: {:?}", s)))
s.into_string().map_err(|s| {
CargoError::from(format!("invalid unicode in argument: {:?}", s))
})
})
.collect());
let rest = &args;
Expand Down Expand Up @@ -180,7 +182,7 @@ fn execute(flags: Flags, config: &Config) -> CliResult {

if let Some(ref code) = flags.flag_explain {
let mut procss = config.rustc()?.process();
procss.arg("--explain").arg(code).exec().map_err(human)?;
procss.arg("--explain").arg(code).exec()?;
return Ok(());
}

Expand Down Expand Up @@ -309,7 +311,7 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[String]) -> C
let command = match path {
Some(command) => command,
None => {
return Err(human(match find_closest(config, cmd) {
return Err(CargoError::from(match find_closest(config, cmd) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where possible I tend to find format!(...).into() a little more ergonomic than wrapping with CargoError::from, but that's not always possible due to inference issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In many cases the compiler was just not figuring it out. I can do another pass to make sure into doesn't work. Is there a preference between a turbofished into and from?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah unfortunately into doesn't work w/ turbofish as the type parameter is in the wrong place, if .into() doesn't work then CargoError::from is fine, no worries!

Some(closest) => {
format!("no such subcommand: `{}`\n\n\tDid you mean `{}`?\n",
cmd,
Expand All @@ -330,11 +332,12 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[String]) -> C
Err(e) => e,
};

if let Some(code) = err.exit.as_ref().and_then(|c| c.code()) {
Err(CliError::code(code))
} else {
Err(CliError::new(Box::new(err), 101))
if let CargoError(CargoErrorKind::ProcessErrorKind(ref perr), ..) = err {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a kind method that should be able to extract this, perhaps that could be used here?

if let Some(code) = perr.exit.as_ref().and_then(|c| c.code()) {
return Err(CliError::code(code));
}
}
Err(CliError::new(err, 101))
}

/// List all runnable commands
Expand Down
4 changes: 2 additions & 2 deletions src/bin/help.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use cargo::util::{CliResult, CliError, Config, human};
use cargo::util::{CliResult, CliError, Config};

#[derive(RustcDecodable)]
pub struct Options;
Expand All @@ -18,5 +18,5 @@ pub fn execute(_: Options, _: &Config) -> CliResult {
// This is a dummy command just so that `cargo help help` works.
// The actual delegation of help flag to subcommands is handled by the
// cargo command.
Err(CliError::new(human("help command should not be executed directly"), 101))
Err(CliError::new("help command should not be executed directly".into(), 101))
}
6 changes: 3 additions & 3 deletions src/bin/locate_project.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo;
use cargo::util::{CliResult, CliError, human, ChainError, Config};
use cargo::util::{CliResult, CliError, Config};
use cargo::util::important_paths::{find_root_manifest_for_wd};

#[derive(RustcDecodable)]
Expand Down Expand Up @@ -28,9 +28,9 @@ pub fn execute(flags: LocateProjectFlags,
let root = find_root_manifest_for_wd(flags.flag_manifest_path, config.cwd())?;

let string = root.to_str()
.chain_error(|| human("Your project path contains \
.ok_or_else(|| "Your project path contains \
characters not representable in \
Unicode"))
Unicode".into())
.map_err(|e| CliError::new(e, 1))?;

let location = ProjectLocation { root: string.to_string() };
Expand Down
6 changes: 3 additions & 3 deletions src/bin/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::io;
use cargo::ops;
use cargo::core::{SourceId, Source};
use cargo::sources::RegistrySource;
use cargo::util::{CliResult, Config, human, ChainError};
use cargo::util::{CliResult, CargoResultExt, Config};

#[derive(RustcDecodable)]
pub struct Options {
Expand Down Expand Up @@ -51,8 +51,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
println!("please visit {}me and paste the API Token below", host);
let mut line = String::new();
let input = io::stdin();
input.lock().read_line(&mut line).chain_error(|| {
human("failed to read stdin")
input.lock().read_line(&mut line).chain_err(|| {
"failed to read stdin"
})?;
line
}
Expand Down
7 changes: 4 additions & 3 deletions src/bin/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::iter::FromIterator;

use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, CliError, Config, Human};
use cargo::util::{CliResult, CliError, Config, CargoErrorKind};
use cargo::util::important_paths::{find_root_manifest_for_wd};

#[derive(RustcDecodable)]
Expand Down Expand Up @@ -113,7 +113,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
// bad and we always want to forward that up.
let exit = match err.exit.clone() {
Some(exit) => exit,
None => return Err(CliError::new(Box::new(Human(err)), 101)),
None => return Err(
CliError::new(CargoErrorKind::ProcessErrorKind(err).into(), 101)),
};

// If `-q` was passed then we suppress extra error information about
Expand All @@ -123,7 +124,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
Err(if options.flag_quiet == Some(true) {
CliError::code(exit_code)
} else {
CliError::new(Box::new(Human(err)), exit_code)
CliError::new(CargoErrorKind::ProcessErrorKind(err).into(), exit_code)
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::env;
use cargo::core::Workspace;
use cargo::ops::{self, CompileOptions, CompileMode, MessageFormat, Packages};
use cargo::util::important_paths::{find_root_manifest_for_wd};
use cargo::util::{CliResult, CliError, Config, human};
use cargo::util::{CliResult, CliError, Config};

#[derive(RustcDecodable)]
pub struct Options {
Expand Down Expand Up @@ -98,8 +98,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
Some("bench") => CompileMode::Bench,
Some("check") => CompileMode::Check,
Some(mode) => {
let err = human(format!("unknown profile: `{}`, use dev,
test, or bench", mode));
let err = format!("unknown profile: `{}`, use dev,
test, or bench", mode).into();
return Err(CliError::new(err, 101))
}
};
Expand Down
6 changes: 3 additions & 3 deletions src/bin/test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, CliError, Human, human, Config};
use cargo::util::{CliResult, CliError, Config, CargoErrorKind};
use cargo::util::important_paths::find_root_manifest_for_wd;

#[derive(RustcDecodable)]
Expand Down Expand Up @@ -163,8 +163,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
None => Ok(()),
Some(err) => {
Err(match err.exit.as_ref().and_then(|e| e.code()) {
Some(i) => CliError::new(human(err.hint()), i),
None => CliError::new(Box::new(Human(err)), 101),
Some(i) => CliError::new(err.hint().into(), i),
None => CliError::new(CargoErrorKind::CargoTestErrorKind(err).into(), 101),
})
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use semver::ReqParseError;
use serde::ser;

use core::{SourceId, Summary, PackageId};
use util::{CargoError, CargoResult, Cfg, CfgExpr, ChainError, human, Config};
use util::{Cfg, CfgExpr, Config};
use util::errors::{CargoResult, CargoResultExt, CargoError};

/// Information about a dependency requested by a Cargo manifest.
/// Cheap to copy.
Expand Down Expand Up @@ -145,7 +146,7 @@ this warning.

Ok(requirement)
}
e => Err(From::from(e)),
e => Err(e.into()),
}
},
Ok(v) => Ok(v),
Expand Down Expand Up @@ -361,13 +362,13 @@ impl ser::Serialize for Platform {
}

impl FromStr for Platform {
type Err = Box<CargoError>;
type Err = CargoError;

fn from_str(s: &str) -> CargoResult<Platform> {
if s.starts_with("cfg(") && s.ends_with(")") {
let s = &s[4..s.len()-1];
s.parse().map(Platform::Cfg).chain_error(|| {
human(format!("failed to parse `{}` as a cfg expression", s))
s.parse().map(Platform::Cfg).chain_err(|| {
format!("failed to parse `{}` as a cfg expression", s)
})
} else {
Ok(Platform::Name(s.to_string()))
Expand Down
11 changes: 6 additions & 5 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use toml;
use core::{Dependency, Manifest, PackageId, SourceId, Target};
use core::{Summary, SourceMap};
use ops;
use util::{CargoResult, Config, LazyCell, ChainError, internal, human, lev_distance};
use util::{Config, LazyCell, internal, lev_distance};
use util::errors::{CargoResult, CargoResultExt};

/// Information about a package that is available somewhere in the file system.
///
Expand Down Expand Up @@ -181,19 +182,19 @@ impl<'cfg> PackageSet<'cfg> {
}

pub fn get(&self, id: &PackageId) -> CargoResult<&Package> {
let slot = self.packages.iter().find(|p| p.0 == *id).chain_error(|| {
let slot = self.packages.iter().find(|p| p.0 == *id).ok_or_else(|| {
internal(format!("couldn't find `{}` in package set", id))
})?;
let slot = &slot.1;
if let Some(pkg) = slot.borrow() {
return Ok(pkg)
}
let mut sources = self.sources.borrow_mut();
let source = sources.get_mut(id.source_id()).chain_error(|| {
let source = sources.get_mut(id.source_id()).ok_or_else(|| {
internal(format!("couldn't find source for `{}`", id))
})?;
let pkg = source.download(id).chain_error(|| {
human("unable to get packages from source")
let pkg = source.download(id).chain_err(|| {
"unable to get packages from source"
})?;
assert!(slot.fill(pkg).is_ok());
Ok(slot.borrow().unwrap())
Expand Down
Loading