Refactor ListError to Use quick-error

src/bin/list/list.rs

@@ -1,5 +1,3 @@
-use std::error::Error;
-
 use pad::{Alignment, PadStr};
 use toml;
 
@@ -8,7 +6,7 @@ use list_error::ListError;
 
 /// List the dependencies for manifest section
 #[allow(deprecated)] // connect -> join
-pub fn list_section(manifest: &Manifest, section: &str) -> Result<String, Box<Error>> {
+pub fn list_section(manifest: &Manifest, section: &str) -> Result<String, ListError> {
     let mut output = vec![];
 
     let list = try!(manifest.data
@@ -26,7 +24,7 @@ pub fn list_section(manifest: &Manifest, section: &str) -> Result<String, Box<Er
                         .and_then(|field| field.as_str().map(|s| s.to_owned()))
                         .or_else(|| val.lookup("git").map(|repo| format!("git: {}", repo)))
                         .or_else(|| val.lookup("path").map(|path| format!("path: {}", path)))
-                        .ok_or(ListError::VersionMissing(name.clone())))
+                        .ok_or_else(|| ListError::VersionMissing(name.clone(), section.to_owned())))
             }
             _ => String::from(""),
         };

src/bin/list/list_error.rs

@@ -1,27 +1,24 @@
-use std::fmt;
-use std::error::Error;
-
-#[derive(Debug)]
-pub enum ListError {
-    SectionMissing(String),
-    VersionMissing(String),
-}
-
-impl Error for ListError {
-    fn description(&self) -> &'static str {
-        match *self {
-            ListError::SectionMissing(_) => "Couldn't read section",
-            ListError::VersionMissing(_) => "Couldn't read version",
+quick_error! {
+    #[derive(Debug)]
+    pub enum ListError {
+        SectionMissing(section: String) {
+            description("Couldn't read section")
+            display("Couldn't read section `{}`.", section)
+        }
+        VersionMissing(dep: String, section: String) {
+            description("Couldn't read version")
+            display("Couldn't read version of `{}` in section `{}`.", dep, section)
+        }
+        PackagesMissing {
+            description("Couldn't read list of packages in `Cargo.lock` file.")
+        }
+        PackageInvalid {
+            description("Invalid package record")
+            display("Invalid package record in `Cargo.lock`")
+        }
+        PackageFieldMissing(field: &'static str) {
+            description("Field missing in package record")
+            display("Field `{}` missing in package record in `Cargo.lock`.", field)
         }
-    }
-}
-
-impl fmt::Display for ListError {
-    fn fmt(&self, format: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        let desc: String = match *self {
-            ListError::SectionMissing(ref name) => format!("Couldn't read section {}", name),
-            ListError::VersionMissing(ref name) => format!("Couldn't read version of {}", name),
-        };
-        format.write_str(&desc)
     }
 }

src/bin/list/main.rs

@@ -8,6 +8,8 @@ extern crate docopt;
 extern crate rustc_serialize;
 extern crate pad;
 extern crate toml;
+#[macro_use]
+extern crate quick_error;
 
 use std::error::Error;
 use std::process;
@@ -67,21 +69,17 @@ impl Args {
     }
 }
 
-fn handle_list(args: &Args) -> Result<(), Box<Error>> {
-    let listing = if args.flag_tree {
+fn handle_list(args: &Args) -> Result<String, Box<Error>> {
+    if args.flag_tree {
         let manifest = try!(Manifest::open_lock_file(&args.flag_manifest_path
                                                           .as_ref()
                                                           .map(|s| &s[..])));
         list_tree(&manifest)
     } else {
         let manifest = try!(Manifest::open(&args.flag_manifest_path.as_ref().map(|s| &s[..])));
         list_section(&manifest, args.get_section())
-    };
-
-    listing.map(|listing| println!("{}", listing)).or_else(|err| {
-        println!("Could not list your stuff.\n\nERROR: {}", err);
-        Err(err)
-    })
+    }
+    .map_err(From::from)
 }
 
 fn main() {
@@ -94,8 +92,13 @@ fn main() {
         process::exit(0);
     }
 
-    if let Err(err) = handle_list(&args) {
-        write!(io::stderr(), "{}", err).unwrap();
-        process::exit(1);
+    match handle_list(&args) {
+        Ok(list) => {
+            println!("{}", list);
+        }
+        Err(err) => {
+            write!(io::stderr(), "{}\n", err).unwrap();
+            process::exit(1);
+        }
     }
 }

src/bin/list/tree.rs

@@ -1,4 +1,3 @@
-use std::error::Error;
 use std::collections::BTreeMap;
 
 use toml;
@@ -28,7 +27,7 @@ fn parse_dep_from_str(input: &str) -> Option<Dependency> {
     Some((String::from(name), String::from(version)))
 }
 
-fn get_root_deps(lock_file: &toml::Table) -> Result<Vec<Dependency>, Box<Error>> {
+fn get_root_deps(lock_file: &toml::Table) -> Result<Vec<Dependency>, ListError> {
     let root_deps = try!(lock_file.get("root")
                                   .and_then(|field| field.lookup("dependencies"))
                                   .ok_or(ListError::SectionMissing("dependencies".to_owned())));
@@ -49,23 +48,21 @@ fn get_root_deps(lock_file: &toml::Table) -> Result<Vec<Dependency>, Box<Error>>
     Ok(output)
 }
 
-fn get_packages(lock_file: &toml::Table) -> Result<Packages, Box<Error>> {
-    let packages: &toml::Value = try!(lock_file.get("package")
-                                               .ok_or(ListError::SectionMissing("package"
-                                                                                    .to_owned())));
+fn get_packages(lock_file: &toml::Table) -> Result<Packages, ListError> {
+    let packages: &toml::Value = try!(lock_file.get("package").ok_or(ListError::PackagesMissing));
 
     let mut output = BTreeMap::new();
 
     for pkg in packages.as_slice().unwrap_or(&vec![]) {
-        let package = try!(pkg.as_table().ok_or(ListError::SectionMissing("package".to_owned())));
+        let package = try!(pkg.as_table().ok_or(ListError::PackageInvalid));
 
         let name = try!(package.get("name")
                                .and_then(|item| item.as_str())
-                               .ok_or(ListError::SectionMissing("name".to_owned())));
+                               .ok_or(ListError::PackageFieldMissing("name")));
 
         let version = try!(package.get("version")
                                   .and_then(|item| item.as_str())
-                                  .ok_or(ListError::SectionMissing("version".to_owned())));
+                                  .ok_or(ListError::PackageFieldMissing("version")));
 
         let deps: Dependencies = package.get("dependencies")
                                         .and_then(|item| {
@@ -93,10 +90,11 @@ fn get_packages(lock_file: &toml::Table) -> Result<Packages, Box<Error>> {
 fn list_deps_helper(pkgs: &Packages,
                     deps: &[Dependency],
                     levels: &mut Vec<bool>)
-                    -> Result<String, Box<Error>> {
+                    -> Result<String, ListError> {
     let mut output = String::new();
     for (i, dep) in deps.iter().enumerate() {
-        // For any indent level where the parent is the last dependency in the list, we don't want
+        // For any indent level where the parent is the last dependency in the list, we
+        // don't want
         // to print out a branch.
         for is_last in levels.iter().cloned() {
             let indent = if is_last {
@@ -126,12 +124,12 @@ fn list_deps_helper(pkgs: &Packages,
     Ok(output)
 }
 
-fn list_deps(pkgs: &Packages, deps: &[Dependency]) -> Result<String, Box<Error>> {
+fn list_deps(pkgs: &Packages, deps: &[Dependency]) -> Result<String, ListError> {
     list_deps_helper(pkgs, deps, &mut vec![])
 }
 
 /// Parse a `Cargo.lock` file and list its dependencies
-pub fn parse_lock_file(manifest: &Manifest) -> Result<String, Box<Error>> {
+pub fn parse_lock_file(manifest: &Manifest) -> Result<String, ListError> {
     let lock_file = &manifest.data;
 
     let root_deps = try!(get_root_deps(lock_file));

src/manifest.rs

@@ -205,7 +205,7 @@ impl Manifest {
                     if empty {
                         section.remove();
                     }
-                };
+                }
                 result
             }
         }