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"); +}