From fe5fa874da6ce1691add81a46f9bfd0a28d39076 Mon Sep 17 00:00:00 2001 From: Kamal Marhubi Date: Sat, 9 Apr 2016 16:15:36 -0400 Subject: [PATCH] rustfmt: Make error handling more idiomatic This commit replaces the `Operation::InvalidInput` variant with `Result`, and uses the `try!()` macro instead of explicit matching. --- src/bin/rustfmt.rs | 85 +++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index dba91612761..a4067f40eda 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -20,7 +20,7 @@ use rustfmt::{run, Input}; use rustfmt::config::{Config, WriteMode}; -use std::env; +use std::{env, error}; use std::fs::{self, File}; use std::io::{self, ErrorKind, Read, Write}; use std::path::{Path, PathBuf}; @@ -28,6 +28,8 @@ use getopts::{Matches, Options}; +type FmtError = Box; +type FmtResult = std::result::Result; /// Rustfmt operations. enum Operation { @@ -42,10 +44,6 @@ enum Operation { Version, /// Print detailed configuration help. ConfigHelp, - /// Invalid program input. - InvalidInput { - reason: String, - }, /// No file specified, read from stdin Stdin { input: String, @@ -55,7 +53,7 @@ enum Operation { /// Try to find a project file in the given directory and its parents. Returns the path of a the /// nearest project file if one exists, or `None` if no project file was found. -fn lookup_project_file(dir: &Path) -> io::Result> { +fn lookup_project_file(dir: &Path) -> FmtResult> { let mut current = if dir.is_relative() { try!(env::current_dir()).join(dir) } else { @@ -77,7 +75,7 @@ fn lookup_project_file(dir: &Path) -> io::Result> { // return the error. Err(e) => { if e.kind() != ErrorKind::NotFound { - return Err(e); + return Err(FmtError::from(e)); } } } @@ -93,7 +91,7 @@ fn lookup_project_file(dir: &Path) -> io::Result> { /// /// Returns the `Config` to use, and the path of the project file if there was /// one. -fn resolve_config(dir: &Path) -> io::Result<(Config, Option)> { +fn resolve_config(dir: &Path) -> FmtResult<(Config, Option)> { let path = try!(lookup_project_file(dir)); if path.is_none() { return Ok((Config::default(), None)); @@ -108,7 +106,7 @@ fn resolve_config(dir: &Path) -> io::Result<(Config, Option)> { /// read the given config file path recursively if present else read the project file path fn match_cli_path_or_file(config_path: Option, input_file: &Path) - -> io::Result<(Config, Option)> { + -> FmtResult<(Config, Option)> { if let Some(config_file) = config_path { let (toml, path) = try!(resolve_config(config_file.as_ref())); @@ -119,7 +117,7 @@ fn match_cli_path_or_file(config_path: Option, resolve_config(input_file) } -fn update_config(config: &mut Config, matches: &Matches) -> Result<(), String> { +fn update_config(config: &mut Config, matches: &Matches) -> FmtResult<()> { config.verbose = matches.opt_present("verbose"); config.skip_children = matches.opt_present("skip-children"); @@ -130,7 +128,10 @@ fn update_config(config: &mut Config, matches: &Matches) -> Result<(), String> { config.write_mode = write_mode; Ok(()) } - Some(Err(_)) => Err(format!("Invalid write-mode: {}", write_mode.expect("cannot happen"))), + Some(Err(_)) => { + Err(FmtError::from(format!("Invalid write-mode: {}", + write_mode.expect("cannot happen")))) + } } } @@ -157,35 +158,18 @@ fn make_opts() -> Options { opts } -fn execute() -> i32 { - let opts = make_opts(); +fn execute(opts: &Options) -> FmtResult<()> { + let matches = try!(opts.parse(env::args().skip(1))); - let matches = match opts.parse(env::args().skip(1)) { - Ok(m) => m, - Err(e) => { - print_usage(&opts, &e.to_string()); - return 1; - } - }; - - let operation = determine_operation(&matches); - - match operation { - Operation::InvalidInput { reason } => { - print_usage(&opts, &reason); - 1 - } + match try!(determine_operation(&matches)) { Operation::Help => { print_usage(&opts, ""); - 0 } Operation::Version => { print_version(); - 0 } Operation::ConfigHelp => { Config::print_docs(); - 0 } Operation::Stdin { input, config_path } => { // try to read config from local directory @@ -196,7 +180,6 @@ fn execute() -> i32 { config.write_mode = WriteMode::Plain; run(Input::Text(input), &config); - 0 } Operation::Format { files, config_path } => { let mut config = Config::default(); @@ -227,21 +210,26 @@ fn execute() -> i32 { config = config_tmp; } - if let Err(e) = update_config(&mut config, &matches) { - print_usage(&opts, &e); - return 1; - } + try!(update_config(&mut config, &matches)); run(Input::File(file), &config); } - 0 } } + Ok(()) } fn main() { let _ = env_logger::init(); - let exit_code = execute(); + let opts = make_opts(); + + let exit_code = match execute(&opts) { + Ok(..) => 0, + Err(e) => { + print_usage(&opts, &e.to_string()); + 1 + } + }; // Make sure standard output is flushed before we exit. std::io::stdout().flush().unwrap(); @@ -267,17 +255,17 @@ fn print_version() { option_env!("CARGO_PKG_VERSION_PRE").unwrap_or("")); } -fn determine_operation(matches: &Matches) -> Operation { +fn determine_operation(matches: &Matches) -> FmtResult { if matches.opt_present("h") { - return Operation::Help; + return Ok(Operation::Help); } if matches.opt_present("config-help") { - return Operation::ConfigHelp; + return Ok(Operation::ConfigHelp); } if matches.opt_present("version") { - return Operation::Version; + return Ok(Operation::Version); } // Read the config_path and convert to parent dir if a file is provided. @@ -294,21 +282,18 @@ fn determine_operation(matches: &Matches) -> Operation { if matches.free.is_empty() { let mut buffer = String::new(); - match io::stdin().read_to_string(&mut buffer) { - Ok(..) => (), - Err(e) => return Operation::InvalidInput { reason: e.to_string() }, - } + try!(io::stdin().read_to_string(&mut buffer)); - return Operation::Stdin { + return Ok(Operation::Stdin { input: buffer, config_path: config_path, - }; + }); } let files: Vec<_> = matches.free.iter().map(PathBuf::from).collect(); - Operation::Format { + Ok(Operation::Format { files: files, config_path: config_path, - } + }) }