Stricter test directives

Contributing.md

@@ -77,13 +77,13 @@ Without explicit settings, the tests will be run using rustfmt's default
 configuration. It is possible to run a test using non-default settings in several
 ways. Firstly, you can include configuration parameters in comments at the top
 of the file. For example: to use 3 spaces per tab, start your test with
-`// rustfmt-tab_spaces: 3`. Just remember that the comment is part of the input,
+`//@ rustfmt-tab_spaces: 3`. Just remember that the comment is part of the input,
 so include in both the source and target files! It is also possible to
 explicitly specify the name of the expected output file in the target directory.
-Use `// rustfmt-target: filename.rs` for this. You can also specify a custom
+Use `//@ rustfmt-target: filename.rs` for this. You can also specify a custom
 configuration by using the `rustfmt-config` directive. Rustfmt will then use
 that toml file located in `./tests/config/` for its configuration. Including
-`// rustfmt-config: small_tabs.toml` will run your test with the configuration
+`//@ rustfmt-config: small_tabs.toml` will run your test with the configuration
 file found at `./tests/config/small_tabs.toml`. The final option is used when the
 test source file contains no configuration parameter comments. In this case, the
 test harness looks for a configuration file with the same filename as the test

src/config/config_type.rs

@@ -244,6 +244,18 @@ macro_rules! create_config {
                 CliConfigWasSet(self)
             }
 
+            #[allow(unreachable_pub)]
+            pub fn was_option_set(&self, key: &str) -> bool {
+                match key {
+                    $(
+                        stringify!($i) => {
+                            ConfigWasSet(self).$i()
+                        }
+                    )+
+                    _ => false
+                }
+            }
+
             fn fill_from_parsed_config(mut self, parsed: PartialConfig, dir: &Path) -> Config {
             $(
                 if let Some(option_value) = parsed.$i {

src/test/mod.rs

@@ -127,7 +127,7 @@ fn verify_config_used(path: &Path, config_name: &str) {
         let entry = entry.expect("couldn't get directory entry");
         let path = entry.path();
         if path.extension().map_or(false, |f| f == "rs") {
-            // check if "// rustfmt-<config_name>:" appears in the file.
+            // check if "//@ rustfmt-<config_name>:" appears in the file.
             let filebuf = BufReader::new(
                 fs::File::open(&path)
                     .unwrap_or_else(|_| panic!("couldn't read file {}", path.display())),
@@ -137,7 +137,7 @@ fn verify_config_used(path: &Path, config_name: &str) {
                     .lines()
                     .map(Result::unwrap)
                     .take_while(|l| l.starts_with("//"))
-                    .any(|l| l.starts_with(&format!("// rustfmt-{}", config_name))),
+                    .any(|l| l.starts_with(&format!("//@ rustfmt-{}", config_name))),
                 "config option file {} does not contain expected config name",
                 path.display()
             );
@@ -258,7 +258,7 @@ fn warning_tests() {
     let files = get_test_files(Path::new("tests/warning/source"), true);
 
     for file in &files {
-        let mut config = read_config(file);
+        let mut config = read_config(file).expect("config is valid");
         config.set().error_on_line_overflow(true);
         config.set().error_on_unformatted(true);
 
@@ -379,7 +379,7 @@ fn modified_test() {
 // Helper function for comparing the results of rustfmt
 // to a known output file generated by one of the write modes.
 fn assert_output(source: &Path, expected_filename: &Path) {
-    let config = read_config(source);
+    let config = read_config(source).expect("config is valid");
     let (_, source_file, _) = format_file(source, config.clone());
 
     // Populate output by writing to a vec.
@@ -790,7 +790,8 @@ fn check_files(files: Vec<PathBuf>, opt_config: &Option<PathBuf>) -> (Vec<Format
     let mut reports = vec![];
 
     for file_name in files {
-        let sig_comments = read_significant_comments(&file_name);
+        let sig_comments =
+            read_significant_comments(&file_name).expect("valid significant comments");
         if sig_comments.contains_key("unstable") && !is_nightly_channel!() {
             debug!(
                 "Skipping '{}' because it requires unstable \
@@ -856,8 +857,8 @@ fn print_mismatches<T: Fn(u32) -> String>(
     }
 }
 
-fn read_config(filename: &Path) -> Config {
-    let sig_comments = read_significant_comments(filename);
+fn read_config(filename: &Path) -> Result<Config, String> {
+    let sig_comments = read_significant_comments(filename)?;
     let (edition, style_edition, version) = get_editions_from_comments(&sig_comments);
     // Look for a config file. If there is a 'config' property in the significant comments, use
     // that. Otherwise, if there are no significant comments at all, look for a config file with
@@ -880,11 +881,40 @@ fn read_config(filename: &Path) -> Config {
 
     for (key, val) in &sig_comments {
         if key != "target" && key != "config" && key != "unstable" && key != "stable" {
+            if !Config::is_valid_name(key) {
+                // Check that all config names are valid
+                return Err(format!(
+                    "{} set unrecognized config option {:?}",
+                    &filename.display(),
+                    key
+                ));
+            }
+
+            if !Config::is_valid_key_val(key, val) {
+                // Check that all config values are valid
+                return Err(format!(
+                    "{} set unrecognized value {:?} for config option {:?}",
+                    &filename.display(),
+                    key,
+                    val
+                ));
+            }
+
             config.override_value(key, val);
+
+            if !config.was_option_set(key) {
+                // Check that all config values were properly set
+                return Err(format!(
+                    "{} didn't correctly set {}: {}",
+                    &filename.display(),
+                    key,
+                    val
+                ));
+            }
         }
     }
 
-    config
+    Ok(config)
 }
 
 fn format_file<P: Into<PathBuf>>(filepath: P, config: Config) -> (bool, SourceFile, FormatReport) {
@@ -923,13 +953,13 @@ fn idempotent_check(
     filename: &PathBuf,
     opt_config: &Option<PathBuf>,
 ) -> Result<FormatReport, IdempotentCheckError> {
-    let sig_comments = read_significant_comments(filename);
+    let sig_comments = read_significant_comments(filename).expect("valid significant comments");
     let config = if let Some(ref config_file_path) = opt_config {
         let (edition, style_edition, version) = get_editions_from_comments(&sig_comments);
         Config::from_toml_path(config_file_path, edition, style_edition, version)
             .expect("`rustfmt.toml` not found")
     } else {
-        read_config(filename)
+        read_config(filename).expect("config is valid")
     };
     let (parsing_errors, source_file, format_report) = format_file(filename, config);
     if parsing_errors {
@@ -985,37 +1015,51 @@ fn get_config(
     .expect("invalid TOML")
 }
 
-// Reads significant comments of the form: `// rustfmt-key: value` into a hash map.
-fn read_significant_comments(file_name: &Path) -> HashMap<String, String> {
+// Reads significant comments of the form: `//@ rustfmt-key: value` into a hash map.
+fn read_significant_comments(file_name: &Path) -> Result<HashMap<String, String>, String> {
     let file = fs::File::open(file_name)
         .unwrap_or_else(|_| panic!("couldn't read file {}", file_name.display()));
     let reader = BufReader::new(file);
-    let pattern = r"^\s*//\s*rustfmt-([^:]+):\s*(\S+)";
-    let regex = regex::Regex::new(pattern).expect("failed creating pattern 1");
 
-    // Matches lines containing significant comments or whitespace.
-    let line_regex = regex::Regex::new(r"(^\s*$)|(^\s*//\s*rustfmt-[^:]+:\s*\S+)")
-        .expect("failed creating pattern 2");
+    // Matches `// rustfmt-{name}: {value}`
+    let old_pattern = r"^\s*//\s+rustfmt-(?P<name>[A-Za-z_]+)\s?:\s*(?P<value>\S+)$";
+    let old_regex = regex::Regex::new(old_pattern).expect("failed creating old pattern");
+
+    // Matches `//@ rustfmt-{name}: {value}`
+    let pattern = r"^\s*//@\s+rustfmt-(?P<name>[A-Za-z_]+)\s?:\s*(?P<value>\S+)$";
+    let regex = regex::Regex::new(pattern).expect("failed creating pattern 1");
 
     reader
         .lines()
         .map(|line| line.expect("failed getting line"))
-        .filter(|line| line_regex.is_match(line))
+        .filter(|l| l.trim_start().starts_with("//"))
         .filter_map(|line| {
-            regex.captures_iter(&line).next().map(|capture| {
-                (
-                    capture
-                        .get(1)
-                        .expect("couldn't unwrap capture")
-                        .as_str()
-                        .to_owned(),
-                    capture
-                        .get(2)
-                        .expect("couldn't unwrap capture")
-                        .as_str()
-                        .to_owned(),
-                )
-            })
+            if let Some(c) = old_regex.captures(&line) {
+                // Don't let old syntax silently slip through.
+                // Guide test authors on how to fix issues with the old syntax.
+                return Some(Err(format!(
+                    "{} uses old directive syntax. Should be //@ rustfmt-{}: {}",
+                    &file_name.display(),
+                    &c["name"],
+                    &c["value"],
+                )));
+            }
+
+            if !line.trim_start().starts_with("//@") {
+                // This is just a normal comment line
+                return None;
+            }
+
+            let value = match regex.captures(&line) {
+                Some(c) => Ok((c["name"].to_owned(), c["value"].to_owned())),
+                None => Err(format!(
+                    "{} has a malformed config directive. Should be //@ rustfmt-{}: {}",
+                    &file_name.display(),
+                    "{name}",
+                    "{value}",
+                )),
+            };
+            Some(value)
         })
         .collect()
 }

src/test/mod_resolver.rs

@@ -7,7 +7,7 @@ use crate::{FileName, Input, Session};
 
 fn verify_mod_resolution(input_file_name: &str, exp_misformatted_files: &[&str]) {
     let input_file = PathBuf::from(input_file_name);
-    let config = read_config(&input_file);
+    let config = read_config(&input_file).expect("config is valid");
     let mut session = Session::<io::Stdout>::new(config, None);
     let report = session
         .format(Input::File(input_file_name.into()))

src/test/parser.rs

@@ -8,7 +8,7 @@ use crate::{ErrorKind, Input, Session};
 
 /// Load the config, but hide expected parse errors
 fn read_config_hide_parse_errors(filename: &std::path::Path) -> crate::Config {
-    let mut config = read_config(&filename);
+    let mut config = read_config(&filename).expect("config is valid");
     config.set().show_parse_errors(false);
     config
 }

tests/coverage/source/comments.rs

@@ -1,4 +1,4 @@
-// rustfmt-emit_mode: coverage
+//@ rustfmt-emit_mode: coverage
 /// Here's a doc comment!
 fn main() {
     // foo is bar

tests/coverage/target/comments.rs

@@ -1,4 +1,4 @@
-XX XXXXXXXXXXXXXXXXXX XXXXXXXX
+XXX XXXXXXXXXXXXXXXXXX XXXXXXXX
 /// Here's a doc comment!
 fn main() {
     XX XXX XX XXX

tests/parser/try_macro_cant_be_chain_child.rs

@@ -1,4 +1,4 @@
-// rustfmt-edition: 2015
+//@ rustfmt-edition: 2015
 // set edition for clarity, since 'try' became a reserved keyword in editions >=2018
 
 fn main() {

tests/source/5131_crate.rs

@@ -1,4 +1,4 @@
-// rustfmt-imports_granularity: Crate
+//@ rustfmt-imports_granularity: Crate
 
 use foo::a;
 use foo::a;

tests/source/5131_module.rs

@@ -1,4 +1,4 @@
-// rustfmt-imports_granularity: Module
+//@ rustfmt-imports_granularity: Module
 
 #![allow(dead_code)]
 

tests/source/5131_one.rs

@@ -1,4 +1,4 @@
-// rustfmt-imports_granularity: One
+//@ rustfmt-imports_granularity: One
 
 pub use foo::x;
 pub use foo::x as x2;

tests/source/alignment_2633/block_style.rs

@@ -1,4 +1,4 @@
-// rustfmt-struct_field_align_threshold: 50
+//@ rustfmt-struct_field_align_threshold: 50
 
 fn func() {
     Ok(ServerInformation { name:         unwrap_message_string(items.get(0)),

tests/source/alignment_2633/visual_style.rs

@@ -1,5 +1,5 @@
-// rustfmt-struct_field_align_threshold: 50
-// rustfmt-indent_style: Visual
+//@ rustfmt-struct_field_align_threshold: 50
+//@ rustfmt-indent_style: Visual
 
 fn func() {
     Ok(ServerInformation { name:         unwrap_message_string(items.get(0)),

tests/source/arrow_in_comments/arrow_in_single_comment.rs

@@ -1,4 +1,4 @@
-// rustfmt-style_edition: 2024
+//@ rustfmt-style_edition: 2024
 fn main() {
     match a {
         _ =>

tests/source/arrow_in_comments/multiple_arrows.rs

@@ -1,4 +1,4 @@
-// rustfmt-style_edition: 2024
+//@ rustfmt-style_edition: 2024
 fn main() {
     match a {
         _ => // comment with => 

tests/source/async_block.rs

@@ -1,4 +1,4 @@
-// rustfmt-edition: 2018
+//@ rustfmt-edition: 2018
 
 fn main() {
     let x = async {

tests/source/async_fn.rs

@@ -1,4 +1,4 @@
-// rustfmt-edition: 2018
+//@ rustfmt-edition: 2018
 
 async fn bar() -> Result<(), ()> {
     Ok(())

tests/source/attrib.rs

@@ -1,4 +1,4 @@
-// rustfmt-wrap_comments: true
+//@ rustfmt-wrap_comments: true
 // Test attributes and doc comments are preserved.
 #![doc(html_logo_url = "https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png",
        html_favicon_url = "https://doc.rust-lang.org/favicon.ico",

tests/source/big-impl-visual.rs

@@ -1,4 +1,4 @@
-// rustfmt-indent_style: Visual
+//@ rustfmt-indent_style: Visual
 
 // #1357
 impl<

tests/source/binop-separator-back/bitwise.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     let value = abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ ^ abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ;

tests/source/binop-separator-back/comp.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     if abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ < abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ {

tests/source/binop-separator-back/logic.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     if abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ && abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ || abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ {

tests/source/binop-separator-back/math.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     let value = abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ;

tests/source/binop-separator-back/patterns.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     match val {

tests/source/binop-separator-back/range.rs

@@ -1,4 +1,4 @@
-// rustfmt-binop_separator: Back
+//@ rustfmt-binop_separator: Back
 
 fn main() {
     let value = abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ..abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ;

tests/source/catch.rs

@@ -1,4 +1,4 @@
-// rustfmt-edition: 2018
+//@ rustfmt-edition: 2018
 #![feature(try_blocks)]
 
 fn main() {

tests/source/cfg_select.rs

@@ -1,6 +1,6 @@
-// rustfmt-unstable: true
-// rustfmt-style_edition: 2024
-// rustfmt-skip_children: true
+//@ rustfmt-unstable: true
+//@ rustfmt-style_edition: 2024
+//@ rustfmt-skip_children: true
 
 // empty cfg_select!
 // Original `{}` delimiters

tests/source/cfg_select_stable.rs

@@ -1,4 +1,4 @@
-// rustfmt-stable: true
+//@ rustfmt-stable: true
 
 // While we gate the `cfg_select!` formatting behind the `is_nightly_channel!()` check
 // this test helps ensure that we don't start formatting `cfg_select!` on the `stable`

tests/source/chain_yield_with_comments.rs

@@ -1,4 +1,4 @@
-// rustfmt-edition: 2024
+//@ rustfmt-edition: 2024
 
 fn f() {
     parent /* comment */ .yield /* some comment */?// comment

tests/source/chains-visual.rs

@@ -1,4 +1,4 @@
-// rustfmt-indent_style: Visual
+//@ rustfmt-indent_style: Visual
 // Test chain formatting.
 
 fn main() {