Check if error code is used

Author: GuillaumeGomez

src/tools/tidy/src/error_codes_check.rs

@@ -1,11 +1,14 @@
 //! Checks that all error codes have at least one test to prevent having error
 //! codes that are silently not thrown by the compiler anymore.
 
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 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",
@@ -17,9 +20,16 @@ const EXEMPTED_FROM_TEST: &[&str] = &[
 // Some error codes don't have any tests apparently...
 const IGNORE_EXPLANATION_CHECK: &[&str] = &["E0570", "E0601", "E0602", "E0729"];
 
+#[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 +40,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 +87,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,14 +100,26 @@ 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);
+            match error_codes.entry(err_code.clone()) {
+                Entry::Occupied(mut e) => {
+                    let mut entry = e.get_mut();
+                    entry.has_explanation = true
+                }
+                Entry::Vacant(e) => {
+                    e.insert(ErrorCodeStatus {
+                        has_test: false,
+                        is_used: false,
+                        has_explanation: true,
+                    });
+                }
             }
             // Now we extract the tests from the markdown file!
             let md_file_name = match s.split_once("include_str!(\"") {
@@ -145,15 +167,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 +186,48 @@ 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;
+            match error_codes.entry(err_code.to_owned()) {
+                Entry::Occupied(mut e) => {
+                    let mut entry = e.get_mut();
+                    entry.has_test = true
+                }
+                Entry::Vacant(e) => {
+                    e.insert(ErrorCodeStatus {
+                        has_test: true,
+                        is_used: false,
+                        has_explanation: false,
+                    });
+                }
+            }
+        }
+    }
+}
+
+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) {
+                match error_codes.entry(error_code.as_str().to_owned()) {
+                    Entry::Occupied(mut e) => {
+                        let mut entry = e.get_mut();
+                        entry.is_used = true
+                    }
+                    Entry::Vacant(e) => {
+                        e.insert(ErrorCodeStatus {
+                            has_test: false,
+                            is_used: true,
+                            has_explanation: false,
+                        });
+                    }
+                }
+            }
         }
     }
 }
@@ -174,8 +236,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 +256,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 ["src/test/", "src/doc/", "src/tools/"].iter().all(|c| !path.contains(c)) {
+                    extract_error_codes_from_source(contents, &mut error_codes, &regex);
+                }
             }
         });
     }
@@ -199,15 +275,22 @@ 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
+                ));
+            }
         }
     }
     errors.sort();