diff --git a/Configurations.md b/Configurations.md index a13fe062d89..01d5cdd033b 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2445,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/bin/main.rs b/src/bin/main.rs index 75102aa0673..67a781b153a 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -170,7 +170,12 @@ fn make_opts() -> Options { "Don't reformat child modules (unstable).", ); } - + opts.optflag( + "r", + "recursive", + "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"); opts.optflag("V", "version", "Show version information"); @@ -500,6 +505,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 +574,16 @@ impl GetOptsOptions { } } + if matches.opt_present("recursive") { + 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); + } + options.config_path = matches.opt_str("config-path").map(PathBuf::from); options.inline_config = matches @@ -658,6 +674,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..7c7e600b7d8 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -111,21 +111,11 @@ 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); - } - } - 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 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; } - let include_nested_test_files = opts.include_nested_test_files; if let Some(specified_manifest_path) = opts.manifest_path { @@ -152,13 +142,12 @@ fn execute() -> i32 { } } -fn convert_message_format_to_rustfmt_args( - message_format: &str, - rustfmt_args: &mut Vec, -) -> Result<(), String> { - let mut contains_emit_mode = false; +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; + let mut contains_recursive = false; + for arg in rustfmt_args.iter() { if arg.starts_with("--emit") { contains_emit_mode = true; @@ -169,37 +158,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 opts.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(ref format) = opts.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 +806,14 @@ mod cargo_fmt_tests { ); } - mod convert_message_format_to_rustfmt_args_tests { + mod build_rustfmt_args_tests { use super::*; #[test] fn invalid_message_format() { + let cargo_fmt_opts = Opts::from_iter(&["test", "--message-format", "awesome"]); assert_eq!( - convert_message_format_to_rustfmt_args("awesome", &mut vec![]), + build_rustfmt_args(&cargo_fmt_opts, &mut vec![]), Err(String::from( "invalid --message-format value: awesome. Allowed values are: short|json|human" )), @@ -816,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!( - convert_message_format_to_rustfmt_args("json", &mut args), + build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args), Err(String::from( "cannot include --check arg when --message-format is set to json" )), @@ -827,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!( - convert_message_format_to_rustfmt_args("json", &mut args), + build_rustfmt_args(&cargo_fmt_opts, &mut rustfmt_args), Err(String::from( "cannot include --emit arg when --message-format is set to json" )), @@ -838,50 +846,155 @@ 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 rustfmt_args = vec![ + String::from("--edition"), + String::from("2018"), + String::from("--recursive"), + ]; + 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"), + 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 mut act_args = exp_args.clone(); - assert!(convert_message_format_to_rustfmt_args("human", &mut act_args).is_ok()); - assert_eq!(act_args, exp_args); + let exp_args = vec![ + String::from("--emit"), + String::from("json"), + String::from("--recursive"), + ]; + 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")]; - assert!(convert_message_format_to_rustfmt_args("short", &mut args).is_ok()); - assert_eq!(args, vec![String::from("--check"), String::from("-l")]); + 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!( + rustfmt_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 rustfmt_args = vec![ + String::from("--check"), + String::from("-l"), + 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!( + rustfmt_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 rustfmt_args = vec![ + String::from("--check"), + String::from("--files-with-diff"), + 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!( + rustfmt_args, + vec![ + String::from("--check"), + String::from("--files-with-diff"), + String::from("--recursive"), + ] + ); + } + + #[test] + fn recursive_shorthand_not_duplicated() { + 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 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 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 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!( + rustfmt_args, + vec![String::from("--check"), String::from("--recursive")], + ); + } + + #[test] + fn check_still_passed_through_when_not_included_in_cargo_fmt() { + 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, - vec![String::from("--check"), String::from("--files-with-diff")] + rustfmt_args, + vec![String::from("--check"), String::from("--recursive")], ); } + + #[test] + fn check_added() { + 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!( + rustfmt_args, + vec![String::from("--recursive"), String::from("--check")], + ); + } + + #[test] + 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")]); + } } mod get_nested_integration_test_files_tests { 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 698a3224591..c05a46a7c37 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -154,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 { @@ -164,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)) } @@ -279,7 +282,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 +494,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!( 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"); +}