Rollup merge of #152949 - Shunpoco:tidy-ci, r=Kobzol

Introduce --ci flag in tidy

## Context
Currently tidy doesn't have `--ci` flag. Even if bootstrap has --ci=true, it isn't passed to tidy. As a result, some parts of tidy where we need to turn on `ci mode` are never executed on our local.

## Change
This PR introduces --ci flag in tidy, and modify CiInfo and other parts of tidy where currently use `CiEnv::is_ci()` for checking if the running environment is ci or not.

Author: matthiaskrgr

src/bootstrap/src/core/build_steps/test.rs

@@ -1334,6 +1334,9 @@ impl Step for Tidy {
         if builder.config.cmd.bless() {
             cmd.arg("--bless");
         }
+        if builder.config.is_running_on_ci() {
+            cmd.arg("--ci=true");
+        }
         if let Some(s) =
             builder.config.cmd.extra_checks().or(builder.config.tidy_extra_checks.as_deref())
         {

src/tools/tidy/src/alphabetical/tests.rs

@@ -5,7 +5,7 @@ use crate::diagnostics::{TidyCtx, TidyFlags};
 
 #[track_caller]
 fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) {
-    let tidy_ctx = TidyCtx::new(Path::new("/"), false, TidyFlags::default());
+    let tidy_ctx = TidyCtx::new(Path::new("/"), false, None, TidyFlags::default());
     let mut check = tidy_ctx.start_check("alphabetical-test");
     check_lines(Path::new(name), lines, &tidy_ctx, &mut check);
 
@@ -37,7 +37,7 @@ fn bless_test(before: &str, after: &str) {
     let temp_path = tempfile::Builder::new().tempfile().unwrap().into_temp_path();
     std::fs::write(&temp_path, before).unwrap();
 
-    let tidy_ctx = TidyCtx::new(Path::new("/"), false, TidyFlags::new(true));
+    let tidy_ctx = TidyCtx::new(Path::new("/"), false, None, TidyFlags::new(true));
 
     let mut check = tidy_ctx.start_check("alphabetical-test");
     check_lines(&temp_path, before, &tidy_ctx, &mut check);

src/tools/tidy/src/arg_parser.rs

@@ -15,6 +15,7 @@ pub struct TidyArgParser {
     pub npm: PathBuf,
     pub verbose: bool,
     pub bless: bool,
+    pub ci: Option<bool>,
     pub extra_checks: Option<Vec<String>>,
     pub pos_args: Vec<String>,
 }
@@ -59,6 +60,7 @@ impl TidyArgParser {
             )
             .arg(Arg::new("verbose").help("verbose").long("verbose").action(ArgAction::SetTrue))
             .arg(Arg::new("bless").help("target files are modified").long("bless").action(ArgAction::SetTrue))
+            .arg(Arg::new("ci").help("ci flag").long("ci").default_missing_value("true").num_args(0..=1).value_parser(value_parser!(bool)))
             .arg(
                 Arg::new("extra_checks")
                     .help("extra checks")
@@ -78,6 +80,7 @@ impl TidyArgParser {
             npm: matches.get_one::<PathBuf>("npm").unwrap().clone(),
             verbose: *matches.get_one::<bool>("verbose").unwrap(),
             bless: *matches.get_one::<bool>("bless").unwrap(),
+            ci: matches.get_one::<bool>("ci").cloned(),
             extra_checks: None,
             pos_args: vec![],
         };

src/tools/tidy/src/arg_parser/tests.rs

@@ -19,6 +19,7 @@ fn test_tidy_parser_full() {
         "yarn",
         "--verbose",
         "--bless",
+        "--ci",
         "--extra-checks",
         "if-installed:auto:js,auto:if-installed:py,if-installed:auto:cpp,if-installed:auto:spellcheck",
         "--", // pos_args
@@ -38,6 +39,8 @@ fn test_tidy_parser_full() {
     assert_eq!(parsed_args.npm, PathBuf::from("yarn"));
     assert!(parsed_args.verbose);
     assert!(parsed_args.bless);
+    assert!(parsed_args.ci.is_some());
+    assert!(parsed_args.ci.unwrap());
     assert_eq!(
         parsed_args.extra_checks,
         Some(vec![
@@ -166,3 +169,46 @@ fn test_tidy_parser_missing_npm_path() {
     let cmd = TidyArgParser::command();
     assert!(cmd.try_get_matches_from(args).is_err());
 }
+
+// --ci has some variations
+#[test]
+fn test_tidy_parse_ci_flag() {
+    // They are requried
+    let base_args = vec![
+        "rust-tidy",
+        "--root-path",
+        "/home/user/rust",
+        "--cargo-path",
+        "/home/user/rust/build/x86_64-unknown-linux-gnu/stage0/bin/cargo",
+        "--output-dir",
+        "/home/user/rust/build",
+        "--concurrency",
+        "16",
+        "--npm-path",
+        "yarn",
+    ];
+
+    // No --ci
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(&base_args));
+    assert!(parsed_args.ci.is_none());
+
+    // --ci
+    let mut args1 = base_args.clone();
+    args1.push("--ci");
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args1));
+    assert!(parsed_args.ci.is_some());
+
+    // --ci=true
+    let mut args2 = base_args.clone();
+    args2.extend_from_slice(&["--ci", "true"]);
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args2));
+    assert!(parsed_args.ci.is_some());
+    assert!(parsed_args.ci.unwrap());
+
+    // --ci=false
+    let mut args2 = base_args.clone();
+    args2.extend_from_slice(&["--ci", "false"]);
+    let parsed_args = TidyArgParser::build(TidyArgParser::command().get_matches_from(args2));
+    assert!(parsed_args.ci.is_some());
+    assert!(!parsed_args.ci.unwrap());
+}

src/tools/tidy/src/deps.rs

@@ -6,7 +6,6 @@ use std::fs::{File, read_dir};
 use std::io::Write;
 use std::path::Path;
 
-use build_helper::ci::CiEnv;
 use cargo_metadata::semver::Version;
 use cargo_metadata::{Metadata, Package, PackageId};
 
@@ -638,7 +637,7 @@ pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) {
     check_proc_macro_dep_list(root, cargo, bless, &mut check);
 
     for &WorkspaceInfo { path, exceptions, crates_and_deps, submodules } in WORKSPACES {
-        if has_missing_submodule(root, submodules) {
+        if has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {
             continue;
         }
 
@@ -758,8 +757,8 @@ pub static CRATES: &[&str] = &[
 /// Used to skip a check if a submodule is not checked out, and not in a CI environment.
 ///
 /// This helps prevent enforcing developers to fetch submodules for tidy.
-pub fn has_missing_submodule(root: &Path, submodules: &[&str]) -> bool {
-    !CiEnv::is_ci()
+pub fn has_missing_submodule(root: &Path, submodules: &[&str], is_ci: bool) -> bool {
+    !is_ci
         && submodules.iter().any(|submodule| {
             let path = root.join(submodule);
             !path.exists()

src/tools/tidy/src/diagnostics.rs

@@ -4,6 +4,9 @@ use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
+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::Color;
 
 /// CLI flags used by tidy.
@@ -28,25 +31,47 @@ impl TidyFlags {
 pub struct TidyCtx {
     tidy_flags: TidyFlags,
     diag_ctx: Arc<Mutex<DiagCtxInner>>,
+    ci_env: CiEnv,
+    pub base_commit: Option<String>,
 }
 
 impl TidyCtx {
-    pub fn new(root_path: &Path, verbose: bool, tidy_flags: TidyFlags) -> Self {
-        Self {
+    pub fn new(
+        root_path: &Path,
+        verbose: bool,
+        ci_flag: Option<bool>,
+        tidy_flags: TidyFlags,
+    ) -> Self {
+        let ci_env = match ci_flag {
+            Some(true) => CiEnv::GitHubActions,
+            Some(false) => CiEnv::None,
+            None => CiEnv::current(),
+        };
+
+        let mut tidy_ctx = Self {
             diag_ctx: Arc::new(Mutex::new(DiagCtxInner {
                 running_checks: Default::default(),
                 finished_checks: Default::default(),
                 root_path: root_path.to_path_buf(),
                 verbose,
             })),
             tidy_flags,
-        }
+            ci_env,
+            base_commit: None,
+        };
+        tidy_ctx.base_commit = find_base_commit(&tidy_ctx);
+
+        tidy_ctx
     }
 
     pub fn is_bless_enabled(&self) -> bool {
         self.tidy_flags.bless
     }
 
+    pub fn is_running_on_ci(&self) -> bool {
+        self.ci_env.is_running_in_ci()
+    }
+
     pub fn start_check<Id: Into<CheckId>>(&self, id: Id) -> RunningCheck {
         let mut id = id.into();
 
@@ -75,6 +100,46 @@ impl TidyCtx {
     }
 }
 
+fn find_base_commit(tidy_ctx: &TidyCtx) -> Option<String> {
+    let mut check = tidy_ctx.start_check("CI history");
+
+    let stage0 = parse_stage0_file();
+    let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config;
+
+    let base_commit = match get_closest_upstream_commit(
+        None,
+        &GitConfig {
+            nightly_branch: &nightly_branch,
+            git_merge_commit_email: &git_merge_commit_email,
+        },
+        tidy_ctx.ci_env,
+    ) {
+        Ok(Some(commit)) => Some(commit),
+        Ok(None) => {
+            error_if_in_ci("no base commit found", tidy_ctx.is_running_on_ci(), &mut check);
+            None
+        }
+        Err(error) => {
+            error_if_in_ci(
+                &format!("failed to retrieve base commit: {error}"),
+                tidy_ctx.is_running_on_ci(),
+                &mut check,
+            );
+            None
+        }
+    };
+
+    base_commit
+}
+
+fn error_if_in_ci(msg: &str, is_ci: bool, check: &mut RunningCheck) {
+    if is_ci {
+        check.error(msg);
+    } else {
+        check.warning(format!("{msg}. Some checks will be skipped."));
+    }
+}
+
 struct DiagCtxInner {
     running_checks: HashSet<CheckId>,
     finished_checks: HashSet<FinishedCheck>,
@@ -175,7 +240,7 @@ impl RunningCheck {
     /// Useful if you want to run some functions from tidy without configuring
     /// diagnostics.
     pub fn new_noop() -> Self {
-        let ctx = TidyCtx::new(Path::new(""), false, TidyFlags::default());
+        let ctx = TidyCtx::new(Path::new(""), false, None, TidyFlags::default());
         ctx.start_check("noop")
     }
 

src/tools/tidy/src/error_codes.rs

@@ -36,11 +36,11 @@ const IGNORE_DOCTEST_CHECK: &[&str] = &["E0464", "E0570", "E0601", "E0602", "E07
 const IGNORE_UI_TEST_CHECK: &[&str] =
     &["E0461", "E0465", "E0514", "E0554", "E0640", "E0717", "E0729"];
 
-pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, tidy_ctx: TidyCtx) {
+pub fn check(root_path: &Path, search_paths: &[&Path], tidy_ctx: TidyCtx) {
     let mut check = tidy_ctx.start_check("error_codes");
 
     // Check that no error code explanation was removed.
-    check_removed_error_code_explanation(ci_info, &mut check);
+    check_removed_error_code_explanation(&tidy_ctx.base_commit, &mut check);
 
     // Stage 1: create list
     let error_codes = extract_error_codes(root_path, &mut check);
@@ -57,8 +57,8 @@ pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo,
     check_error_codes_used(search_paths, &error_codes, &mut check, &no_longer_emitted);
 }
 
-fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, check: &mut RunningCheck) {
-    let Some(base_commit) = &ci_info.base_commit else {
+fn check_removed_error_code_explanation(base_commit: &Option<String>, check: &mut RunningCheck) {
+    let Some(base_commit) = base_commit else {
         check.verbose_msg("Skipping error code explanation removal check");
         return;
     };

src/tools/tidy/src/extdeps.rs

@@ -19,7 +19,7 @@ pub fn check(root: &Path, tidy_ctx: TidyCtx) {
     let mut check = tidy_ctx.start_check("extdeps");
 
     for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES {
-        if crate::deps::has_missing_submodule(root, submodules) {
+        if crate::deps::has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {
             continue;
         }
 

src/tools/tidy/src/extra_checks/mod.rs

@@ -23,9 +23,6 @@ use std::process::Command;
 use std::str::FromStr;
 use std::{env, fmt, fs, io};
 
-use build_helper::ci::CiEnv;
-
-use crate::CiInfo;
 use crate::diagnostics::TidyCtx;
 
 mod rustdoc_js;
@@ -53,7 +50,6 @@ const SPELLCHECK_VER: &str = "1.38.1";
 pub fn check(
     root_path: &Path,
     outdir: &Path,
-    ci_info: &CiInfo,
     librustdoc_path: &Path,
     tools_path: &Path,
     npm: &Path,
@@ -67,7 +63,6 @@ pub fn check(
     if let Err(e) = check_impl(
         root_path,
         outdir,
-        ci_info,
         librustdoc_path,
         tools_path,
         npm,
@@ -83,7 +78,6 @@ pub fn check(
 fn check_impl(
     root_path: &Path,
     outdir: &Path,
-    ci_info: &CiInfo,
     librustdoc_path: &Path,
     tools_path: &Path,
     npm: &Path,
@@ -121,9 +115,12 @@ fn check_impl(
     };
     lint_args.retain(|ck| ck.is_non_if_installed_or_matches(root_path, outdir));
     if lint_args.iter().any(|ck| ck.auto) {
-        crate::files_modified_batch_filter(ci_info, &mut lint_args, |ck, path| {
-            ck.is_non_auto_or_matches(path)
-        });
+        crate::files_modified_batch_filter(
+            &tidy_ctx.base_commit,
+            tidy_ctx.is_running_on_ci(),
+            &mut lint_args,
+            |ck, path| ck.is_non_auto_or_matches(path),
+        );
     }
 
     macro_rules! extra_check {
@@ -321,7 +318,7 @@ fn check_impl(
         } else {
             eprintln!("spellchecking files");
         }
-        let res = spellcheck_runner(root_path, &outdir, &cargo, &args);
+        let res = spellcheck_runner(root_path, &outdir, &cargo, &args, tidy_ctx.is_running_on_ci());
         if res.is_err() {
             rerun_with_bless("spellcheck", "fix typos");
         }
@@ -629,9 +626,16 @@ fn spellcheck_runner(
     outdir: &Path,
     cargo: &Path,
     args: &[&str],
+    is_ci: bool,
 ) -> Result<(), Error> {
-    let bin_path =
-        ensure_version_or_cargo_install(outdir, cargo, "typos-cli", "typos", SPELLCHECK_VER)?;
+    let bin_path = ensure_version_or_cargo_install(
+        outdir,
+        cargo,
+        "typos-cli",
+        "typos",
+        SPELLCHECK_VER,
+        is_ci,
+    )?;
     match Command::new(bin_path).current_dir(src_root).args(args).status() {
         Ok(status) => {
             if status.success() {
@@ -713,6 +717,7 @@ fn ensure_version_or_cargo_install(
     pkg_name: &str,
     bin_name: &str,
     version: &str,
+    is_ci: bool,
 ) -> Result<PathBuf, Error> {
     if let Ok(bin_path) = ensure_version(build_dir, bin_name, version) {
         return Ok(bin_path);
@@ -746,7 +751,7 @@ fn ensure_version_or_cargo_install(
     // On CI, we set opt-level flag for quicker installation.
     // Since lower opt-level decreases the tool's performance,
     // we don't set this option on local.
-    if CiEnv::is_ci() {
+    if is_ci {
         cmd.env("RUSTFLAGS", "-Copt-level=0");
     }