diff --git a/compiler/rustc_error_codes/src/error_codes.rs b/compiler/rustc_error_codes/src/error_codes.rs index f10efd832361c..ff7a2344e6953 100644 --- a/compiler/rustc_error_codes/src/error_codes.rs +++ b/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 diff --git a/src/tools/tidy/src/error_codes_check.rs b/src/tools/tidy/src/error_codes_check.rs index d6e0ebaa5410c..63fbee34bd6e4 100644 --- a/src/tools/tidy/src/error_codes_check.rs +++ b/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, + error_codes: &mut HashMap, err_code: String, ) -> bool { let mut invalid_compile_fail_format = false; @@ -30,7 +43,7 @@ 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") { @@ -38,7 +51,7 @@ fn check_error_code_explanation( } } 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, + error_codes: &mut HashMap, path: &Path, errors: &mut Vec, ) { @@ -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,7 +159,7 @@ 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; @@ -153,7 +167,7 @@ fn extract_error_codes( } } -fn extract_error_codes_from_tests(f: &str, error_codes: &mut HashMap) { +fn extract_error_codes_from_tests(f: &str, error_codes: &mut HashMap) { 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 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, + 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 = 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 = 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, ®ex); + } } }); } @@ -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();