Rollup merge of #86137 - GuillaumeGomez:error-code-cleanup, r=Mark-Simulacrum

Error code cleanup and enforce checks

Fixes #86097.

It now checks if an error code is unused, and if so, will report an error if the error code wasn't commented out in the `error_codes.rs` file. It also checks that the constant used in the tidy check is up-to-date.

r? `@Mark-Simulacrum`

Author: JohnTitor

compiler/rustc_error_codes/src/error_codes.rs

@@ -609,7 +609,7 @@ E0783: include_str!("./error_codes/E0783.md"),
 //  E0540, // multiple rustc_deprecated attributes
     E0544, // multiple stability levels
 //  E0548, // replaced with a generic attribute input check
-    E0553, // multiple rustc_const_unstable attributes
+//  E0553, // multiple rustc_const_unstable attributes
 //  E0555, // replaced with a generic attribute input check
 //  E0558, // replaced with a generic attribute input check
 //  E0563, // cannot determine a type for this `impl Trait` removed in 6383de15
@@ -620,10 +620,9 @@ E0783: include_str!("./error_codes/E0783.md"),
 //  E0612, // merged into E0609
 //  E0613, // Removed (merged with E0609)
     E0625, // thread-local statics cannot be accessed at compile-time
-    E0629, // missing 'feature' (rustc_const_unstable)
-    // rustc_const_unstable attribute must be paired with stable/unstable
-    // attribute
-    E0630,
+//  E0629, // missing 'feature' (rustc_const_unstable)
+//  E0630, // rustc_const_unstable attribute must be paired with stable/unstable
+           // attribute
     E0632, // cannot provide explicit generic arguments when `impl Trait` is
            // used in argument position
     E0640, // infer outlives requirements

src/tools/tidy/src/error_codes_check.rs

@@ -6,20 +6,33 @@ use std::ffi::OsStr;
 use std::fs::read_to_string;
 use std::path::Path;
 
+use regex::Regex;
+
 // A few of those error codes can't be tested but all the others can and *should* be tested!
 const EXEMPTED_FROM_TEST: &[&str] = &[
-    "E0227", "E0279", "E0280", "E0313", "E0314", "E0315", "E0377", "E0461", "E0462", "E0464",
-    "E0465", "E0473", "E0474", "E0475", "E0476", "E0479", "E0480", "E0481", "E0482", "E0483",
-    "E0484", "E0485", "E0486", "E0487", "E0488", "E0489", "E0514", "E0519", "E0523", "E0553",
-    "E0554", "E0570", "E0629", "E0630", "E0640", "E0717", "E0729",
+    "E0227", "E0279", "E0280", "E0313", "E0377", "E0461", "E0462", "E0464", "E0465", "E0476",
+    "E0482", "E0514", "E0519", "E0523", "E0554", "E0570", "E0640", "E0717", "E0729",
 ];
 
 // Some error codes don't have any tests apparently...
 const IGNORE_EXPLANATION_CHECK: &[&str] = &["E0570", "E0601", "E0602", "E0729"];
 
+// If the file path contains any of these, we don't want to try to extract error codes from it.
+//
+// We need to declare each path in the windows version (with backslash).
+const PATHS_TO_IGNORE_FOR_EXTRACTION: &[&str] =
+    &["src/test/", "src\\test\\", "src/doc/", "src\\doc\\", "src/tools/", "src\\tools\\"];
+
+#[derive(Default, Debug)]
+struct ErrorCodeStatus {
+    has_test: bool,
+    has_explanation: bool,
+    is_used: bool,
+}
+
 fn check_error_code_explanation(
     f: &str,
-    error_codes: &mut HashMap<String, bool>,
+    error_codes: &mut HashMap<String, ErrorCodeStatus>,
     err_code: String,
 ) -> bool {
     let mut invalid_compile_fail_format = false;
@@ -30,15 +43,15 @@ fn check_error_code_explanation(
         if s.starts_with("```") {
             if s.contains("compile_fail") && s.contains('E') {
                 if !found_error_code {
-                    error_codes.insert(err_code.clone(), true);
+                    error_codes.get_mut(&err_code).map(|x| x.has_test = true);
                     found_error_code = true;
                 }
             } else if s.contains("compile-fail") {
                 invalid_compile_fail_format = true;
             }
         } else if s.starts_with("#### Note: this error code is no longer emitted by the compiler") {
             if !found_error_code {
-                error_codes.get_mut(&err_code).map(|x| *x = true);
+                error_codes.get_mut(&err_code).map(|x| x.has_test = true);
                 found_error_code = true;
             }
         }
@@ -77,7 +90,7 @@ macro_rules! some_or_continue {
 
 fn extract_error_codes(
     f: &str,
-    error_codes: &mut HashMap<String, bool>,
+    error_codes: &mut HashMap<String, ErrorCodeStatus>,
     path: &Path,
     errors: &mut Vec<String>,
 ) {
@@ -90,15 +103,16 @@ fn extract_error_codes(
                 .split_once(':')
                 .expect(
                     format!(
-                        "Expected a line with the format `E0xxx: include_str!(\"..\")`, but got {} without a `:` delimiter",
+                        "Expected a line with the format `E0xxx: include_str!(\"..\")`, but got {} \
+                         without a `:` delimiter",
                         s,
-                    ).as_str()
+                    )
+                    .as_str(),
                 )
                 .0
                 .to_owned();
-            if !error_codes.contains_key(&err_code) {
-                error_codes.insert(err_code.clone(), false);
-            }
+            error_codes.entry(err_code.clone()).or_default().has_explanation = true;
+
             // Now we extract the tests from the markdown file!
             let md_file_name = match s.split_once("include_str!(\"") {
                 None => continue,
@@ -145,15 +159,15 @@ fn extract_error_codes(
             .to_string();
             if !error_codes.contains_key(&err_code) {
                 // this check should *never* fail!
-                error_codes.insert(err_code, false);
+                error_codes.insert(err_code, ErrorCodeStatus::default());
             }
         } else if s == ";" {
             reached_no_explanation = true;
         }
     }
 }
 
-fn extract_error_codes_from_tests(f: &str, error_codes: &mut HashMap<String, bool>) {
+fn extract_error_codes_from_tests(f: &str, error_codes: &mut HashMap<String, ErrorCodeStatus>) {
     for line in f.lines() {
         let s = line.trim();
         if s.starts_with("error[E") || s.starts_with("warning[E") {
@@ -164,8 +178,24 @@ fn extract_error_codes_from_tests(f: &str, error_codes: &mut HashMap<String, boo
                     Some((_, err_code)) => err_code,
                 },
             };
-            let nb = error_codes.entry(err_code.to_owned()).or_insert(false);
-            *nb = true;
+            error_codes.entry(err_code.to_owned()).or_default().has_test = true;
+        }
+    }
+}
+
+fn extract_error_codes_from_source(
+    f: &str,
+    error_codes: &mut HashMap<String, ErrorCodeStatus>,
+    regex: &Regex,
+) {
+    for line in f.lines() {
+        if line.trim_start().starts_with("//") {
+            continue;
+        }
+        for cap in regex.captures_iter(line) {
+            if let Some(error_code) = cap.get(1) {
+                error_codes.entry(error_code.as_str().to_owned()).or_default().is_used = true;
+            }
         }
     }
 }
@@ -174,8 +204,17 @@ pub fn check(paths: &[&Path], bad: &mut bool) {
     let mut errors = Vec::new();
     let mut found_explanations = 0;
     let mut found_tests = 0;
+    let mut error_codes: HashMap<String, ErrorCodeStatus> = HashMap::new();
+    // We want error codes which match the following cases:
+    //
+    // * foo(a, E0111, a)
+    // * foo(a, E0111)
+    // * foo(E0111, a)
+    // * #[error = "E0111"]
+    let regex = Regex::new(r#"[(,"\s](E\d{4})[,)"]"#).unwrap();
+
     println!("Checking which error codes lack tests...");
-    let mut error_codes: HashMap<String, bool> = HashMap::new();
+
     for path in paths {
         super::walk(path, &mut |path| super::filter_dirs(path), &mut |entry, contents| {
             let file_name = entry.file_name();
@@ -185,6 +224,11 @@ pub fn check(paths: &[&Path], bad: &mut bool) {
             } else if entry.path().extension() == Some(OsStr::new("stderr")) {
                 extract_error_codes_from_tests(contents, &mut error_codes);
                 found_tests += 1;
+            } else if entry.path().extension() == Some(OsStr::new("rs")) {
+                let path = entry.path().to_string_lossy();
+                if PATHS_TO_IGNORE_FOR_EXTRACTION.iter().all(|c| !path.contains(c)) {
+                    extract_error_codes_from_source(contents, &mut error_codes, &regex);
+                }
             }
         });
     }
@@ -199,15 +243,43 @@ pub fn check(paths: &[&Path], bad: &mut bool) {
     if errors.is_empty() {
         println!("Found {} error codes", error_codes.len());
 
-        for (err_code, nb) in &error_codes {
-            if !*nb && !EXEMPTED_FROM_TEST.contains(&err_code.as_str()) {
+        for (err_code, error_status) in &error_codes {
+            if !error_status.has_test && !EXEMPTED_FROM_TEST.contains(&err_code.as_str()) {
                 errors.push(format!("Error code {} needs to have at least one UI test!", err_code));
-            } else if *nb && EXEMPTED_FROM_TEST.contains(&err_code.as_str()) {
+            } else if error_status.has_test && EXEMPTED_FROM_TEST.contains(&err_code.as_str()) {
                 errors.push(format!(
                     "Error code {} has a UI test, it shouldn't be listed into EXEMPTED_FROM_TEST!",
                     err_code
                 ));
             }
+            if !error_status.is_used && !error_status.has_explanation {
+                errors.push(format!(
+                    "Error code {} isn't used and doesn't have an error explanation, it should be \
+                     commented in error_codes.rs file",
+                    err_code
+                ));
+            }
+        }
+    }
+    if errors.is_empty() {
+        // Checking if local constants need to be cleaned.
+        for err_code in EXEMPTED_FROM_TEST {
+            match error_codes.get(err_code.to_owned()) {
+                Some(status) => {
+                    if status.has_test {
+                        errors.push(format!(
+                            "{} error code has a test and therefore should be \
+                            removed from the `EXEMPTED_FROM_TEST` constant",
+                            err_code
+                        ));
+                    }
+                }
+                None => errors.push(format!(
+                    "{} error code isn't used anymore and therefore should be removed \
+                        from `EXEMPTED_FROM_TEST` constant",
+                    err_code
+                )),
+            }
         }
     }
     errors.sort();