Migrate the error_codes check to diagnostics

Author: Kobzol

src/tools/tidy/src/error_codes.rs

@@ -22,6 +22,7 @@ use std::path::Path;
 
 use regex::Regex;
 
+use crate::diagnostics::{DiagCtx, RunningCheck};
 use crate::walk::{filter_dirs, walk, walk_many};
 
 const ERROR_CODES_PATH: &str = "compiler/rustc_error_codes/src/lib.rs";
@@ -35,71 +36,50 @@ const IGNORE_DOCTEST_CHECK: &[&str] = &["E0464", "E0570", "E0601", "E0602", "E07
 const IGNORE_UI_TEST_CHECK: &[&str] =
     &["E0461", "E0465", "E0514", "E0554", "E0640", "E0717", "E0729"];
 
-macro_rules! verbose_print {
-    ($verbose:expr, $($fmt:tt)*) => {
-        if $verbose {
-            println!("{}", format_args!($($fmt)*));
-        }
-    };
-}
-
-pub fn check(
-    root_path: &Path,
-    search_paths: &[&Path],
-    verbose: bool,
-    ci_info: &crate::CiInfo,
-    bad: &mut bool,
-) {
-    let mut errors = Vec::new();
+pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, diag_ctx: DiagCtx) {
+    let mut check = diag_ctx.start_check("error_codes");
 
     // Check that no error code explanation was removed.
-    check_removed_error_code_explanation(ci_info, bad);
+    check_removed_error_code_explanation(ci_info, &mut check);
 
     // Stage 1: create list
-    let error_codes = extract_error_codes(root_path, &mut errors);
-    if verbose {
-        println!("Found {} error codes", error_codes.len());
-        println!("Highest error code: `{}`", error_codes.iter().max().unwrap());
-    }
+    let error_codes = extract_error_codes(root_path, &mut check);
+    check.verbose_msg(format!("Found {} error codes", error_codes.len()));
+    check.verbose_msg(format!("Highest error code: `{}`", error_codes.iter().max().unwrap()));
 
     // Stage 2: check list has docs
-    let no_longer_emitted = check_error_codes_docs(root_path, &error_codes, &mut errors, verbose);
+    let no_longer_emitted = check_error_codes_docs(root_path, &error_codes, &mut check);
 
     // Stage 3: check list has UI tests
-    check_error_codes_tests(root_path, &error_codes, &mut errors, verbose, &no_longer_emitted);
+    check_error_codes_tests(root_path, &error_codes, &mut check, &no_longer_emitted);
 
     // Stage 4: check list is emitted by compiler
-    check_error_codes_used(search_paths, &error_codes, &mut errors, &no_longer_emitted, verbose);
-
-    // Print any errors.
-    for error in errors {
-        tidy_error!(bad, "{}", error);
-    }
+    check_error_codes_used(search_paths, &error_codes, &mut check, &no_longer_emitted);
 }
 
-fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, bad: &mut bool) {
+fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, check: &mut RunningCheck) {
     let Some(base_commit) = &ci_info.base_commit else {
-        eprintln!("Skipping error code explanation removal check");
+        check.verbose_msg("Skipping error code explanation removal check");
         return;
     };
     let Some(diff) = crate::git_diff(base_commit, "--name-status") else {
-        *bad = true;
-        eprintln!("removed error code explanation tidy check: Failed to run git diff");
+        check.error(format!("removed error code explanation: Failed to run git diff"));
         return;
     };
     if diff.lines().any(|line| {
         line.starts_with('D') && line.contains("compiler/rustc_error_codes/src/error_codes/")
     }) {
-        *bad = true;
-        eprintln!("tidy check error: Error code explanations should never be removed!");
-        eprintln!("Take a look at E0001 to see how to handle it.");
+        check.error(format!(
+            r#"Error code explanations should never be removed!
+Take a look at E0001 to see how to handle it."#
+        ));
         return;
     }
-    println!("No error code explanation was removed!");
+    check.verbose_msg("No error code explanation was removed!");
 }
 
 /// Stage 1: Parses a list of error codes from `error_codes.rs`.
-fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String> {
+fn extract_error_codes(root_path: &Path, check: &mut RunningCheck) -> Vec<String> {
     let path = root_path.join(Path::new(ERROR_CODES_PATH));
     let file =
         fs::read_to_string(&path).unwrap_or_else(|e| panic!("failed to read `{path:?}`: {e}"));
@@ -117,7 +97,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
             // Extract the error code from the line. Emit a fatal error if it is not in the correct
             // format.
             let Some(split_line) = split_line else {
-                errors.push(format!(
+                check.error(format!(
                     "{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
                     but got \"{line}\" without a `:` delimiter",
                 ));
@@ -128,8 +108,9 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
 
             // If this is a duplicate of another error code, emit a fatal error.
             if error_codes.contains(&err_code) {
-                errors
-                    .push(format!("{path}:{line_index}: Found duplicate error code: `{err_code}`"));
+                check.error(format!(
+                    "{path}:{line_index}: Found duplicate error code: `{err_code}`"
+                ));
                 continue;
             }
 
@@ -140,14 +121,14 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
             // Ensure that the line references the correct markdown file.
             let rest = split_line.1.split_once(',');
             let Some(rest) = rest else {
-                errors.push(format!(
+                check.error(format!(
                     "{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
                     but got \"{line}\" without a `,` delimiter",
                 ));
                 continue;
             };
             if error_num_as_str != rest.0.trim() {
-                errors.push(format!(
+                check.error(format!(
                     "{path}:{line_index}: `{}:` should be followed by `{},` but instead found `{}` in \
                     `compiler/rustc_error_codes/src/lib.rs`",
                     err_code,
@@ -157,7 +138,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
                 continue;
             }
             if !rest.1.trim().is_empty() && !rest.1.trim().starts_with("//") {
-                errors.push(format!("{path}:{line_index}: should only have one error per line"));
+                check.error(format!("{path}:{line_index}: should only have one error per line"));
                 continue;
             }
 
@@ -172,8 +153,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
 fn check_error_codes_docs(
     root_path: &Path,
     error_codes: &[String],
-    errors: &mut Vec<String>,
-    verbose: bool,
+    check: &mut RunningCheck,
 ) -> Vec<String> {
     let docs_path = root_path.join(Path::new(ERROR_DOCS_PATH));
 
@@ -184,7 +164,7 @@ fn check_error_codes_docs(
 
         // Error if the file isn't markdown.
         if path.extension() != Some(OsStr::new("md")) {
-            errors.push(format!(
+            check.error(format!(
                 "Found unexpected non-markdown file in error code docs directory: {}",
                 path.display()
             ));
@@ -196,7 +176,7 @@ fn check_error_codes_docs(
         let err_code = filename.unwrap().0; // `unwrap` is ok because we know the filename is in the correct format.
 
         if error_codes.iter().all(|e| e != err_code) {
-            errors.push(format!(
+            check.error(format!(
                 "Found valid file `{}` in error code docs directory without corresponding \
                 entry in `rustc_error_codes/src/lib.rs`",
                 path.display()
@@ -208,36 +188,34 @@ fn check_error_codes_docs(
             check_explanation_has_doctest(contents, err_code);
 
         if emit_ignore_warning {
-            verbose_print!(
-                verbose,
+            check.verbose_msg(format!(
                 "warning: Error code `{err_code}` uses the ignore header. This should not be used, add the error code to the \
                 `IGNORE_DOCTEST_CHECK` constant instead."
-            );
+            ));
         }
 
         if no_longer_emitted {
             no_longer_emitted_codes.push(err_code.to_owned());
         }
 
         if !found_code_example {
-            verbose_print!(
-                verbose,
+            check.verbose_msg(format!(
                 "warning: Error code `{err_code}` doesn't have a code example, all error codes are expected to have one \
                 (even if untested)."
-            );
+            ));
             return;
         }
 
         let test_ignored = IGNORE_DOCTEST_CHECK.contains(&err_code);
 
         // Check that the explanation has a doctest, and if it shouldn't, that it doesn't
         if !found_proper_doctest && !test_ignored {
-            errors.push(format!(
+            check.error(format!(
                 "`{}` doesn't use its own error code in compile_fail example",
                 path.display(),
             ));
         } else if found_proper_doctest && test_ignored {
-            errors.push(format!(
+            check.error(format!(
                 "`{}` has a compile_fail doctest with its own error code, it shouldn't \
                 be listed in `IGNORE_DOCTEST_CHECK`",
                 path.display(),
@@ -289,8 +267,7 @@ fn check_explanation_has_doctest(explanation: &str, err_code: &str) -> (bool, bo
 fn check_error_codes_tests(
     root_path: &Path,
     error_codes: &[String],
-    errors: &mut Vec<String>,
-    verbose: bool,
+    check: &mut RunningCheck,
     no_longer_emitted: &[String],
 ) {
     let tests_path = root_path.join(Path::new(ERROR_TESTS_PATH));
@@ -299,15 +276,14 @@ fn check_error_codes_tests(
         let test_path = tests_path.join(format!("{code}.stderr"));
 
         if !test_path.exists() && !IGNORE_UI_TEST_CHECK.contains(&code.as_str()) {
-            verbose_print!(
-                verbose,
+            check.verbose_msg(format!(
                 "warning: Error code `{code}` needs to have at least one UI test in the `tests/error-codes/` directory`!"
-            );
+            ));
             continue;
         }
         if IGNORE_UI_TEST_CHECK.contains(&code.as_str()) {
             if test_path.exists() {
-                errors.push(format!(
+                check.error(format!(
                     "Error code `{code}` has a UI test in `tests/ui/error-codes/{code}.rs`, it shouldn't be listed in `EXEMPTED_FROM_TEST`!"
                 ));
             }
@@ -317,11 +293,10 @@ fn check_error_codes_tests(
         let file = match fs::read_to_string(&test_path) {
             Ok(file) => file,
             Err(err) => {
-                verbose_print!(
-                    verbose,
+                check.verbose_msg(format!(
                     "warning: Failed to read UI test file (`{}`) for `{code}` but the file exists. The test is assumed to work:\n{err}",
                     test_path.display()
-                );
+                ));
                 continue;
             }
         };
@@ -343,10 +318,9 @@ fn check_error_codes_tests(
         }
 
         if !found_code {
-            verbose_print!(
-                verbose,
+            check.verbose_msg(format!(
                 "warning: Error code `{code}` has a UI test file, but doesn't contain its own error code!"
-            );
+            ));
         }
     }
 }
@@ -355,9 +329,8 @@ fn check_error_codes_tests(
 fn check_error_codes_used(
     search_paths: &[&Path],
     error_codes: &[String],
-    errors: &mut Vec<String>,
+    check: &mut RunningCheck,
     no_longer_emitted: &[String],
-    verbose: bool,
 ) {
     // Search for error codes in the form `E0123`.
     let regex = Regex::new(r#"\bE\d{4}\b"#).unwrap();
@@ -384,7 +357,7 @@ fn check_error_codes_used(
 
                     if !error_codes.contains(&error_code) {
                         // This error code isn't properly defined, we must error.
-                        errors.push(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`."));
+                        check.error(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`."));
                         continue;
                     }
 
@@ -397,7 +370,7 @@ fn check_error_codes_used(
 
     for code in error_codes {
         if !found_codes.contains(code) && !no_longer_emitted.contains(code) {
-            errors.push(format!(
+            check.error(format!(
                 "Error code `{code}` exists, but is not emitted by the compiler!\n\
                 Please mark the code as no longer emitted by adding the following note to the top of the `EXXXX.md` file:\n\
                 `#### Note: this error code is no longer emitted by the compiler`\n\
@@ -406,10 +379,9 @@ fn check_error_codes_used(
         }
 
         if found_codes.contains(code) && no_longer_emitted.contains(code) {
-            verbose_print!(
-                verbose,
+            check.verbose_msg(format!(
                 "warning: Error code `{code}` is used when it's marked as \"no longer emitted\""
-            );
+            ));
         }
     }
 }

src/tools/tidy/src/main.rs

@@ -118,15 +118,15 @@ fn main() {
         check!(unknown_revision, &tests_path);
 
         // Checks that only make sense for the compiler.
-        // check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info);
+        check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], &ci_info);
         // check!(fluent_alphabetical, &compiler_path, bless);
         // check!(fluent_period, &compiler_path);
         // check!(fluent_lowercase, &compiler_path);
         // check!(target_policy, &root_path);
         // check!(gcc_submodule, &root_path, &compiler_path);
 
         // Checks that only make sense for the std libs.
-        // check!(pal, &library_path);
+        check!(pal, &library_path);
 
         // Checks that need to be done for both the compiler and std libraries.
         check!(unit_tests, &src_path, false);