Rollup merge of #142827 - GuillaumeGomez:tidy-error-code-removal, r=Kobzol

Move error code explanation removal check into tidy

Follow-up of #142677.

This PR replaces a shell script with rust code.

r? ghost

Author: workingjubilee

src/ci/docker/host-x86_64/mingw-check-1/Dockerfile

@@ -39,7 +39,6 @@ RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-require
 
 COPY host-x86_64/mingw-check-1/check-default-config-profiles.sh /scripts/
 COPY host-x86_64/mingw-check-1/validate-toolstate.sh /scripts/
-COPY host-x86_64/mingw-check-1/validate-error-codes.sh /scripts/
 
 # Check library crates on all tier 1 targets.
 # We disable optimized compiler built-ins because that requires a C toolchain for the target.
@@ -52,7 +51,6 @@ ENV SCRIPT \
            python3 ../x.py check --stage 1 --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \
            python3 ../x.py check --stage 1 --set build.optimized-compiler-builtins=false core alloc std --target=aarch64-unknown-linux-gnu,i686-pc-windows-msvc,i686-unknown-linux-gnu,x86_64-apple-darwin,x86_64-pc-windows-gnu,x86_64-pc-windows-msvc && \
            /scripts/validate-toolstate.sh && \
-           /scripts/validate-error-codes.sh && \
            reuse --include-submodules lint && \
            python3 ../x.py test collect-license-metadata && \
            # Runs checks to ensure that there are no issues in our JS code.

src/ci/docker/host-x86_64/mingw-check-1/validate-error-codes.sh

@@ -39,7 +39,6 @@ RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-require
     && pip3 install virtualenv
 
 COPY host-x86_64/mingw-check-1/validate-toolstate.sh /scripts/
-COPY host-x86_64/mingw-check-1/validate-error-codes.sh /scripts/
 
 RUN bash -c 'npm install -g eslint@$(cat /tmp/eslint.version)'
 

src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile

@@ -43,9 +43,18 @@ macro_rules! verbose_print {
     };
 }
 
-pub fn check(root_path: &Path, search_paths: &[&Path], verbose: bool, bad: &mut bool) {
+pub fn check(
+    root_path: &Path,
+    search_paths: &[&Path],
+    verbose: bool,
+    ci_info: &crate::CiInfo,
+    bad: &mut bool,
+) {
     let mut errors = Vec::new();
 
+    // Check that no error code explanation was removed.
+    check_removed_error_code_explanation(ci_info, bad);
+
     // Stage 1: create list
     let error_codes = extract_error_codes(root_path, &mut errors);
     if verbose {
@@ -68,6 +77,27 @@ pub fn check(root_path: &Path, search_paths: &[&Path], verbose: bool, bad: &mut
     }
 }
 
+fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, bad: &mut bool) {
+    let Some(base_commit) = &ci_info.base_commit else {
+        eprintln!("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");
+        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.");
+        return;
+    }
+    println!("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> {
     let path = root_path.join(Path::new(ERROR_CODES_PATH));

src/tools/tidy/src/error_codes.rs

@@ -3,6 +3,12 @@
 //! This library contains the tidy lints and exposes it
 //! to be used by tools.
 
+use std::ffi::OsStr;
+use std::process::Command;
+
+use build_helper::ci::CiEnv;
+use build_helper::git::{GitConfig, get_closest_upstream_commit};
+use build_helper::stage0_parser::{Stage0Config, parse_stage0_file};
 use termcolor::WriteColor;
 
 macro_rules! static_regex {
@@ -63,6 +69,61 @@ fn tidy_error(args: &str) -> std::io::Result<()> {
     Ok(())
 }
 
+pub struct CiInfo {
+    pub git_merge_commit_email: String,
+    pub nightly_branch: String,
+    pub base_commit: Option<String>,
+    pub ci_env: CiEnv,
+}
+
+impl CiInfo {
+    pub fn new(bad: &mut bool) -> Self {
+        let stage0 = parse_stage0_file();
+        let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config;
+
+        let mut info = Self {
+            nightly_branch,
+            git_merge_commit_email,
+            ci_env: CiEnv::current(),
+            base_commit: None,
+        };
+        let base_commit = match get_closest_upstream_commit(None, &info.git_config(), info.ci_env) {
+            Ok(Some(commit)) => Some(commit),
+            Ok(None) => {
+                info.error_if_in_ci("no base commit found", bad);
+                None
+            }
+            Err(error) => {
+                info.error_if_in_ci(&format!("failed to retrieve base commit: {error}"), bad);
+                None
+            }
+        };
+        info.base_commit = base_commit;
+        info
+    }
+
+    pub fn git_config(&self) -> GitConfig<'_> {
+        GitConfig {
+            nightly_branch: &self.nightly_branch,
+            git_merge_commit_email: &self.git_merge_commit_email,
+        }
+    }
+
+    pub fn error_if_in_ci(&self, msg: &str, bad: &mut bool) {
+        if self.ci_env.is_running_in_ci() {
+            *bad = true;
+            eprintln!("tidy check error: {msg}");
+        } else {
+            eprintln!("tidy check warning: {msg}. Some checks will be skipped.");
+        }
+    }
+}
+
+pub fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<String> {
+    let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?;
+    Some(String::from_utf8_lossy(&output.stdout).into())
+}
+
 pub mod alphabetical;
 pub mod bins;
 pub mod debug_artifacts;

src/tools/tidy/src/lib.rs

@@ -48,7 +48,9 @@ fn main() {
     let extra_checks =
         cfg_args.iter().find(|s| s.starts_with("--extra-checks=")).map(String::as_str);
 
-    let bad = std::sync::Arc::new(AtomicBool::new(false));
+    let mut bad = false;
+    let ci_info = CiInfo::new(&mut bad);
+    let bad = std::sync::Arc::new(AtomicBool::new(bad));
 
     let drain_handles = |handles: &mut VecDeque<ScopedJoinHandle<'_, ()>>| {
         // poll all threads for completion before awaiting the oldest one
@@ -110,12 +112,12 @@ fn main() {
         check!(rustdoc_css_themes, &librustdoc_path);
         check!(rustdoc_templates, &librustdoc_path);
         check!(rustdoc_js, &librustdoc_path, &tools_path, &src_path);
-        check!(rustdoc_json, &src_path);
+        check!(rustdoc_json, &src_path, &ci_info);
         check!(known_bug, &crashes_path);
         check!(unknown_revision, &tests_path);
 
         // Checks that only make sense for the compiler.
-        check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose);
+        check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info);
         check!(fluent_alphabetical, &compiler_path, bless);
         check!(fluent_period, &compiler_path);
         check!(target_policy, &root_path);

src/tools/tidy/src/main.rs

@@ -1,56 +1,20 @@
 //! Tidy check to ensure that `FORMAT_VERSION` was correctly updated if `rustdoc-json-types` was
 //! updated as well.
 
-use std::ffi::OsStr;
 use std::path::Path;
-use std::process::Command;
 use std::str::FromStr;
 
-use build_helper::ci::CiEnv;
-use build_helper::git::{GitConfig, get_closest_upstream_commit};
-use build_helper::stage0_parser::parse_stage0_file;
-
 const RUSTDOC_JSON_TYPES: &str = "src/rustdoc-json-types";
 
-fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<String> {
-    let output = Command::new("git").arg("diff").arg(base_commit).arg(extra_arg).output().ok()?;
-    Some(String::from_utf8_lossy(&output.stdout).into())
-}
-
-fn error_if_in_ci(ci_env: CiEnv, msg: &str, bad: &mut bool) {
-    if ci_env.is_running_in_ci() {
-        *bad = true;
-        eprintln!("error in `rustdoc_json` tidy check: {msg}");
-    } else {
-        eprintln!("{msg}. Skipping `rustdoc_json` tidy check");
-    }
-}
-
-pub fn check(src_path: &Path, bad: &mut bool) {
+pub fn check(src_path: &Path, ci_info: &crate::CiInfo, bad: &mut bool) {
     println!("Checking tidy rustdoc_json...");
-    let stage0 = parse_stage0_file();
-    let ci_env = CiEnv::current();
-    let base_commit = match get_closest_upstream_commit(
-        None,
-        &GitConfig {
-            nightly_branch: &stage0.config.nightly_branch,
-            git_merge_commit_email: &stage0.config.git_merge_commit_email,
-        },
-        ci_env,
-    ) {
-        Ok(Some(commit)) => commit,
-        Ok(None) => {
-            error_if_in_ci(ci_env, "no base commit found", bad);
-            return;
-        }
-        Err(error) => {
-            error_if_in_ci(ci_env, &format!("failed to retrieve base commit: {error}"), bad);
-            return;
-        }
+    let Some(base_commit) = &ci_info.base_commit else {
+        eprintln!("No base commit, skipping rustdoc_json check");
+        return;
     };
 
     // First we check that `src/rustdoc-json-types` was modified.
-    match git_diff(&base_commit, "--name-status") {
+    match crate::git_diff(&base_commit, "--name-status") {
         Some(output) => {
             if !output
                 .lines()
@@ -68,7 +32,7 @@ pub fn check(src_path: &Path, bad: &mut bool) {
         }
     }
     // Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated.
-    match git_diff(&base_commit, src_path.join("rustdoc-json-types")) {
+    match crate::git_diff(&base_commit, src_path.join("rustdoc-json-types")) {
         Some(output) => {
             let mut format_version_updated = false;
             let mut latest_feature_comment_updated = false;