Skip to content

Move error code explanation removal check into tidy #142827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/ci/docker/host-x86_64/mingw-check-1/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
20 changes: 0 additions & 20 deletions src/ci/docker/host-x86_64/mingw-check-1/validate-error-codes.sh

This file was deleted.

1 change: 0 additions & 1 deletion src/ci/docker/host-x86_64/mingw-check-tidy/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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)'

Expand Down
32 changes: 31 additions & 1 deletion src/tools/tidy/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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));
Expand Down
61 changes: 61 additions & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
48 changes: 6 additions & 42 deletions src/tools/tidy/src/rustdoc_json.rs
Original file line number Diff line number Diff line change
@@ -1,55 +1,19 @@
//! 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 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()
Expand All @@ -67,7 +31,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;
Expand Down
Loading