Merge pull request #3569 from sphynx/master

Use structopt for command line arguments parsing in cargo-fmt
2025-02-15 00:13:02 +00:00 · 2019-05-22 06:58:39 +09:00 · 2019-05-22 06:58:39 +09:00 · 2787138936
commit 2787138936
parent 0ef2144c82 a7afdeb9b8
3 changed files with 265 additions and 73 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -135,6 +135,20 @@ name = "cfg-if"
 version = "0.1.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"

+[[package]]
+name = "clap"
+version = "2.33.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
+ "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
+ "strsim 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "textwrap 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vec_map 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "cloudabi"
 version = "0.0.3"
@ -314,6 +328,14 @@ dependencies = [
 "regex 1.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

+[[package]]
+name = "heck"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "unicode-segmentation 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "humantime"
 version = "1.2.0"
@ -799,6 +821,7 @@ dependencies = [
 "rustc-workspace-hack 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "serde 1.0.90 (registry+https://github.com/rust-lang/crates.io-index)",
 "serde_json 1.0.39 (registry+https://github.com/rust-lang/crates.io-index)",
+ "structopt 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)",
 "term 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "toml 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "unicode-segmentation 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
@ -886,6 +909,31 @@ name = "stable_deref_trait"
 version = "1.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"

+[[package]]
+name = "strsim"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
+[[package]]
+name = "structopt"
+version = "0.2.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "structopt-derive 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
+name = "structopt-derive"
+version = "0.2.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "heck 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "proc-macro2 0.4.27 (registry+https://github.com/rust-lang/crates.io-index)",
+ "quote 0.6.12 (registry+https://github.com/rust-lang/crates.io-index)",
+ "syn 0.15.32 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "syn"
 version = "0.15.32"
@ -935,6 +983,14 @@ dependencies = [
 "redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

+[[package]]
+name = "textwrap"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "thread_local"
 version = "0.3.6"
@ -981,6 +1037,11 @@ name = "utf8-ranges"
 version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"

+[[package]]
+name = "vec_map"
+version = "0.8.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "walkdir"
 version = "2.2.7"
@ -1045,6 +1106,7 @@ dependencies = [
 "checksum cargo_metadata 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)" = "178d62b240c34223f265a4c1e275e37d62da163d421fc8d7f7e3ee340f803c57"
 "checksum cc 1.0.35 (registry+https://github.com/rust-lang/crates.io-index)" = "5e5f3fee5eeb60324c2781f1e41286bdee933850fff9b3c672587fed5ec58c83"
 "checksum cfg-if 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "11d43355396e872eefb45ce6342e4374ed7bc2b3a502d1b28e36d6e23c05d1f4"
+"checksum clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5067f5bb2d80ef5d68b4c87db81601f0b75bca627bc2ef76b141d7b846a3c6d9"
 "checksum cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f"
 "checksum constant_time_eq 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8ff012e225ce166d4422e0e78419d901719760f62ae2b7969ca6b564d1b54a9e"
 "checksum crossbeam-channel 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "0f0ed1a4de2235cabda8558ff5840bffb97fcb64c97827f354a451307df5f72b"
@ -1065,6 +1127,7 @@ dependencies = [
 "checksum fuchsia-cprng 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 "checksum getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "0a7292d30132fb5424b354f5dc02512a86e4c516fe544bb7a25e7f266951b797"
 "checksum globset 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ef4feaabe24a0a658fd9cf4a9acf6ed284f045c77df0f49020ba3245cfb7b454"
+"checksum heck 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "20564e78d53d2bb135c343b3f47714a56af2061f1c928fdb541dc7b9fdd94205"
 "checksum humantime 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3ca7e5f2e110db35f93b837c81797f3714500b81d517bf20c431b16d3ca4f114"
 "checksum ignore 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "8dc57fa12805f367736a38541ac1a9fc6a52812a0ca959b1d4d4b640a89eb002"
 "checksum itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5b8467d9c1cebe26feb08c640139247fac215782d35371ade9a2136ed6085358"
@ -1128,11 +1191,15 @@ dependencies = [
 "checksum serde_json 1.0.39 (registry+https://github.com/rust-lang/crates.io-index)" = "5a23aa71d4a4d43fdbfaac00eff68ba8a06a51759a89ac3304323e800c4dd40d"
 "checksum smallvec 0.6.9 (registry+https://github.com/rust-lang/crates.io-index)" = "c4488ae950c49d403731982257768f48fada354a5203fe81f9bb6f43ca9002be"
 "checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8"
+"checksum strsim 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
+"checksum structopt 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)" = "3d0760c312538987d363c36c42339b55f5ee176ea8808bbe4543d484a291c8d1"
+"checksum structopt-derive 0.2.15 (registry+https://github.com/rust-lang/crates.io-index)" = "528aeb7351d042e6ffbc2a6fb76a86f9b622fdf7c25932798e7a82cb03bc94c6"
 "checksum syn 0.15.32 (registry+https://github.com/rust-lang/crates.io-index)" = "846620ec526c1599c070eff393bfeeeb88a93afa2513fc3b49f1fea84cf7b0ed"
 "checksum synstructure 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "73687139bf99285483c96ac0add482c3776528beac1d97d444f6e91f203a2015"
 "checksum term 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "edd106a334b7657c10b7c540a0106114feadeb4dc314513e97df481d5d966f42"
 "checksum termcolor 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "4096add70612622289f2fdcdbd5086dc81c1e2675e6ae58d6c4f62a16c6d7f2f"
 "checksum termion 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "689a3bdfaab439fd92bc87df5c4c78417d3cbe537487274e9b0b2dce76e92096"
+"checksum textwrap 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d326610f408c7a4eb6f51c37c330e496b08506c9457c9d34287ecc38809fb060"
 "checksum thread_local 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "c6b53e329000edc2b34dbe8545fd20e55a333362d0a321909685a19bd28c3f1b"
 "checksum toml 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "87c5890a989fa47ecdc7bcb4c63a77a82c18f306714104b1decfd722db17b39e"
 "checksum ucd-util 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "535c204ee4d8434478593480b8f86ab45ec9aae0e83c568ca81abf0fd0e88f86"
@ -1141,6 +1208,7 @@ dependencies = [
 "checksum unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fc72304796d0818e357ead4e000d19c9c174ab23dc11093ac919054d20a6a7fc"
 "checksum unicode_categories 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e"
 "checksum utf8-ranges 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "796f7e48bef87609f7ade7e06495a87d5cd06c7866e6a5cbfceffc558a243737"
+"checksum vec_map 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "05c78687fb1a80548ae3250346c3db86a80a7cdd77bda190189f2d0a0987c81a"
 "checksum walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d9d7ed3431229a144296213105a390676cc49c9b6a72bd19f3176c98e129fa1"
 "checksum winapi 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)" = "f10e386af2b13e47c89e7236a7a14a086791a2b88ebad6df9bf42040195cf770"
 "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
--- a/Cargo.toml
+++ b/Cargo.toml
@ -58,6 +58,7 @@ unicode_categories = "0.1.1"
 dirs = "1.0.4"
 ignore = "0.4.6"
 annotate-snippets = { version = "0.5.0", features = ["ansi_term"] }
+structopt = "0.2.15"

 config_proc_macro = { path = "config_proc_macro" }

--- a/src/cargo-fmt/main.rs
+++ b/src/cargo-fmt/main.rs
@ -1,10 +1,8 @@
 // Inspired by Paul Woolcock's cargo-fmt (https://github.com/pwoolcoc/cargo-fmt/).

-#![cfg(not(test))]
 #![deny(warnings)]

 use cargo_metadata;
-use getopts;

 use std::cmp::Ordering;
 use std::collections::{BTreeMap, BTreeSet};
@ -17,7 +15,41 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::str;

-use getopts::{Matches, Options};
+use structopt::StructOpt;
+
+#[derive(StructOpt, Debug)]
+#[structopt(
+    bin_name = "cargo fmt",
+    author = "",
+    about = "This utility formats all bin and lib files of \
+             the current crate using rustfmt."
+)]
+pub struct Opts {
+    /// No output printed to stdout
+    #[structopt(short = "q", long = "quiet")]
+    quiet: bool,
+
+    /// Use verbose output
+    #[structopt(short = "v", long = "verbose")]
+    verbose: bool,
+
+    /// Print rustfmt version and exit
+    #[structopt(long = "version")]
+    version: bool,
+
+    /// Specify package to format (only usable in workspaces)
+    #[structopt(short = "p", long = "package", value_name = "package")]
+    packages: Vec<String>,
+
+    /// Options passed to rustfmt
+    // 'raw = true' to make `--` explicit.
+    #[structopt(name = "rustfmt_options", raw(raw = "true"))]
+    rustfmt_options: Vec<String>,
+
+    /// Format all packages (only usable in workspaces)
+    #[structopt(long = "all")]
+    format_all: bool,
+}

 fn main() {
    let exit_status = execute();
@ -29,80 +61,43 @@ const SUCCESS: i32 = 0;
 const FAILURE: i32 = 1;

 fn execute() -> i32 {
-    let mut opts = getopts::Options::new();
-    opts.optflag("h", "help", "show this message");
-    opts.optflag("q", "quiet", "no output printed to stdout");
-    opts.optflag("v", "verbose", "use verbose output");
-    opts.optmulti(
-        "p",
-        "package",
-        "specify package to format (only usable in workspaces)",
-        "<package>",
-    );
-    opts.optflag("", "version", "print rustfmt version and exit");
-    opts.optflag("", "all", "format all packages (only usable in workspaces)");
-
-    // If there is any invalid argument passed to `cargo fmt`, return without formatting.
-    let mut is_package_arg = false;
-    for arg in env::args().skip(2).take_while(|a| a != "--") {
-        if arg.starts_with('-') {
-            is_package_arg = arg.starts_with("--package") | arg.starts_with("-p");
-        } else if !is_package_arg {
-            print_usage_to_stderr(&opts, &format!("Invalid argument: `{}`.", arg));
-            return FAILURE;
+    // Drop extra `fmt` argument provided by `cargo`.
+    let mut found_fmt = false;
+    let args = env::args().filter(|x| {
+        if found_fmt {
+            true
        } else {
-            is_package_arg = false;
+            found_fmt = x == "fmt";
+            x != "fmt"
        }
-    }
+    });

-    let matches = match opts.parse(env::args().skip(1).take_while(|a| a != "--")) {
-        Ok(m) => m,
-        Err(e) => {
-            print_usage_to_stderr(&opts, &e.to_string());
-            return FAILURE;
-        }
-    };
+    let opts = Opts::from_iter(args);

-    let verbosity = match (matches.opt_present("v"), matches.opt_present("q")) {
+    let verbosity = match (opts.verbose, opts.quiet) {
        (false, false) => Verbosity::Normal,
        (false, true) => Verbosity::Quiet,
        (true, false) => Verbosity::Verbose,
        (true, true) => {
-            print_usage_to_stderr(&opts, "quiet mode and verbose mode are not compatible");
+            print_usage_to_stderr("quiet mode and verbose mode are not compatible");
            return FAILURE;
        }
    };

-    if matches.opt_present("h") {
-        print_usage_to_stdout(&opts, "");
-        return SUCCESS;
+    if opts.version {
+        return handle_command_status(get_version());
    }

-    if matches.opt_present("version") {
-        return handle_command_status(get_version(), &opts);
-    }
+    let strategy = CargoFmtStrategy::from_opts(&opts);

-    let strategy = CargoFmtStrategy::from_matches(&matches);
-    handle_command_status(format_crate(verbosity, &strategy), &opts)
+    handle_command_status(format_crate(verbosity, &strategy, opts.rustfmt_options))
 }

-macro_rules! print_usage {
-    ($print:ident, $opts:ident, $reason:expr) => {{
-        let msg = format!("{}\nusage: cargo fmt [options]", $reason);
-        $print!(
-            "{}\nThis utility formats all bin and lib files of the current crate using rustfmt. \
-             Arguments after `--` are passed to rustfmt.",
-            $opts.usage(&msg)
-        );
-    }};
-}
-
-fn print_usage_to_stdout(opts: &Options, reason: &str) {
-    print_usage!(println, opts, reason);
-}
-
-fn print_usage_to_stderr(opts: &Options, reason: &str) {
-    print_usage!(eprintln, opts, reason);
+fn print_usage_to_stderr(reason: &str) {
+    eprintln!("{}", reason);
+    let app = Opts::clap();
+    app.write_help(&mut io::stderr())
+        .expect("failed to write to stderr");
 }

 #[derive(Debug, Clone, Copy, PartialEq)]
@ -112,10 +107,10 @@ pub enum Verbosity {
    Quiet,
 }

-fn handle_command_status(status: Result<i32, io::Error>, opts: &getopts::Options) -> i32 {
+fn handle_command_status(status: Result<i32, io::Error>) -> i32 {
    match status {
        Err(e) => {
-            print_usage_to_stderr(opts, &e.to_string());
+            print_usage_to_stderr(&e.to_string());
            FAILURE
        }
        Ok(status) => status,
@ -142,8 +137,11 @@ fn get_version() -> Result<i32, io::Error> {
    }
 }

-fn format_crate(verbosity: Verbosity, strategy: &CargoFmtStrategy) -> Result<i32, io::Error> {
-    let rustfmt_args = get_fmt_args();
+fn format_crate(
+    verbosity: Verbosity,
+    strategy: &CargoFmtStrategy,
+    rustfmt_args: Vec<String>,
+) -> Result<i32, io::Error> {
    let targets = if rustfmt_args
        .iter()
        .any(|s| ["--print-config", "-h", "--help", "-V", "--version"].contains(&s.as_str()))
@ -157,11 +155,6 @@ fn format_crate(verbosity: Verbosity, strategy: &CargoFmtStrategy) -> Result<i32
    run_rustfmt(&targets, &rustfmt_args, verbosity)
 }

-fn get_fmt_args() -> Vec<String> {
-    // All arguments after -- are passed to rustfmt.
-    env::args().skip_while(|a| a != "--").skip(1).collect()
-}
-
 /// Target uses a `path` field for equality and hashing.
 #[derive(Debug)]
 pub struct Target {
@ -223,11 +216,11 @@ pub enum CargoFmtStrategy {
 }

 impl CargoFmtStrategy {
-    pub fn from_matches(matches: &Matches) -> CargoFmtStrategy {
-        match (matches.opt_present("all"), matches.opt_present("p")) {
-            (false, false) => CargoFmtStrategy::Root,
+    pub fn from_opts(opts: &Opts) -> CargoFmtStrategy {
+        match (opts.format_all, opts.packages.is_empty()) {
+            (false, true) => CargoFmtStrategy::Root,
            (true, _) => CargoFmtStrategy::All,
-            (false, true) => CargoFmtStrategy::Some(matches.opt_strs("p")),
+            (false, false) => CargoFmtStrategy::Some(opts.packages.clone()),
        }
    }
 }
@ -403,3 +396,133 @@ fn get_cargo_metadata(manifest_path: Option<&Path>) -> Result<cargo_metadata::Me
        Err(error) => Err(io::Error::new(io::ErrorKind::Other, error.to_string())),
    }
 }
+
+#[cfg(test)]
+mod cargo_fmt_tests {
+    use super::*;
+
+    #[test]
+    fn default_options() {
+        let empty: Vec<String> = vec![];
+        let o = Opts::from_iter(&empty);
+        assert_eq!(false, o.quiet);
+        assert_eq!(false, o.verbose);
+        assert_eq!(false, o.version);
+        assert_eq!(empty, o.packages);
+        assert_eq!(empty, o.rustfmt_options);
+        assert_eq!(false, o.format_all);
+    }
+
+    #[test]
+    fn good_options() {
+        let o = Opts::from_iter(&[
+            "test",
+            "-q",
+            "-p",
+            "p1",
+            "-p",
+            "p2",
+            "--",
+            "--edition",
+            "2018",
+        ]);
+        assert_eq!(true, o.quiet);
+        assert_eq!(false, o.verbose);
+        assert_eq!(false, o.version);
+        assert_eq!(vec!["p1", "p2"], o.packages);
+        assert_eq!(vec!["--edition", "2018"], o.rustfmt_options);
+        assert_eq!(false, o.format_all);
+    }
+
+    #[test]
+    fn unexpected_option() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "unexpected"])
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn unexpected_flag() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "--flag"])
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn mandatory_separator() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "--check"])
+                .is_err()
+        );
+        assert!(
+            !Opts::clap()
+                .get_matches_from_safe(&["test", "--", "--check"])
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn multiple_packages_one_by_one() {
+        let o = Opts::from_iter(&[
+            "test",
+            "-p",
+            "package1",
+            "--package",
+            "package2",
+            "-p",
+            "package3",
+        ]);
+        assert_eq!(3, o.packages.len());
+    }
+
+    #[test]
+    fn multiple_packages_grouped() {
+        let o = Opts::from_iter(&[
+            "test",
+            "--package",
+            "package1",
+            "package2",
+            "-p",
+            "package3",
+            "package4",
+        ]);
+        assert_eq!(4, o.packages.len());
+    }
+
+    #[test]
+    fn empty_packages_1() {
+        assert!(Opts::clap().get_matches_from_safe(&["test", "-p"]).is_err());
+    }
+
+    #[test]
+    fn empty_packages_2() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "-p", "--", "--check"])
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn empty_packages_3() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "-p", "--verbose"])
+                .is_err()
+        );
+    }
+
+    #[test]
+    fn empty_packages_4() {
+        assert!(
+            Opts::clap()
+                .get_matches_from_safe(&["test", "-p", "--check"])
+                .is_err()
+        );
+    }
+}