Use test_ok for inline tests, merge test report (#10801)

## Summary

This PR makes two quality of life improvements for parser's inline test
implementation:
1. Rename `test` to `test_ok` to explicitly specify that this is a valid
syntax test
2. Merge the reporting of "ok" and "err" test cases and unreferenced
test cases

The output of (2) now looks like:

```
---- generate_inline_tests stdout ----
Error: Unreferenced test files found for which no comment exists:
  crates/ruff_python_parser/resources/inline/err/something_else.py
Please delete these files manually

Following files were not up-to date and has been updated:
  crates/ruff_python_parser/resources/inline/err/renamed_it.py
Re-run the tests with `cargo test` to update the test snapshots

```

## Test Plan

1. Add two random test cases:

	```rs
	// test_ok something
	// x = 1
	
	// test_err something_else
	// [1, 2
	```

2. Run the generation step:

	```console
	cargo test --package ruff_python_parser --test generate_inline_tests
	```

3. Rename `something_else` to `renamed_it` and run the generation step
again to see the above output

.gitattributes

@@ -8,5 +8,7 @@ crates/ruff_linter/resources/test/fixtures/pycodestyle/W391_3.py text eol=crlf
 crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples_crlf.py text eol=crlf
 crates/ruff_python_formatter/tests/snapshots/format@docstring_code_examples_crlf.py.snap text eol=crlf
 
+crates/ruff_python_parser/resources/inline linguist-generated=true
+
 ruff.schema.json linguist-generated=true text=auto eol=lf
 *.md.snap linguist-language=Markdown

crates/ruff_python_parser/CONTRIBUTING.md

@@ -13,7 +13,7 @@ Test that the parser successfully parses the input with no syntax errors. They'r
 written in the following format:
 
 ```rs
-// test this_is_the_test_name
+// test_ok this_is_the_test_name
 // def foo():
 //     pass
 println!("some rust code");
@@ -28,14 +28,14 @@ in the following format:
 println!("some rust code");
 ```
 
-Note that the difference between the two is the `test` and `test_err` keywords.
+Note that the difference between the two is the `test_ok` and `test_err` keywords.
 The comment block must be independent of any other comment blocks. For example, the
 following is not extracted:
 
 ```rs
 // Some random comment
 //
-// test this_is_the_test_name
+// test_ok this_is_the_test_name
 // def foo():
 //     pass
 println!("some rust code");

crates/ruff_python_parser/src/parser/expression.rs

@@ -2294,7 +2294,7 @@ impl<'src> Parser<'src> {
         self.bump(TokenKind::Lambda);
 
         let parameters = if self.at(TokenKind::Colon) {
-            // test lambda_with_no_parameters
+            // test_ok lambda_with_no_parameters
             // lambda: 1
             None
         } else {
@@ -2303,7 +2303,7 @@ impl<'src> Parser<'src> {
 
         self.expect(TokenKind::Colon);
 
-        // test lambda_with_valid_body
+        // test_ok lambda_with_valid_body
         // lambda x: x
         // lambda x: x if True else y
         // lambda x: await x

crates/ruff_python_parser/src/parser/mod.rs

@@ -958,7 +958,7 @@ impl RecoveryContextKind {
                     // When the parser is parsing f-string elements inside format spec,
                     // the terminator would be `}`.
 
-                    // test fstring_format_spec_terminator
+                    // test_ok fstring_format_spec_terminator
                     // f"hello {x:} world"
                     // f"hello {x:.3f} world"
                     TokenKind::Rbrace,

crates/ruff_python_parser/src/parser/statement.rs

@@ -456,7 +456,7 @@ impl<'src> Parser<'src> {
             None
         };
 
-        // test from_import_no_space
+        // test_ok from_import_no_space
         // from.import x
         // from...import x
         self.expect(TokenKind::Import);
@@ -580,7 +580,7 @@ impl<'src> Parser<'src> {
             dotted_name.push_str(&self.parse_identifier());
         }
 
-        // test dotted_name_normalized_spaces
+        // test_ok dotted_name_normalized_spaces
         // import a.b.c
         // import a .  b  . c
         ast::Identifier {
@@ -714,7 +714,7 @@ impl<'src> Parser<'src> {
             self.add_error(ParseErrorType::EmptyGlobalNames, self.current_token_range());
         }
 
-        // test global_stmt
+        // test_ok global_stmt
         // global x
         // global x, y, z
         ast::StmtGlobal {
@@ -755,7 +755,7 @@ impl<'src> Parser<'src> {
             );
         }
 
-        // test nonlocal_stmt
+        // test_ok nonlocal_stmt
         // nonlocal x
         // nonlocal x, y, z
         ast::StmtNonlocal {

crates/ruff_python_parser/tests/generate_inline_tests.rs

@@ -6,12 +6,12 @@
 //! - <https://github.com/rust-lang/rust-analyzer/blob/e4a405f877efd820bef9c0e77a02494e47c17512/crates/parser/src/tests/sourcegen_inline_tests.rs>
 //! - <https://github.com/biomejs/biome/blob/b9f8ffea9967b098ec4c8bf74fa96826a879f043/xtask/codegen/src/parser_tests.rs>
 use std::collections::HashMap;
+use std::fmt;
 use std::fs;
-use std::ops::{Deref, DerefMut};
+use std::ops::{AddAssign, Deref, DerefMut};
 use std::path::{Path, PathBuf};
 
 use anyhow::{Context, Result};
-use itertools::Itertools;
 
 fn project_root() -> PathBuf {
     PathBuf::from(env!("CARGO_MANIFEST_DIR"))
@@ -25,13 +25,80 @@ fn generate_inline_tests() -> Result<()> {
     let parser_dir = project_root().join("crates/ruff_python_parser/src/parser");
     let tests = TestCollection::try_from(parser_dir.as_path())?;
 
-    install_tests(&tests.ok, "crates/ruff_python_parser/resources/inline/ok")?;
-    install_tests(&tests.err, "crates/ruff_python_parser/resources/inline/err")?;
+    let mut test_files = TestFiles::default();
+    test_files += install_tests(&tests.ok, "crates/ruff_python_parser/resources/inline/ok")?;
+    test_files += install_tests(&tests.err, "crates/ruff_python_parser/resources/inline/err")?;
+
+    if !test_files.is_empty() {
+        anyhow::bail!("{}", test_files);
+    }
 
     Ok(())
 }
 
-fn install_tests(tests: &HashMap<String, Test>, target_dir: &str) -> Result<()> {
+#[derive(Debug, Default)]
+struct TestFiles {
+    unreferenced: Vec<PathBuf>,
+    updated: Vec<PathBuf>,
+}
+
+impl TestFiles {
+    fn is_empty(&self) -> bool {
+        self.unreferenced.is_empty() && self.updated.is_empty()
+    }
+}
+
+impl AddAssign<TestFiles> for TestFiles {
+    fn add_assign(&mut self, other: TestFiles) {
+        self.unreferenced.extend(other.unreferenced);
+        self.updated.extend(other.updated);
+    }
+}
+
+impl fmt::Display for TestFiles {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if self.is_empty() {
+            writeln!(f, "No unreferenced or updated test files found")
+        } else {
+            let root_dir = project_root();
+            if !self.unreferenced.is_empty() {
+                writeln!(
+                    f,
+                    "Unreferenced test files found for which no comment exists:",
+                )?;
+                for path in &self.unreferenced {
+                    writeln!(f, "  {}", path.strip_prefix(&root_dir).unwrap().display())?;
+                }
+                writeln!(f, "Please delete these files manually")?;
+            }
+            if !self.updated.is_empty() {
+                if !self.unreferenced.is_empty() {
+                    writeln!(f)?;
+                }
+                writeln!(
+                    f,
+                    "Following files were not up-to date and has been updated:",
+                )?;
+                for path in &self.updated {
+                    writeln!(f, "  {}", path.strip_prefix(&root_dir).unwrap().display())?;
+                }
+                writeln!(
+                    f,
+                    "Re-run the tests with `cargo test` to update the test snapshots"
+                )?;
+                if std::env::var("CI").is_ok() {
+                    writeln!(
+                        f,
+                        "NOTE: Run the tests locally and commit the updated files"
+                    )?;
+                }
+            }
+            Ok(())
+        }
+    }
+}
+
+fn install_tests(tests: &HashMap<String, Test>, target_dir: &str) -> Result<TestFiles> {
     let root_dir = project_root();
     let tests_dir = root_dir.join(target_dir);
     if !tests_dir.is_dir() {
@@ -41,22 +108,6 @@ fn install_tests(tests: &HashMap<String, Test>, target_dir: &str) -> Result<()>
     // Test kind is irrelevant for existing test cases.
     let existing = existing_tests(&tests_dir)?;
 
-    let unreferenced = existing
-        .iter()
-        .filter(|&(name, _)| !tests.contains_key(name))
-        .map(|(_, path)| path)
-        .collect::<Vec<_>>();
-
-    if !unreferenced.is_empty() {
-        anyhow::bail!(
-            "Unreferenced test files found for which no comment exists:\n  {}\nPlease delete these files manually\n",
-            unreferenced
-                .iter()
-                .map(|path| path.strip_prefix(&root_dir).unwrap().display())
-                .join("\n  ")
-        );
-    }
-
     let mut updated_files = vec![];
 
     for (name, test) in tests {
@@ -73,21 +124,14 @@ fn install_tests(tests: &HashMap<String, Test>, target_dir: &str) -> Result<()>
         updated_files.push(path);
     }
 
-    if !updated_files.is_empty() {
-        let mut message = format!(
-            "Following files were not up-to date and has been updated:\n  {}\nRe-run the tests with `cargo test` to update the test snapshots\n",
-            updated_files
-                .iter()
-                .map(|path| path.strip_prefix(&root_dir).unwrap().display())
-                .join("\n  ")
-        );
-        if std::env::var("CI").is_ok() {
-            message.push_str("NOTE: Run the tests locally and commit the updated files\n");
-        }
-        anyhow::bail!(message);
-    }
-
-    Ok(())
+    Ok(TestFiles {
+        unreferenced: existing
+            .into_iter()
+            .filter(|(name, _)| !tests.contains_key(name))
+            .map(|(_, path)| path)
+            .collect::<Vec<_>>(),
+        updated: updated_files,
+    })
 }
 
 #[derive(Default, Debug)]
@@ -115,7 +159,7 @@ impl TryFrom<&Path> for TestCollection {
                 if test.is_ok() {
                     if let Some(old_test) = tests.ok.insert(test.name.clone(), test) {
                         anyhow::bail!(
-                            "Duplicate test found: {name:?} (search '// test {name}' for the location)\n",
+                            "Duplicate test found: {name:?} (search '// test_ok {name}' for the location)\n",
                             name = old_test.name
                         );
                     }
@@ -141,7 +185,7 @@ enum TestKind {
 /// A test of the following form:
 ///
 /// ```text
-/// // (test|test_err) name
+/// // (test_ok|test_err) name
 /// // <code>
 /// ```
 #[derive(Debug)]
@@ -165,7 +209,7 @@ fn collect_tests(text: &str) -> Vec<Test> {
         let first_line = &comment_block[0];
 
         let (kind, name) = match first_line.split_once(' ') {
-            Some(("test", suffix)) => (TestKind::Ok, suffix),
+            Some(("test_ok", suffix)) => (TestKind::Ok, suffix),
             Some(("test_err", suffix)) => (TestKind::Err, suffix),
             _ => continue,
         };