diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs index 37d91b14ca1b6..927d72e8ccb4b 100644 --- a/src/bootstrap/src/core/build_steps/check.rs +++ b/src/bootstrap/src/core/build_steps/check.rs @@ -11,6 +11,16 @@ use crate::core::config::TargetSelection; use crate::{Compiler, Mode, Subcommand}; use std::path::{Path, PathBuf}; +pub fn cargo_subcommand(kind: Kind) -> &'static str { + match kind { + Kind::Check + // We ensure check steps for both std and rustc from build_steps/clippy, so handle `Kind::Clippy` as well. + | Kind::Clippy => "check", + Kind::Fix => "fix", + _ => unreachable!(), + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Std { pub target: TargetSelection, @@ -22,97 +32,6 @@ pub struct Std { crates: Vec, } -/// Returns args for the subcommand itself (not for cargo) -fn args(builder: &Builder<'_>) -> Vec { - fn strings<'a>(arr: &'a [&str]) -> impl Iterator + 'a { - arr.iter().copied().map(String::from) - } - - if let Subcommand::Clippy { fix, allow_dirty, allow_staged, allow, deny, warn, forbid } = - &builder.config.cmd - { - // disable the most spammy clippy lints - let ignored_lints = [ - "many_single_char_names", // there are a lot in stdarch - "collapsible_if", - "type_complexity", - "missing_safety_doc", // almost 3K warnings - "too_many_arguments", - "needless_lifetimes", // people want to keep the lifetimes - "wrong_self_convention", - ]; - let mut args = vec![]; - if *fix { - #[rustfmt::skip] - args.extend(strings(&[ - "--fix", "-Zunstable-options", - // FIXME: currently, `--fix` gives an error while checking tests for libtest, - // possibly because libtest is not yet built in the sysroot. - // As a workaround, avoid checking tests and benches when passed --fix. - "--lib", "--bins", "--examples", - ])); - - if *allow_dirty { - args.push("--allow-dirty".to_owned()); - } - - if *allow_staged { - args.push("--allow-staged".to_owned()); - } - } - - args.extend(strings(&["--"])); - - if deny.is_empty() && forbid.is_empty() { - args.extend(strings(&["--cap-lints", "warn"])); - } - - let all_args = std::env::args().collect::>(); - args.extend(get_clippy_rules_in_order(&all_args, allow, deny, warn, forbid)); - - args.extend(ignored_lints.iter().map(|lint| format!("-Aclippy::{}", lint))); - args.extend(builder.config.free_args.clone()); - args - } else { - builder.config.free_args.clone() - } -} - -/// We need to keep the order of the given clippy lint rules before passing them. -/// Since clap doesn't offer any useful interface for this purpose out of the box, -/// we have to handle it manually. -pub(crate) fn get_clippy_rules_in_order( - all_args: &[String], - allow_rules: &[String], - deny_rules: &[String], - warn_rules: &[String], - forbid_rules: &[String], -) -> Vec { - let mut result = vec![]; - - for (prefix, item) in - [("-A", allow_rules), ("-D", deny_rules), ("-W", warn_rules), ("-F", forbid_rules)] - { - item.iter().for_each(|v| { - let rule = format!("{prefix}{v}"); - let position = all_args.iter().position(|t| t == &rule).unwrap(); - result.push((position, rule)); - }); - } - - result.sort_by_key(|&(position, _)| position); - result.into_iter().map(|v| v.1).collect() -} - -fn cargo_subcommand(kind: Kind) -> &'static str { - match kind { - Kind::Check => "check", - Kind::Clippy => "clippy", - Kind::Fix => "fix", - _ => unreachable!(), - } -} - impl Std { pub fn new(target: TargetSelection) -> Self { Self { target, crates: vec![] } @@ -164,7 +83,7 @@ impl Step for Std { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &libstd_stamp(builder, compiler, target), vec![], true, @@ -221,7 +140,7 @@ impl Step for Std { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &libstd_test_stamp(builder, compiler, target), vec![], true, @@ -318,7 +237,7 @@ impl Step for Rustc { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &librustc_stamp(builder, compiler, target), vec![], true, @@ -384,7 +303,7 @@ impl Step for CodegenBackend { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &codegen_backend_stamp(builder, compiler, target, backend), vec![], true, @@ -450,7 +369,7 @@ impl Step for RustAnalyzer { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &stamp(builder, compiler, target), vec![], true, @@ -513,7 +432,7 @@ macro_rules! tool_check_step { run_cargo( builder, cargo, - args(builder), + builder.config.free_args.clone(), &stamp(builder, compiler, target), vec![], true, diff --git a/src/bootstrap/src/core/build_steps/clippy.rs b/src/bootstrap/src/core/build_steps/clippy.rs new file mode 100644 index 0000000000000..33323ec1e5de1 --- /dev/null +++ b/src/bootstrap/src/core/build_steps/clippy.rs @@ -0,0 +1,330 @@ +//! Implementation of running clippy on the compiler, standard library and various tools. + +use std::path::Path; + +use crate::builder::Builder; +use crate::builder::ShouldRun; +use crate::core::builder; +use crate::core::builder::crate_description; +use crate::core::builder::Alias; +use crate::core::builder::Kind; +use crate::core::builder::RunConfig; +use crate::core::builder::Step; +use crate::Mode; +use crate::Subcommand; +use crate::TargetSelection; + +use super::check; +use super::compile; +use super::compile::librustc_stamp; +use super::compile::libstd_stamp; +use super::compile::run_cargo; +use super::compile::rustc_cargo; +use super::compile::std_cargo; +use super::tool::prepare_tool_cargo; +use super::tool::SourceType; + +// Disable the most spammy clippy lints +const IGNORED_RULES_FOR_STD_AND_RUSTC: &[&str] = &[ + "many_single_char_names", // there are a lot in stdarch + "collapsible_if", + "type_complexity", + "missing_safety_doc", // almost 3K warnings + "too_many_arguments", + "needless_lifetimes", // people want to keep the lifetimes + "wrong_self_convention", +]; + +fn lint_args(builder: &Builder<'_>, ignored_rules: &[&str]) -> Vec { + fn strings<'a>(arr: &'a [&str]) -> impl Iterator + 'a { + arr.iter().copied().map(String::from) + } + + let Subcommand::Clippy { fix, allow_dirty, allow_staged, allow, deny, warn, forbid } = + &builder.config.cmd + else { + unreachable!("clippy::lint_args can only be called from `clippy` subcommands."); + }; + + let mut args = vec![]; + if *fix { + #[rustfmt::skip] + args.extend(strings(&[ + "--fix", "-Zunstable-options", + // FIXME: currently, `--fix` gives an error while checking tests for libtest, + // possibly because libtest is not yet built in the sysroot. + // As a workaround, avoid checking tests and benches when passed --fix. + "--lib", "--bins", "--examples", + ])); + + if *allow_dirty { + args.push("--allow-dirty".to_owned()); + } + + if *allow_staged { + args.push("--allow-staged".to_owned()); + } + } + + args.extend(strings(&["--"])); + + if deny.is_empty() && forbid.is_empty() { + args.extend(strings(&["--cap-lints", "warn"])); + } + + let all_args = std::env::args().collect::>(); + args.extend(get_clippy_rules_in_order(&all_args, allow, deny, warn, forbid)); + + args.extend(ignored_rules.iter().map(|lint| format!("-Aclippy::{}", lint))); + args.extend(builder.config.free_args.clone()); + args +} + +/// We need to keep the order of the given clippy lint rules before passing them. +/// Since clap doesn't offer any useful interface for this purpose out of the box, +/// we have to handle it manually. +pub(crate) fn get_clippy_rules_in_order( + all_args: &[String], + allow_rules: &[String], + deny_rules: &[String], + warn_rules: &[String], + forbid_rules: &[String], +) -> Vec { + let mut result = vec![]; + + for (prefix, item) in + [("-A", allow_rules), ("-D", deny_rules), ("-W", warn_rules), ("-F", forbid_rules)] + { + item.iter().for_each(|v| { + let rule = format!("{prefix}{v}"); + let position = all_args.iter().position(|t| t == &rule).unwrap(); + result.push((position, rule)); + }); + } + + result.sort_by_key(|&(position, _)| position); + result.into_iter().map(|v| v.1).collect() +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Std { + pub target: TargetSelection, + /// Whether to lint only a subset of crates. + crates: Vec, +} + +impl Step for Std { + type Output = (); + const DEFAULT: bool = true; + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.crate_or_deps("sysroot").path("library") + } + + fn make_run(run: RunConfig<'_>) { + let crates = run.make_run_crates(Alias::Library); + run.builder.ensure(Std { target: run.target, crates }); + } + + fn run(self, builder: &Builder<'_>) { + builder.update_submodule(&Path::new("library").join("stdarch")); + + let target = self.target; + let compiler = builder.compiler(builder.top_stage, builder.config.build); + + let mut cargo = + builder::Cargo::new(builder, compiler, Mode::Std, SourceType::InTree, target, "clippy"); + + std_cargo(builder, target, compiler.stage, &mut cargo); + + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } + + let _guard = + builder.msg_clippy(format_args!("library{}", crate_description(&self.crates)), target); + + run_cargo( + builder, + cargo, + lint_args(builder, IGNORED_RULES_FOR_STD_AND_RUSTC), + &libstd_stamp(builder, compiler, target), + vec![], + true, + false, + ); + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Rustc { + pub target: TargetSelection, + /// Whether to lint only a subset of crates. + crates: Vec, +} + +impl Step for Rustc { + type Output = (); + const ONLY_HOSTS: bool = true; + const DEFAULT: bool = true; + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.crate_or_deps("rustc-main").path("compiler") + } + + fn make_run(run: RunConfig<'_>) { + let crates = run.make_run_crates(Alias::Compiler); + run.builder.ensure(Rustc { target: run.target, crates }); + } + + /// Lints the compiler. + /// + /// This will lint the compiler for a particular stage of the build using + /// the `compiler` targeting the `target` architecture. + fn run(self, builder: &Builder<'_>) { + let compiler = builder.compiler(builder.top_stage, builder.config.build); + let target = self.target; + + if compiler.stage != 0 { + // If we're not in stage 0, then we won't have a std from the beta + // compiler around. That means we need to make sure there's one in + // the sysroot for the compiler to find. Otherwise, we're going to + // fail when building crates that need to generate code (e.g., build + // scripts and their dependencies). + builder.ensure(compile::Std::new(compiler, compiler.host)); + builder.ensure(compile::Std::new(compiler, target)); + } else { + builder.ensure(check::Std::new(target)); + } + + let mut cargo = builder::Cargo::new( + builder, + compiler, + Mode::Rustc, + SourceType::InTree, + target, + "clippy", + ); + + rustc_cargo(builder, &mut cargo, target, &compiler); + + // Explicitly pass -p for all compiler crates -- this will force cargo + // to also lint the tests/benches/examples for these crates, rather + // than just the leaf crate. + for krate in &*self.crates { + cargo.arg("-p").arg(krate); + } + + let _guard = + builder.msg_clippy(format_args!("compiler{}", crate_description(&self.crates)), target); + + run_cargo( + builder, + cargo, + lint_args(builder, IGNORED_RULES_FOR_STD_AND_RUSTC), + &librustc_stamp(builder, compiler, target), + vec![], + true, + false, + ); + } +} + +macro_rules! lint_any { + ($( + $name:ident, $path:expr, $readable_name:expr + $(,lint_by_default = $lint_by_default:expr)* + ; + )+) => { + $( + + #[derive(Debug, Clone, Hash, PartialEq, Eq)] + pub struct $name { + pub target: TargetSelection, + } + + impl Step for $name { + type Output = (); + const DEFAULT: bool = if false $(|| $lint_by_default)* { true } else { false }; + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.path($path) + } + + fn make_run(run: RunConfig<'_>) { + run.builder.ensure($name { + target: run.target, + }); + } + + fn run(self, builder: &Builder<'_>) -> Self::Output { + let compiler = builder.compiler(builder.top_stage, builder.config.build); + let target = self.target; + + builder.ensure(check::Rustc::new(target, builder)); + + let cargo = prepare_tool_cargo( + builder, + compiler, + Mode::ToolRustc, + target, + "clippy", + $path, + SourceType::InTree, + &[], + ); + + let _guard = builder.msg_tool( + Kind::Clippy, + Mode::ToolRustc, + $readable_name, + compiler.stage, + &compiler.host, + &target, + ); + + let stamp = builder + .cargo_out(compiler, Mode::ToolRustc, target) + .join(format!(".{}-check.stamp", stringify!($name).to_lowercase())); + + run_cargo( + builder, + cargo, + lint_args(builder, &[]), + &stamp, + vec![], + true, + false, + ); + } + } + )+ + } +} + +lint_any!( + Bootstrap, "src/bootstrap", "bootstrap"; + BuildHelper, "src/tools/build_helper", "build_helper"; + BuildManifest, "src/tools/build-manifest", "build-manifest"; + CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri"; + Clippy, "src/tools/clippy", "clippy"; + CollectLicenseMetadata, "src/tools/collect-license-metadata", "collect-license-metadata"; + Compiletest, "src/tools/compiletest", "compiletest"; + CoverageDump, "src/tools/coverage-dump", "coverage-dump"; + Jsondocck, "src/tools/jsondocck", "jsondocck"; + Jsondoclint, "src/tools/jsondoclint", "jsondoclint"; + LintDocs, "src/tools/lint-docs", "lint-docs"; + LlvmBitcodeLinker, "src/tools/llvm-bitcode-linker", "llvm-bitcode-linker"; + Miri, "src/tools/miri", "miri"; + MiroptTestTools, "src/tools/miropt-test-tools", "miropt-test-tools"; + OptDist, "src/tools/opt-dist", "opt-dist"; + RemoteTestClient, "src/tools/remote-test-client", "remote-test-client"; + RemoteTestServer, "src/tools/remote-test-server", "remote-test-server"; + Rls, "src/tools/rls", "rls"; + RustAnalyzer, "src/tools/rust-analyzer", "rust-analyzer"; + RustDemangler, "src/tools/rust-demangler", "rust-demangler"; + Rustdoc, "src/tools/rustdoc", "clippy"; + Rustfmt, "src/tools/rustfmt", "rustfmt"; + RustInstaller, "src/tools/rust-installer", "rust-installer"; + Tidy, "src/tools/tidy", "tidy"; +); diff --git a/src/bootstrap/src/core/build_steps/mod.rs b/src/bootstrap/src/core/build_steps/mod.rs index 50d83789be82a..9b7378165de41 100644 --- a/src/bootstrap/src/core/build_steps/mod.rs +++ b/src/bootstrap/src/core/build_steps/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod check; pub(crate) mod clean; +pub(crate) mod clippy; pub(crate) mod compile; pub(crate) mod dist; pub(crate) mod doc; diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 8d1ff2fcb245f..45b1d5a05f35c 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -36,8 +36,9 @@ struct ToolBuild { impl Builder<'_> { #[track_caller] - fn msg_tool( + pub(crate) fn msg_tool( &self, + kind: Kind, mode: Mode, tool: &str, build_stage: u32, @@ -47,7 +48,7 @@ impl Builder<'_> { match mode { // depends on compiler stage, different to host compiler Mode::ToolRustc => self.msg_sysroot_tool( - Kind::Build, + kind, build_stage, format_args!("tool {tool}"), *host, @@ -100,6 +101,7 @@ impl Step for ToolBuild { cargo.allow_features(self.allow_features); } let _guard = builder.msg_tool( + Kind::Build, self.mode, self.tool, self.compiler.stage, @@ -481,6 +483,7 @@ impl Step for Rustdoc { ); let _guard = builder.msg_tool( + Kind::Build, Mode::ToolRustc, "rustdoc", build_compiler.stage, diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index f31bc46d25fcb..499a74be6b151 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -13,9 +13,9 @@ use std::process::Command; use std::sync::OnceLock; use std::time::{Duration, Instant}; -use crate::core::build_steps::llvm; use crate::core::build_steps::tool::{self, SourceType}; use crate::core::build_steps::{check, clean, compile, dist, doc, install, run, setup, test}; +use crate::core::build_steps::{clippy, llvm}; use crate::core::config::flags::{Color, Subcommand}; use crate::core::config::{DryRun, SplitDebuginfo, TargetSelection}; use crate::prepare_behaviour_dump_dir; @@ -672,6 +672,7 @@ impl Kind { Kind::Doc => "Documenting", Kind::Run => "Running", Kind::Suggest => "Suggesting", + Kind::Clippy => "Linting", _ => { let title_letter = self.as_str()[0..1].to_ascii_uppercase(); return format!("{title_letter}{}ing", &self.as_str()[1..]); @@ -726,7 +727,35 @@ impl<'a> Builder<'a> { tool::CoverageDump, tool::LlvmBitcodeLinker ), - Kind::Check | Kind::Clippy | Kind::Fix => describe!( + Kind::Clippy => describe!( + clippy::Std, + clippy::Rustc, + clippy::Bootstrap, + clippy::BuildHelper, + clippy::BuildManifest, + clippy::CargoMiri, + clippy::Clippy, + clippy::CollectLicenseMetadata, + clippy::Compiletest, + clippy::CoverageDump, + clippy::Jsondocck, + clippy::Jsondoclint, + clippy::LintDocs, + clippy::LlvmBitcodeLinker, + clippy::Miri, + clippy::MiroptTestTools, + clippy::OptDist, + clippy::RemoteTestClient, + clippy::RemoteTestServer, + clippy::Rls, + clippy::RustAnalyzer, + clippy::RustDemangler, + clippy::Rustdoc, + clippy::Rustfmt, + clippy::RustInstaller, + clippy::Tidy, + ), + Kind::Check | Kind::Fix => describe!( check::Std, check::Rustc, check::Rustdoc, diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 8cd538953c56d..59e16b6542728 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -1,5 +1,5 @@ use super::{flags::Flags, ChangeIdWrapper, Config}; -use crate::core::build_steps::check::get_clippy_rules_in_order; +use crate::core::build_steps::clippy::get_clippy_rules_in_order; use crate::core::config::{LldMode, TomlConfig}; use clap::CommandFactory; diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 1a8322c0dfd88..e1ae9fe2bf71a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1078,6 +1078,16 @@ impl Build { } } + #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] + fn msg_clippy( + &self, + what: impl Display, + target: impl Into>, + ) -> Option { + self.msg(Kind::Clippy, self.config.stage, what, self.config.build, target) + } + #[must_use = "Groups should not be dropped until the Step finishes running"] #[track_caller] fn msg_check( diff --git a/src/ci/docker/host-x86_64/mingw-check/Dockerfile b/src/ci/docker/host-x86_64/mingw-check/Dockerfile index 6918574814fff..ae8dfadec738a 100644 --- a/src/ci/docker/host-x86_64/mingw-check/Dockerfile +++ b/src/ci/docker/host-x86_64/mingw-check/Dockerfile @@ -46,7 +46,7 @@ ENV SCRIPT python3 ../x.py --stage 2 test src/tools/expand-yaml-anchors && \ # We also skip the x86_64-unknown-linux-gnu target as it is well-tested by other jobs. python3 ../x.py check --stage 0 --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 && \ python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \ - python3 ../x.py clippy compiler -Aclippy::all -Dclippy::correctness && \ + python3 ../x.py clippy compiler library -Aclippy::all -Dclippy::correctness && \ python3 ../x.py build --stage 0 src/tools/build-manifest && \ python3 ../x.py test --stage 0 src/tools/compiletest && \ python3 ../x.py test --stage 0 core alloc std test proc_macro && \