From fe5f6687da8a9f8927d87853c8a871d146c2dbbf Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 21 Nov 2019 22:56:54 -0600 Subject: [PATCH 1/4] refactor: switch to non-recursive mode for default --- Configurations.md | 8 + src/bin/main.rs | 23 +- src/cargo-fmt/main.rs | 223 +++++++++++++----- src/config/mod.rs | 32 ++- src/formatting.rs | 6 +- src/syntux/parser.rs | 4 +- src/test/mod.rs | 4 + .../source/configs/recursive/disabled/foo.rs | 6 + .../source/configs/recursive/disabled/lib.rs | 10 + tests/source/configs/recursive/enabled/foo.rs | 6 + tests/source/configs/recursive/enabled/lib.rs | 10 + .../target/configs/recursive/disabled/foo.rs | 6 + .../target/configs/recursive/disabled/lib.rs | 7 + tests/target/configs/recursive/enabled/foo.rs | 3 + tests/target/configs/recursive/enabled/lib.rs | 7 + 15 files changed, 288 insertions(+), 67 deletions(-) create mode 100644 tests/source/configs/recursive/disabled/foo.rs create mode 100644 tests/source/configs/recursive/disabled/lib.rs create mode 100644 tests/source/configs/recursive/enabled/foo.rs create mode 100644 tests/source/configs/recursive/enabled/lib.rs create mode 100644 tests/target/configs/recursive/disabled/foo.rs create mode 100644 tests/target/configs/recursive/disabled/lib.rs create mode 100644 tests/target/configs/recursive/enabled/foo.rs create mode 100644 tests/target/configs/recursive/enabled/lib.rs diff --git a/Configurations.md b/Configurations.md index a13fe062d89..36c264f86f3 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1720,6 +1720,14 @@ fn example() { } ``` +## `recursive` + +Format all encountered modules recursively, including those defined in external files. + +- **Default value**: `false` +- **Possible values**: `true`, `false` +- **Stable**: Yes + ## `remove_nested_parens` Remove nested parens. diff --git a/src/bin/main.rs b/src/bin/main.rs index 75102aa0673..bbf46199801 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -170,7 +170,11 @@ fn make_opts() -> Options { "Don't reformat child modules (unstable).", ); } - + opts.optflag( + "r", + "recursive", + "Format all encountered modules recursively, including those defined in external files.", + ); opts.optflag("v", "verbose", "Print verbose output"); opts.optflag("q", "quiet", "Print less output"); opts.optflag("V", "version", "Show version information"); @@ -500,6 +504,7 @@ const STABLE_EMIT_MODES: [EmitMode; 3] = [EmitMode::Files, EmitMode::Stdout, Emi #[derive(Clone, Debug, Default)] struct GetOptsOptions { skip_children: Option, + recursive: Option, quiet: bool, verbose: bool, config_path: Option, @@ -568,6 +573,19 @@ impl GetOptsOptions { } } + if matches.opt_present("recursive") { + if let Some(skip) = options.skip_children { + if skip { + return Err(format_err!( + "Conflicting config options `skip_children` and `recursive` are \ + both enabled. `skip_children` has been deprecated and should be \ + removed from your config.", + )); + } + } + options.recursive = Some(true); + } + options.config_path = matches.opt_str("config-path").map(PathBuf::from); options.inline_config = matches @@ -658,6 +676,9 @@ impl CliOptions for GetOptsOptions { if let Some(skip_children) = self.skip_children { config.set().skip_children(skip_children); } + if let Some(recursive) = self.recursive { + config.set().recursive(recursive); + } if let Some(error_on_unformatted) = self.error_on_unformatted { config.set().error_on_unformatted(error_on_unformatted); } diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index 68f901df5e4..3c240eee5b1 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -112,20 +112,13 @@ fn execute() -> i32 { let strategy = CargoFmtStrategy::from_opts(&opts); let mut rustfmt_args = opts.rustfmt_options; - if opts.check { - let check_flag = String::from("--check"); - if !rustfmt_args.contains(&check_flag) { - rustfmt_args.push(check_flag); - } + let cargo_fmt_check = opts.check; + if let Err(ref msg) = + build_rustfmt_args(opts.message_format, &mut rustfmt_args, cargo_fmt_check) + { + print_usage_to_stderr(msg); + return FAILURE; } - if let Some(message_format) = opts.message_format { - if let Err(msg) = convert_message_format_to_rustfmt_args(&message_format, &mut rustfmt_args) - { - print_usage_to_stderr(&msg); - return FAILURE; - } - } - let include_nested_test_files = opts.include_nested_test_files; if let Some(specified_manifest_path) = opts.manifest_path { @@ -152,13 +145,16 @@ fn execute() -> i32 { } } -fn convert_message_format_to_rustfmt_args( - message_format: &str, +fn build_rustfmt_args( + message_format: Option, rustfmt_args: &mut Vec, + check: bool, ) -> Result<(), String> { - let mut contains_emit_mode = false; let mut contains_check = false; + let mut contains_emit_mode = false; let mut contains_list_files = false; + let mut contains_recursive = false; + for arg in rustfmt_args.iter() { if arg.starts_with("--emit") { contains_emit_mode = true; @@ -169,37 +165,53 @@ fn convert_message_format_to_rustfmt_args( if arg == "-l" || arg == "--files-with-diff" { contains_list_files = true; } + if arg == "-r" || arg == "--recursive" { + contains_recursive = true; + } + } + + if check && !contains_check { + rustfmt_args.push(String::from("--check")); } - match message_format { - "short" => { - if !contains_list_files { - rustfmt_args.push(String::from("-l")); + + if !contains_recursive { + rustfmt_args.push(String::from("--recursive")); + } + + if let Some(format) = message_format { + return match format.as_ref() { + "short" => { + if !contains_list_files { + rustfmt_args.push(String::from("-l")); + } + Ok(()) } - Ok(()) - } - "json" => { - if contains_emit_mode { - return Err(String::from( - "cannot include --emit arg when --message-format is set to json", - )); + "json" => { + if contains_emit_mode { + return Err(String::from( + "cannot include --emit arg when --message-format is set to json", + )); + } + if contains_check { + return Err(String::from( + "cannot include --check arg when --message-format is set to json", + )); + } + rustfmt_args.push(String::from("--emit")); + rustfmt_args.push(String::from("json")); + Ok(()) } - if contains_check { - return Err(String::from( - "cannot include --check arg when --message-format is set to json", + "human" => Ok(()), + _ => { + return Err(format!( + "invalid --message-format value: {}. Allowed values are: short|json|human", + format )); } - rustfmt_args.push(String::from("--emit")); - rustfmt_args.push(String::from("json")); - Ok(()) - } - "human" => Ok(()), - _ => { - return Err(format!( - "invalid --message-format value: {}. Allowed values are: short|json|human", - message_format - )); - } + }; } + + Ok(()) } fn print_usage_to_stderr(reason: &str) { @@ -801,13 +813,13 @@ mod cargo_fmt_tests { ); } - mod convert_message_format_to_rustfmt_args_tests { + mod build_rustfmt_args_tests { use super::*; #[test] fn invalid_message_format() { assert_eq!( - convert_message_format_to_rustfmt_args("awesome", &mut vec![]), + build_rustfmt_args(Some(String::from("awesome")), &mut vec![], false), Err(String::from( "invalid --message-format value: awesome. Allowed values are: short|json|human" )), @@ -818,7 +830,7 @@ mod cargo_fmt_tests { fn json_message_format_and_check_arg() { let mut args = vec![String::from("--check")]; assert_eq!( - convert_message_format_to_rustfmt_args("json", &mut args), + build_rustfmt_args(Some(String::from("json")), &mut args, false), Err(String::from( "cannot include --check arg when --message-format is set to json" )), @@ -829,7 +841,7 @@ mod cargo_fmt_tests { fn json_message_format_and_emit_arg() { let mut args = vec![String::from("--emit"), String::from("checkstyle")]; assert_eq!( - convert_message_format_to_rustfmt_args("json", &mut args), + build_rustfmt_args(Some(String::from("json")), &mut args, false), Err(String::from( "cannot include --emit arg when --message-format is set to json" )), @@ -838,50 +850,143 @@ mod cargo_fmt_tests { #[test] fn json_message_format() { - let mut args = vec![String::from("--edition"), String::from("2018")]; - assert!(convert_message_format_to_rustfmt_args("json", &mut args).is_ok()); + let mut args = vec![ + String::from("--edition"), + String::from("2018"), + String::from("--recursive"), + ]; + assert!(build_rustfmt_args(Some(String::from("json")), &mut args, false).is_ok()); assert_eq!( args, vec![ String::from("--edition"), String::from("2018"), + String::from("--recursive"), String::from("--emit"), - String::from("json") + String::from("json"), ] ); } #[test] fn human_message_format() { - let exp_args = vec![String::from("--emit"), String::from("json")]; + let exp_args = vec![ + String::from("--emit"), + String::from("json"), + String::from("--recursive"), + ]; let mut act_args = exp_args.clone(); - assert!(convert_message_format_to_rustfmt_args("human", &mut act_args).is_ok()); + assert!(build_rustfmt_args(Some(String::from("human")), &mut act_args, false).is_ok()); assert_eq!(act_args, exp_args); } #[test] fn short_message_format() { - let mut args = vec![String::from("--check")]; - assert!(convert_message_format_to_rustfmt_args("short", &mut args).is_ok()); - assert_eq!(args, vec![String::from("--check"), String::from("-l")]); + let mut args = vec![String::from("--check"), String::from("--recursive")]; + assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + assert_eq!( + args, + vec![ + String::from("--check"), + String::from("--recursive"), + String::from("-l"), + ], + ); } #[test] fn short_message_format_included_short_list_files_flag() { - let mut args = vec![String::from("--check"), String::from("-l")]; - assert!(convert_message_format_to_rustfmt_args("short", &mut args).is_ok()); - assert_eq!(args, vec![String::from("--check"), String::from("-l")]); + let mut args = vec![ + String::from("--check"), + String::from("-l"), + String::from("--recursive"), + ]; + assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + assert_eq!( + args, + vec![ + String::from("--check"), + String::from("-l"), + String::from("--recursive"), + ], + ); } #[test] fn short_message_format_included_long_list_files_flag() { - let mut args = vec![String::from("--check"), String::from("--files-with-diff")]; - assert!(convert_message_format_to_rustfmt_args("short", &mut args).is_ok()); + let mut args = vec![ + String::from("--check"), + String::from("--files-with-diff"), + String::from("--recursive"), + ]; + assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + assert_eq!( + args, + vec![ + String::from("--check"), + String::from("--files-with-diff"), + String::from("--recursive"), + ] + ); + } + + #[test] + fn recursive_shorthand_not_duplicated() { + let mut args = vec![String::from("-r")]; + assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + assert_eq!(args, vec![String::from("-r")]); + } + + #[test] + fn recursive_long_not_duplicated() { + let mut args = vec![String::from("--recursive")]; + assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + assert_eq!(args, vec![String::from("--recursive")]); + } + + #[test] + fn recursive_added() { + let mut args = vec![]; + assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + assert_eq!(args, vec![String::from("--recursive")]); + } + + #[test] + fn check_not_duplicated_when_included_in_cargo_fmt() { + let mut args = vec![String::from("--check"), String::from("--recursive")]; + assert!(build_rustfmt_args(None, &mut args, true).is_ok()); assert_eq!( args, - vec![String::from("--check"), String::from("--files-with-diff")] + vec![String::from("--check"), String::from("--recursive")], ); } + + #[test] + fn check_still_passed_through_when_not_included_in_cargo_fmt() { + let mut args = vec![String::from("--check"), String::from("--recursive")]; + assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + assert_eq!( + args, + vec![String::from("--check"), String::from("--recursive")], + ); + } + + #[test] + fn check_added() { + let mut args = vec![String::from("--recursive")]; + assert!(build_rustfmt_args(None, &mut args, true).is_ok()); + assert_eq!( + args, + vec![String::from("--recursive"), String::from("--check")], + ); + } + + #[test] + fn check_not_added() { + let mut args = vec![String::from("--recursive")]; + assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + assert_eq!(args, vec![String::from("--recursive")]); + } } mod get_nested_integration_test_files_tests { diff --git a/src/config/mod.rs b/src/config/mod.rs index 698a3224591..ccff9eaa2ce 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -130,6 +130,8 @@ create_config! { "Enables unstable features. Only available on nightly channel"; disable_all_formatting: bool, false, false, "Don't reformat anything"; skip_children: bool, false, false, "Don't reformat out of line modules"; + recursive: bool, false, true, + "Format all encountered modules recursively, including those defined in external files."; hide_parse_errors: bool, false, false, "Hide errors from the parser"; error_on_line_overflow: bool, false, false, "Error if unable to get all lines within max_width"; error_on_unformatted: bool, false, false, @@ -279,7 +281,15 @@ impl Config { if !err.is_empty() { eprint!("{}", err); } - Ok(Config::default().fill_from_parsed_config(parsed_config, dir)) + let config = Config::default().fill_from_parsed_config(parsed_config, dir); + if config.skip_children() && config.recursive() { + return Err(String::from( + "Error: Conflicting config options `skip_children` and `recursive` are \ + both enabled. `skip_children` has been deprecated and should be \ + removed from your config.", + )); + } + Ok(config) } Err(e) => { err.push_str("Error: Decoding config file failed:\n"); @@ -483,6 +493,25 @@ mod test { assert!(config.license_template.is_some()); } + #[test] + fn test_conflicting_recursive_skip_children() { + if !crate::is_nightly_channel!() { + return; + } + + let toml = "skip_children = true\nrecursive = true"; + // Update to `contains_err()` once it lands + // https://github.com/rust-lang/rust/issues/62358 + // https://doc.rust-lang.org/std/result/enum.Result.html#method.contains_err + match Config::from_toml(toml, Path::new("")) { + Ok(_) => panic!("Expected configuration error"), + Err(msg) => assert_eq!( + msg, + "Error: Conflicting config options `skip_children` and `recursive` are both enabled. `skip_children` has been deprecated and should be removed from your config.", + ), + } + } + #[test] fn test_dump_default_config() { let default_config = format!( @@ -544,6 +573,7 @@ required_version = "{}" unstable_features = false disable_all_formatting = false skip_children = false +recursive = false hide_parse_errors = false error_on_line_overflow = false error_on_unformatted = false diff --git a/src/formatting.rs b/src/formatting.rs index 0433542f4eb..eb4f37d1ab2 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -63,7 +63,7 @@ fn format_project( let input_is_stdin = main_file == FileName::Stdin; let mut parse_session = ParseSess::new(config)?; - if config.skip_children() && parse_session.ignore_file(&main_file) { + if !config.recursive() && parse_session.ignore_file(&main_file) { return Ok(FormatReport::new()); } @@ -91,14 +91,14 @@ fn format_project( let files = modules::ModResolver::new( &context.parse_session, directory_ownership.unwrap_or(DirectoryOwnership::UnownedViaMod(true)), - !(input_is_stdin || config.skip_children()), + !(input_is_stdin || !config.recursive()), ) .visit_crate(&krate) .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; for (path, module) in files { let should_ignore = !input_is_stdin && context.ignore_file(&path); - if (config.skip_children() && path != main_file) || should_ignore { + if (!config.recursive() && path != main_file) || should_ignore { continue; } should_emit_verbose(input_is_stdin, config, || println!("Formatting {}", path)); diff --git a/src/syntux/parser.rs b/src/syntux/parser.rs index 3cfed10a459..b4da22f23ae 100644 --- a/src/syntux/parser.rs +++ b/src/syntux/parser.rs @@ -84,10 +84,8 @@ impl<'a> ParserBuilder<'a> { }; parser.cfg_mods = false; + parser.recurse_into_file_modules = config.recursive(); - if config.skip_children() { - parser.recurse_into_file_modules = false; - } Ok(Parser { parser, sess }) } diff --git a/src/test/mod.rs b/src/test/mod.rs index fe3378ac1e0..a208b1d75b6 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -40,6 +40,10 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[ "cfg_mod/bar.rs", "cfg_mod/foo.rs", "cfg_mod/wasm32.rs", + // We want to ensure `recursive` is working correctly, so do not test + // these files directly + "configs/recursive/disabled/foo.rs", + "configs/recursive/enabled/foo.rs", ]; fn init_log() { diff --git a/tests/source/configs/recursive/disabled/foo.rs b/tests/source/configs/recursive/disabled/foo.rs new file mode 100644 index 00000000000..d9f4c13f822 --- /dev/null +++ b/tests/source/configs/recursive/disabled/foo.rs @@ -0,0 +1,6 @@ +pub fn foo( ) { + +println!( + "bar" + ); +} diff --git a/tests/source/configs/recursive/disabled/lib.rs b/tests/source/configs/recursive/disabled/lib.rs new file mode 100644 index 00000000000..77603889f16 --- /dev/null +++ b/tests/source/configs/recursive/disabled/lib.rs @@ -0,0 +1,10 @@ +// rustfmt-recursive: false + +mod foo; + +fn bar( + +){ +println!( + "baz") +;} diff --git a/tests/source/configs/recursive/enabled/foo.rs b/tests/source/configs/recursive/enabled/foo.rs new file mode 100644 index 00000000000..d9f4c13f822 --- /dev/null +++ b/tests/source/configs/recursive/enabled/foo.rs @@ -0,0 +1,6 @@ +pub fn foo( ) { + +println!( + "bar" + ); +} diff --git a/tests/source/configs/recursive/enabled/lib.rs b/tests/source/configs/recursive/enabled/lib.rs new file mode 100644 index 00000000000..f6b405452d7 --- /dev/null +++ b/tests/source/configs/recursive/enabled/lib.rs @@ -0,0 +1,10 @@ +// rustfmt-recursive: true + +mod foo; + +fn bar( + +){ +println!( + "baz") +;} diff --git a/tests/target/configs/recursive/disabled/foo.rs b/tests/target/configs/recursive/disabled/foo.rs new file mode 100644 index 00000000000..d9f4c13f822 --- /dev/null +++ b/tests/target/configs/recursive/disabled/foo.rs @@ -0,0 +1,6 @@ +pub fn foo( ) { + +println!( + "bar" + ); +} diff --git a/tests/target/configs/recursive/disabled/lib.rs b/tests/target/configs/recursive/disabled/lib.rs new file mode 100644 index 00000000000..6bc334d2eb0 --- /dev/null +++ b/tests/target/configs/recursive/disabled/lib.rs @@ -0,0 +1,7 @@ +// rustfmt-recursive: false + +mod foo; + +fn bar() { + println!("baz"); +} diff --git a/tests/target/configs/recursive/enabled/foo.rs b/tests/target/configs/recursive/enabled/foo.rs new file mode 100644 index 00000000000..e63457e6eb9 --- /dev/null +++ b/tests/target/configs/recursive/enabled/foo.rs @@ -0,0 +1,3 @@ +pub fn foo() { + println!("bar"); +} diff --git a/tests/target/configs/recursive/enabled/lib.rs b/tests/target/configs/recursive/enabled/lib.rs new file mode 100644 index 00000000000..3a1be93899e --- /dev/null +++ b/tests/target/configs/recursive/enabled/lib.rs @@ -0,0 +1,7 @@ +// rustfmt-recursive: true + +mod foo; + +fn bar() { + println!("baz"); +} From 3c76a63621f256e793dfa307cd32753c4481a8d0 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sun, 24 Nov 2019 20:29:07 -0600 Subject: [PATCH 2/4] refactor: pass opts ref to build_rustfmt_args --- src/bin/main.rs | 13 ++--- src/cargo-fmt/main.rs | 116 ++++++++++++++++++++++-------------------- 2 files changed, 67 insertions(+), 62 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index bbf46199801..47c746f855b 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -574,14 +574,11 @@ impl GetOptsOptions { } if matches.opt_present("recursive") { - if let Some(skip) = options.skip_children { - if skip { - return Err(format_err!( - "Conflicting config options `skip_children` and `recursive` are \ - both enabled. `skip_children` has been deprecated and should be \ - removed from your config.", - )); - } + if let Some(true) = options.skip_children { + return Err(format_err!( + "Conflicting options `skip_children` and `recursive` were specified. \ + `skip_children` has been deprecated and should no longer be used. ", + )); } options.recursive = Some(true); } diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index 3c240eee5b1..7c7e600b7d8 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -111,11 +111,8 @@ fn execute() -> i32 { } let strategy = CargoFmtStrategy::from_opts(&opts); - let mut rustfmt_args = opts.rustfmt_options; - let cargo_fmt_check = opts.check; - if let Err(ref msg) = - build_rustfmt_args(opts.message_format, &mut rustfmt_args, cargo_fmt_check) - { + let mut rustfmt_args = opts.rustfmt_options.to_owned(); + if let Err(ref msg) = build_rustfmt_args(&opts, &mut rustfmt_args) { print_usage_to_stderr(msg); return FAILURE; } @@ -145,11 +142,7 @@ fn execute() -> i32 { } } -fn build_rustfmt_args( - message_format: Option, - rustfmt_args: &mut Vec, - check: bool, -) -> Result<(), String> { +fn build_rustfmt_args(opts: &Opts, rustfmt_args: &mut Vec) -> Result<(), String> { let mut contains_check = false; let mut contains_emit_mode = false; let mut contains_list_files = false; @@ -170,7 +163,7 @@ fn build_rustfmt_args( } } - if check && !contains_check { + if opts.check && !contains_check { rustfmt_args.push(String::from("--check")); } @@ -178,7 +171,7 @@ fn build_rustfmt_args( rustfmt_args.push(String::from("--recursive")); } - if let Some(format) = message_format { + if let Some(ref format) = opts.message_format { return match format.as_ref() { "short" => { if !contains_list_files { @@ -818,8 +811,9 @@ mod cargo_fmt_tests { #[test] fn invalid_message_format() { + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "awesome"]); assert_eq!( - build_rustfmt_args(Some(String::from("awesome")), &mut vec![], false), + build_rustfmt_args(&cargo_fmt_opts, &mut vec![]), Err(String::from( "invalid --message-format value: awesome. Allowed values are: short|json|human" )), @@ -828,9 +822,10 @@ mod cargo_fmt_tests { #[test] fn json_message_format_and_check_arg() { - let mut args = vec![String::from("--check")]; + let mut rustfmt_args = vec![String::from("--check")]; + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "json"]); assert_eq!( - build_rustfmt_args(Some(String::from("json")), &mut args, false), + build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args), Err(String::from( "cannot include --check arg when --message-format is set to json" )), @@ -839,9 +834,10 @@ mod cargo_fmt_tests { #[test] fn json_message_format_and_emit_arg() { - let mut args = vec![String::from("--emit"), String::from("checkstyle")]; + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "json"]); + let mut rustfmt_args = vec![String::from("--emit"), String::from("checkstyle")]; assert_eq!( - build_rustfmt_args(Some(String::from("json")), &mut args, false), + build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args), Err(String::from( "cannot include --emit arg when --message-format is set to json" )), @@ -850,14 +846,15 @@ mod cargo_fmt_tests { #[test] fn json_message_format() { - let mut args = vec![ + let mut rustfmt_args = vec![ String::from("--edition"), String::from("2018"), String::from("--recursive"), ]; - assert!(build_rustfmt_args(Some(String::from("json")), &mut args, false).is_ok()); + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "json"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![ String::from("--edition"), String::from("2018"), @@ -875,17 +872,19 @@ mod cargo_fmt_tests { String::from("json"), String::from("--recursive"), ]; - let mut act_args = exp_args.clone(); - assert!(build_rustfmt_args(Some(String::from("human")), &mut act_args, false).is_ok()); - assert_eq!(act_args, exp_args); + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "human"]); + let mut rustfmt_args = exp_args.clone(); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); + assert_eq!(rustfmt_args, exp_args); } #[test] fn short_message_format() { - let mut args = vec![String::from("--check"), String::from("--recursive")]; - assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + let mut rustfmt_args = vec![String::from("--check"), String::from("--recursive")]; + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "short"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![ String::from("--check"), String::from("--recursive"), @@ -896,14 +895,15 @@ mod cargo_fmt_tests { #[test] fn short_message_format_included_short_list_files_flag() { - let mut args = vec![ + let mut rustfmt_args = vec![ String::from("--check"), String::from("-l"), String::from("--recursive"), ]; - assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "short"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![ String::from("--check"), String::from("-l"), @@ -914,14 +914,15 @@ mod cargo_fmt_tests { #[test] fn short_message_format_included_long_list_files_flag() { - let mut args = vec![ + let mut rustfmt_args = vec![ String::from("--check"), String::from("--files-with-diff"), String::from("--recursive"), ]; - assert!(build_rustfmt_args(Some(String::from("short")), &mut args, false).is_ok()); + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "short"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![ String::from("--check"), String::from("--files-with-diff"), @@ -932,60 +933,67 @@ mod cargo_fmt_tests { #[test] fn recursive_shorthand_not_duplicated() { - let mut args = vec![String::from("-r")]; - assert!(build_rustfmt_args(None, &mut args, false).is_ok()); - assert_eq!(args, vec![String::from("-r")]); + let mut rustfmt_args = vec![String::from("-r")]; + let empty: Vec = vec![]; + assert!(build_rustfmt_args(&Opts::from_iter(&empty), &mut rustfmt_args).is_ok()); + assert_eq!(rustfmt_args, vec![String::from("-r")]); } #[test] fn recursive_long_not_duplicated() { - let mut args = vec![String::from("--recursive")]; - assert!(build_rustfmt_args(None, &mut args, false).is_ok()); - assert_eq!(args, vec![String::from("--recursive")]); + let mut rustfmt_args = vec![String::from("--recursive")]; + let empty: Vec = vec![]; + assert!(build_rustfmt_args(&Opts::from_iter(&empty), &mut rustfmt_args).is_ok()); + assert_eq!(rustfmt_args, vec![String::from("--recursive")]); } #[test] fn recursive_added() { - let mut args = vec![]; - assert!(build_rustfmt_args(None, &mut args, false).is_ok()); - assert_eq!(args, vec![String::from("--recursive")]); + let mut rustfmt_args = vec![]; + let empty: Vec = vec![]; + assert!(build_rustfmt_args(&Opts::from_iter(&empty), &mut rustfmt_args).is_ok()); + assert_eq!(rustfmt_args, vec![String::from("--recursive")]); } #[test] fn check_not_duplicated_when_included_in_cargo_fmt() { - let mut args = vec![String::from("--check"), String::from("--recursive")]; - assert!(build_rustfmt_args(None, &mut args, true).is_ok()); + let mut rustfmt_args = vec![String::from("--check"), String::from("--recursive")]; + let cargo_fmt_opts = Opts::from_iter(&["test", "--check"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![String::from("--check"), String::from("--recursive")], ); } #[test] fn check_still_passed_through_when_not_included_in_cargo_fmt() { - let mut args = vec![String::from("--check"), String::from("--recursive")]; - assert!(build_rustfmt_args(None, &mut args, false).is_ok()); + let mut rustfmt_args = vec![String::from("--check"), String::from("--recursive")]; + let empty: Vec = vec![]; + assert!(build_rustfmt_args(&Opts::from_iter(&empty), &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![String::from("--check"), String::from("--recursive")], ); } #[test] fn check_added() { - let mut args = vec![String::from("--recursive")]; - assert!(build_rustfmt_args(None, &mut args, true).is_ok()); + let mut rustfmt_args = vec![String::from("--recursive")]; + let cargo_fmt_opts = Opts::from_iter(&["test", "--check"]); + assert!(build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args).is_ok()); assert_eq!( - args, + rustfmt_args, vec![String::from("--recursive"), String::from("--check")], ); } #[test] - fn check_not_added() { - let mut args = vec![String::from("--recursive")]; - assert!(build_rustfmt_args(None, &mut args, false).is_ok()); - assert_eq!(args, vec![String::from("--recursive")]); + fn check_not_added_when_flag_disabled() { + let mut rustfmt_args = vec![String::from("--recursive")]; + let empty: Vec = vec![]; + assert!(build_rustfmt_args(&Opts::from_iter(&empty), &mut rustfmt_args).is_ok()); + assert_eq!(rustfmt_args, vec![String::from("--recursive")]); } } From 01828f0465145bafabfb3e9c9daf9e5dde281851 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Mon, 25 Nov 2019 19:01:01 -0600 Subject: [PATCH 3/4] refactor: update description for recursive mode --- Configurations.md | 2 +- src/bin/main.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Configurations.md b/Configurations.md index 36c264f86f3..35d7c526061 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1722,7 +1722,7 @@ fn example() { ## `recursive` -Format all encountered modules recursively, including those defined in external files. +Format all encountered modules recursively regardless of whether the modules are defined inline or in another file - **Default value**: `false` - **Possible values**: `true`, `false` diff --git a/src/bin/main.rs b/src/bin/main.rs index 47c746f855b..67a781b153a 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -173,7 +173,8 @@ fn make_opts() -> Options { opts.optflag( "r", "recursive", - "Format all encountered modules recursively, including those defined in external files.", + "Format all encountered modules recursively regardless of whether the modules\ + are defined inline or in another file", ); opts.optflag("v", "verbose", "Print verbose output"); opts.optflag("q", "quiet", "Print less output"); From bfd111466b5d2ad6938fb51d86cc5c38c8e5d8d6 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Wed, 27 Nov 2019 23:19:51 -0500 Subject: [PATCH 4/4] refactor: make recursive config optional internal only --- Configurations.md | 11 +++-------- src/config/config_type.rs | 4 ++-- src/config/mod.rs | 6 +++--- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Configurations.md b/Configurations.md index 35d7c526061..01d5cdd033b 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1720,14 +1720,6 @@ fn example() { } ``` -## `recursive` - -Format all encountered modules recursively regardless of whether the modules are defined inline or in another file - -- **Default value**: `false` -- **Possible values**: `true`, `false` -- **Stable**: Yes - ## `remove_nested_parens` Remove nested parens. @@ -2453,3 +2445,6 @@ Internal option ## `print_misformatted_file_names` Internal option, use `-l` or `--files-with-diff` + +## `recursive` +Internal option, use `-r` or `--recursive` diff --git a/src/config/config_type.rs b/src/config/config_type.rs index b8f0691a7b9..618de16bc15 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -248,8 +248,8 @@ macro_rules! create_config { #[allow(unreachable_pub)] pub fn is_hidden_option(name: &str) -> bool { - const HIDE_OPTIONS: [&str; 4] = - ["verbose", "verbose_diff", "file_lines", "width_heuristics"]; + const HIDE_OPTIONS: [&str; 6] = + ["verbose", "verbose_diff", "file_lines", "width_heuristics", "recursive", "print_misformatted_file_names"]; HIDE_OPTIONS.contains(&name) } diff --git a/src/config/mod.rs b/src/config/mod.rs index ccff9eaa2ce..c05a46a7c37 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -130,8 +130,6 @@ create_config! { "Enables unstable features. Only available on nightly channel"; disable_all_formatting: bool, false, false, "Don't reformat anything"; skip_children: bool, false, false, "Don't reformat out of line modules"; - recursive: bool, false, true, - "Format all encountered modules recursively, including those defined in external files."; hide_parse_errors: bool, false, false, "Hide errors from the parser"; error_on_line_overflow: bool, false, false, "Error if unable to get all lines within max_width"; error_on_unformatted: bool, false, false, @@ -156,6 +154,8 @@ create_config! { print_misformatted_file_names: bool, false, true, "Prints the names of mismatched files that were formatted. Prints the names of \ files that would be formated when used with `--check` mode. "; + recursive: bool, false, true, + "Format all encountered modules recursively, including those defined in external files."; } impl PartialConfig { @@ -166,6 +166,7 @@ impl PartialConfig { cloned.verbose = None; cloned.width_heuristics = None; cloned.print_misformatted_file_names = None; + cloned.recursive = None; ::toml::to_string(&cloned).map_err(|e| format!("Could not output config: {}", e)) } @@ -573,7 +574,6 @@ required_version = "{}" unstable_features = false disable_all_formatting = false skip_children = false -recursive = false hide_parse_errors = false error_on_line_overflow = false error_on_unformatted = false