Merge #167

167: Switch to use error-chain r=bjgill a=bjgill

I've kept the quick-error style errors where they're used multiple times, and otherwise converted to string errors - our errors are for diagnostics only (not for anyone to try and do anything with).

This is the first step to solving #159.

Cargo.toml

@@ -49,8 +49,8 @@ repository = "killercup/cargo-edit"
 
 [dependencies]
 docopt = "0.8"
+error-chain = "0.10.0"
 pad = "0.1"
-quick-error = "1.0.0"
 regex = "0.2"
 reqwest = "0.7.1"
 serde = "1.0"

src/bin/add/args.rs

@@ -4,9 +4,10 @@ use cargo_edit::Dependency;
 use cargo_edit::{get_crate_name_from_github, get_crate_name_from_gitlab, get_crate_name_from_path,
                  get_latest_dependency};
 use semver;
-use std::error::Error;
 use std::path::PathBuf;
 
+use errors::*;
+
 #[derive(Debug, Deserialize)]
 /// Docopts input args.
 pub struct Args {
@@ -60,9 +61,9 @@ impl Args {
     }
 
     /// Build dependencies from arguments
-    pub fn parse_dependencies(&self) -> Result<Vec<Dependency>, Box<Error>> {
+    pub fn parse_dependencies(&self) -> Result<Vec<Dependency>> {
         if !self.arg_crates.is_empty() {
-            let mut result = Vec::<Dependency>::new();
+            let mut result = Vec::new();
             for arg_crate in &self.arg_crates {
                 let le_crate = if crate_name_has_version(arg_crate) {
                     parse_crate_name_with_version(arg_crate)?
@@ -87,7 +88,8 @@ impl Args {
             let dependency = Dependency::new(&self.arg_crate);
 
             if let Some(ref version) = self.flag_vers {
-                semver::VersionReq::parse(version)?;
+                semver::VersionReq::parse(version)
+                    .chain_err(|| "Invalid dependency version requirement")?;
                 dependency.set_version(version)
             } else if let Some(ref repo) = self.flag_git {
                 dependency.set_git(repo)
@@ -171,17 +173,18 @@ fn crate_name_is_path(name: &str) -> bool {
     name.contains('.') || name.contains('/') || name.contains('\\')
 }
 
-fn parse_crate_name_with_version(name: &str) -> Result<Dependency, Box<Error>> {
+fn parse_crate_name_with_version(name: &str) -> Result<Dependency> {
     assert!(crate_name_has_version(name));
 
     let xs: Vec<_> = name.splitn(2, '@').collect();
     let (name, version) = (xs[0], xs[1]);
-    semver::VersionReq::parse(version)?;
+    semver::VersionReq::parse(version)
+        .chain_err(|| "Invalid crate version requirement")?;
 
     Ok(Dependency::new(name).set_version(version))
 }
 
-fn parse_crate_name_from_uri(name: &str) -> Result<Dependency, Box<Error>> {
+fn parse_crate_name_from_uri(name: &str) -> Result<Dependency> {
     if crate_name_is_github_url(name) {
         if let Ok(ref crate_name) = get_crate_name_from_github(name) {
             return Ok(Dependency::new(crate_name).set_git(name));

src/bin/add/main.rs

@@ -3,27 +3,31 @@
        trivial_numeric_casts, unsafe_code, unstable_features, unused_import_braces,
        unused_qualifications)]
 
-extern crate reqwest;
 extern crate docopt;
-extern crate toml;
 #[macro_use]
-extern crate serde_derive;
+extern crate error_chain;
 extern crate semver;
-extern crate serde;
-extern crate serde_json;
+#[macro_use]
+extern crate serde_derive;
 
-use std::error::Error;
 use std::io::{self, Write};
 use std::process;
 
 extern crate cargo_edit;
 use cargo_edit::Manifest;
 
-extern crate regex;
-
 mod args;
 use args::Args;
 
+mod errors {
+    error_chain!{
+        links {
+            CargoEditLib(::cargo_edit::Error, ::cargo_edit::ErrorKind);
+        }
+    }
+}
+use errors::*;
+
 static USAGE: &'static str = r#"
 Usage:
     cargo add <crate> [--dev|--build|--optional] [--vers=<ver>|--git=<uri>|--path=<uri>] [options]
@@ -66,21 +70,27 @@ crates.io registry suggests. One goal of `cargo add` is to prevent you from usin
 dependencies (version set to "*").
 "#;
 
-fn handle_add(args: &Args) -> Result<(), Box<Error>> {
+fn handle_add(args: &Args) -> Result<()> {
     let manifest_path = args.flag_manifest_path.as_ref().map(From::from);
     let mut manifest = Manifest::open(&manifest_path)?;
     let deps = &args.parse_dependencies()?;
 
     deps.iter()
-        .map(|dep| manifest.insert_into_table(&args.get_section(), dep))
-        .collect::<Result<Vec<_>, _>>()
+        .map(|dep| {
+            manifest
+                .insert_into_table(&args.get_section(), dep)
+                .map_err(Into::into)
+        })
+        .collect::<Result<Vec<_>>>()
         .map_err(|err| {
             println!("Could not edit `Cargo.toml`.\n\nERROR: {}", err);
             err
         })?;
 
     let mut file = Manifest::find_file(&manifest_path)?;
-    manifest.write_to_file(&mut file)
+    manifest.write_to_file(&mut file)?;
+
+    Ok(())
 }
 
 fn main() {
@@ -94,11 +104,18 @@ fn main() {
     }
 
     if let Err(err) = handle_add(&args) {
-        writeln!(
-            io::stderr(),
-            "Command failed due to unhandled error: {}\n",
-            err
-        ).unwrap();
+        let mut stderr = io::stderr();
+
+        writeln!(stderr, "Command failed due to unhandled error: {}\n", err).unwrap();
+
+        for e in err.iter().skip(1) {
+            writeln!(stderr, "Caused by: {}", e).unwrap();
+        }
+
+        if let Some(backtrace) = err.backtrace() {
+            writeln!(stderr, "Backtrace: {:?}", backtrace).unwrap();
+        }
+
         process::exit(1);
     }
 }

src/bin/rm/main.rs

@@ -4,12 +4,11 @@
        unused_qualifications)]
 
 extern crate docopt;
-extern crate toml;
-extern crate semver;
+#[macro_use]
+extern crate error_chain;
 #[macro_use]
 extern crate serde_derive;
 
-use std::error::Error;
 use std::io::{self, Write};
 use std::process;
 
@@ -19,6 +18,15 @@ use cargo_edit::Manifest;
 mod args;
 use args::Args;
 
+mod errors {
+    error_chain!{
+        links {
+            CargoEditLib(::cargo_edit::Error, ::cargo_edit::ErrorKind);
+        }
+    }
+}
+use errors::*;
+
 static USAGE: &'static str = r"
 Usage:
     cargo rm <crate> [--dev|--build] [options]
@@ -35,7 +43,7 @@ Options:
 Remove a dependency from a Cargo.toml manifest file.
 ";
 
-fn handle_rm(args: &Args) -> Result<(), Box<Error>> {
+fn handle_rm(args: &Args) -> Result<()> {
     let manifest_path = args.flag_manifest_path.as_ref().map(From::from);
     let mut manifest = Manifest::open(&manifest_path)?;
 
@@ -44,7 +52,9 @@ fn handle_rm(args: &Args) -> Result<(), Box<Error>> {
         .map_err(From::from)
         .and_then(|_| {
             let mut file = Manifest::find_file(&manifest_path)?;
-            manifest.write_to_file(&mut file)
+            manifest.write_to_file(&mut file)?;
+
+            Ok(())
         })
 }
 
@@ -59,11 +69,18 @@ fn main() {
     }
 
     if let Err(err) = handle_rm(&args) {
-        writeln!(
-            io::stderr(),
-            "Could not edit `Cargo.toml`.\n\nERROR: {}",
-            err
-        ).unwrap();
+        let mut stderr = io::stderr();
+
+        writeln!(stderr, "Command failed due to unhandled error: {}\n", err).unwrap();
+
+        for e in err.iter().skip(1) {
+            writeln!(stderr, "Caused by: {}", e).unwrap();
+        }
+
+        if let Some(backtrace) = err.backtrace() {
+            writeln!(stderr, "Backtrace: {:?}", backtrace).unwrap();
+        }
+
         process::exit(1);
     }
 }

src/bin/upgrade/main.rs

@@ -4,19 +4,28 @@
        unused_qualifications)]
 
 extern crate docopt;
-extern crate pad;
+#[macro_use]
+extern crate error_chain;
 #[macro_use]
 extern crate serde_derive;
 extern crate serde_json;
 extern crate toml;
 
-use std::error::Error;
 use std::io::{self, Write};
 use std::process::{self, Command};
 
 extern crate cargo_edit;
 use cargo_edit::{get_latest_dependency, Manifest};
 
+mod errors {
+    error_chain!{
+        links {
+            CargoEditLib(::cargo_edit::Error, ::cargo_edit::ErrorKind);
+        }
+    }
+}
+use errors::*;
+
 static USAGE: &'static str = r"
 Upgrade all dependencies in a manifest file to the latest version.
 
@@ -69,9 +78,9 @@ fn update_manifest(
     manifest_path: &Option<String>,
     only_update: &[String],
     allow_prerelease: bool,
-) -> Result<(), Box<Error>> {
+) -> Result<()> {
     let manifest_path = manifest_path.as_ref().map(From::from);
-    let mut manifest = Manifest::open(&manifest_path).unwrap();
+    let mut manifest = Manifest::open(&manifest_path)?;
 
     for (table_path, table) in manifest.get_sections() {
         for (name, old_value) in &table {
@@ -86,23 +95,28 @@ fn update_manifest(
     }
 
     let mut file = Manifest::find_file(&manifest_path)?;
-    manifest.write_to_file(&mut file)
+    manifest.write_to_file(&mut file)?;
+
+    Ok(())
 }
 
 /// Get a list of the paths of all the (non-virtual) manifests in the workspace.
-fn get_workspace_manifests(manifest_path: &Option<String>) -> Result<Vec<String>, Box<Error>> {
+fn get_workspace_manifests(manifest_path: &Option<String>) -> Result<Vec<String>> {
     let mut metadata_gatherer = Command::new("cargo");
     metadata_gatherer.args(&["metadata", "--no-deps", "--format-version", "1", "-q"]);
 
     if let Some(ref manifest_path) = *manifest_path {
         metadata_gatherer.args(&["--manifest-path", manifest_path]);
     }
 
-    let output = metadata_gatherer.output()?;
+    let output = metadata_gatherer
+        .output()
+        .chain_err(|| "Failed to run `cargo metadata`")?;
 
     if output.status.success() {
-        let metadata: serde_json::Value =
-            serde_json::from_str(&String::from_utf8_lossy(&output.stdout))?;
+        let metadata: serde_json::Value = serde_json::from_str(
+            &String::from_utf8_lossy(&output.stdout),
+        ).chain_err(|| "Cargo metadata not valid JSON")?;
 
         let workspace_members = metadata["packages"]
             .as_array()
@@ -158,11 +172,18 @@ fn main() {
     };
 
     if let Err(err) = output {
-        writeln!(
-            io::stderr(),
-            "Command failed due to unhandled error: {}\n",
-            err
-        ).unwrap();
+        let mut stderr = io::stderr();
+
+        writeln!(stderr, "Command failed due to unhandled error: {}\n", err).unwrap();
+
+        for e in err.iter().skip(1) {
+            writeln!(stderr, "Caused by: {}", e).unwrap();
+        }
+
+        if let Some(backtrace) = err.backtrace() {
+            writeln!(stderr, "Backtrace: {:?}", backtrace).unwrap();
+        }
+
         process::exit(1);
     }
 }