Rollup merge of rust-lang#132649 - klensy:pa-clippy-ci, r=onur-ozkan

add ./x clippy ci This is rebase of rust-lang#126321 also https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/enable.20more.20clippy.20lints.20for.20compiler.20.28and.5Cor.20std.29 for context
GuillaumeGomez · Nov 14, 2024 · 1a1efaf · 1a1efaf
2 parents 5ee347e + cdd948c
commit 1a1efaf
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 29 deletions.
diff --git a/src/bootstrap/src/core/build_steps/clean.rs b/src/bootstrap/src/core/build_steps/clean.rs
@@ -226,6 +226,8 @@ where
         Err(ref e) if e.kind() == ErrorKind::PermissionDenied => {
             let m = t!(path.symlink_metadata());
             let mut p = m.permissions();
+            // this os not unix, so clippy gives FP
+            #[expect(clippy::permissions_set_readonly_false)]
             p.set_readonly(false);
             t!(fs::set_permissions(path, p));
             f(path).unwrap_or_else(|e| {

diff --git a/src/bootstrap/src/core/build_steps/clippy.rs b/src/bootstrap/src/core/build_steps/clippy.rs
@@ -20,14 +20,12 @@ const IGNORED_RULES_FOR_STD_AND_RUSTC: &[&str] = &[
     "wrong_self_convention",
 ];
 
-fn lint_args(builder: &Builder<'_>, ignored_rules: &[&str]) -> Vec<String> {
+fn lint_args(builder: &Builder<'_>, config: &LintConfig, ignored_rules: &[&str]) -> Vec<String> {
     fn strings<'a>(arr: &'a [&str]) -> impl Iterator<Item = String> + 'a {
         arr.iter().copied().map(String::from)
     }
 
-    let Subcommand::Clippy { fix, allow_dirty, allow_staged, allow, deny, warn, forbid } =
-        &builder.config.cmd
-    else {
+    let Subcommand::Clippy { fix, allow_dirty, allow_staged, .. } = &builder.config.cmd else {
         unreachable!("clippy::lint_args can only be called from `clippy` subcommands.");
     };
 
@@ -53,12 +51,12 @@ fn lint_args(builder: &Builder<'_>, ignored_rules: &[&str]) -> Vec<String> {
 
     args.extend(strings(&["--"]));
 
-    if deny.is_empty() && forbid.is_empty() {
+    if config.deny.is_empty() && config.forbid.is_empty() {
         args.extend(strings(&["--cap-lints", "warn"]));
     }
 
     let all_args = std::env::args().collect::<Vec<_>>();
-    args.extend(get_clippy_rules_in_order(&all_args, allow, deny, warn, forbid));
+    args.extend(get_clippy_rules_in_order(&all_args, config));
 
     args.extend(ignored_rules.iter().map(|lint| format!("-Aclippy::{}", lint)));
     args.extend(builder.config.free_args.clone());
@@ -68,21 +66,17 @@ fn lint_args(builder: &Builder<'_>, ignored_rules: &[&str]) -> Vec<String> {
 /// 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<String> {
+pub fn get_clippy_rules_in_order(all_args: &[String], config: &LintConfig) -> Vec<String> {
     let mut result = vec![];
 
     for (prefix, item) in
-        [("-A", allow_rules), ("-D", deny_rules), ("-W", warn_rules), ("-F", forbid_rules)]
+        [("-A", &config.allow), ("-D", &config.deny), ("-W", &config.warn), ("-F", &config.forbid)]
     {
         item.iter().for_each(|v| {
             let rule = format!("{prefix}{v}");
-            let position = all_args.iter().position(|t| t == &rule || t == v).unwrap();
+            // Arguments added by bootstrap in LintConfig won't show up in the all_args list, so
+            // put them at the end of the command line.
+            let position = all_args.iter().position(|t| t == &rule || t == v).unwrap_or(usize::MAX);
             result.push((position, rule));
         });
     }
@@ -91,9 +85,42 @@ pub(crate) fn get_clippy_rules_in_order(
     result.into_iter().map(|v| v.1).collect()
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct LintConfig {
+    pub allow: Vec<String>,
+    pub warn: Vec<String>,
+    pub deny: Vec<String>,
+    pub forbid: Vec<String>,
+}
+
+impl LintConfig {
+    fn new(builder: &Builder<'_>) -> Self {
+        match builder.config.cmd.clone() {
+            Subcommand::Clippy { allow, deny, warn, forbid, .. } => {
+                Self { allow, warn, deny, forbid }
+            }
+            _ => unreachable!("LintConfig can only be called from `clippy` subcommands."),
+        }
+    }
+
+    fn merge(&self, other: &Self) -> Self {
+        let merged = |self_attr: &[String], other_attr: &[String]| -> Vec<String> {
+            self_attr.iter().cloned().chain(other_attr.iter().cloned()).collect()
+        };
+        // This is written this way to ensure we get a compiler error if we add a new field.
+        Self {
+            allow: merged(&self.allow, &other.allow),
+            warn: merged(&self.warn, &other.warn),
+            deny: merged(&self.deny, &other.deny),
+            forbid: merged(&self.forbid, &other.forbid),
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Std {
     pub target: TargetSelection,
+    config: LintConfig,
     /// Whether to lint only a subset of crates.
     crates: Vec<String>,
 }
@@ -108,7 +135,8 @@ impl Step for Std {
 
     fn make_run(run: RunConfig<'_>) {
         let crates = std_crates_for_run_make(&run);
-        run.builder.ensure(Std { target: run.target, crates });
+        let config = LintConfig::new(run.builder);
+        run.builder.ensure(Std { target: run.target, config, crates });
     }
 
     fn run(self, builder: &Builder<'_>) {
@@ -138,7 +166,7 @@ impl Step for Std {
         run_cargo(
             builder,
             cargo,
-            lint_args(builder, IGNORED_RULES_FOR_STD_AND_RUSTC),
+            lint_args(builder, &self.config, IGNORED_RULES_FOR_STD_AND_RUSTC),
             &libstd_stamp(builder, compiler, target),
             vec![],
             true,
@@ -150,6 +178,7 @@ impl Step for Std {
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Rustc {
     pub target: TargetSelection,
+    config: LintConfig,
     /// Whether to lint only a subset of crates.
     crates: Vec<String>,
 }
@@ -165,7 +194,8 @@ impl Step for Rustc {
 
     fn make_run(run: RunConfig<'_>) {
         let crates = run.make_run_crates(Alias::Compiler);
-        run.builder.ensure(Rustc { target: run.target, crates });
+        let config = LintConfig::new(run.builder);
+        run.builder.ensure(Rustc { target: run.target, config, crates });
     }
 
     /// Lints the compiler.
@@ -212,7 +242,7 @@ impl Step for Rustc {
         run_cargo(
             builder,
             cargo,
-            lint_args(builder, IGNORED_RULES_FOR_STD_AND_RUSTC),
+            lint_args(builder, &self.config, IGNORED_RULES_FOR_STD_AND_RUSTC),
             &librustc_stamp(builder, compiler, target),
             vec![],
             true,
@@ -232,6 +262,7 @@ macro_rules! lint_any {
         #[derive(Debug, Clone, Hash, PartialEq, Eq)]
         pub struct $name {
             pub target: TargetSelection,
+            config: LintConfig,
         }
 
         impl Step for $name {
@@ -243,8 +274,10 @@ macro_rules! lint_any {
             }
 
             fn make_run(run: RunConfig<'_>) {
+                let config = LintConfig::new(run.builder);
                 run.builder.ensure($name {
                     target: run.target,
+                    config,
                 });
             }
 
@@ -281,7 +314,7 @@ macro_rules! lint_any {
                 run_cargo(
                     builder,
                     cargo,
-                    lint_args(builder, &[]),
+                    lint_args(builder, &self.config, &[]),
                     &stamp,
                     vec![],
                     true,
@@ -319,3 +352,58 @@ lint_any!(
     Tidy, "src/tools/tidy", "tidy";
     TestFloatParse, "src/etc/test-float-parse", "test-float-parse";
 );
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct CI {
+    target: TargetSelection,
+    config: LintConfig,
+}
+
+impl Step for CI {
+    type Output = ();
+    const DEFAULT: bool = false;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.alias("ci")
+    }
+
+    fn make_run(run: RunConfig<'_>) {
+        let config = LintConfig::new(run.builder);
+        run.builder.ensure(CI { target: run.target, config });
+    }
+
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
+        builder.ensure(Bootstrap {
+            target: self.target,
+            config: self.config.merge(&LintConfig {
+                allow: vec![],
+                warn: vec![],
+                deny: vec!["warnings".into()],
+                forbid: vec![],
+            }),
+        });
+        let library_clippy_cfg = LintConfig {
+            allow: vec!["clippy::all".into()],
+            warn: vec![],
+            deny: vec!["clippy::correctness".into()],
+            forbid: vec![],
+        };
+        let compiler_clippy_cfg = LintConfig {
+            allow: vec!["clippy::all".into()],
+            warn: vec![],
+            deny: vec!["clippy::correctness".into(), "clippy::clone_on_ref_ptr".into()],
+            forbid: vec![],
+        };
+
+        builder.ensure(Std {
+            target: self.target,
+            config: self.config.merge(&library_clippy_cfg),
+            crates: vec![],
+        });
+        builder.ensure(Rustc {
+            target: self.target,
+            config: self.config.merge(&compiler_clippy_cfg),
+            crates: vec![],
+        });
+    }
+}
diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs
@@ -839,6 +839,7 @@ impl<'a> Builder<'a> {
                 clippy::RustInstaller,
                 clippy::TestFloatParse,
                 clippy::Tidy,
+                clippy::CI,
             ),
             Kind::Check | Kind::Fix => describe!(
                 check::Std,

diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs
@@ -9,7 +9,7 @@ use serde::Deserialize;
 
 use super::flags::Flags;
 use super::{ChangeIdWrapper, Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS};
-use crate::core::build_steps::clippy::get_clippy_rules_in_order;
+use crate::core::build_steps::clippy::{LintConfig, get_clippy_rules_in_order};
 use crate::core::build_steps::llvm;
 use crate::core::config::{LldMode, Target, TargetSelection, TomlConfig};
 
@@ -309,9 +309,10 @@ fn order_of_clippy_rules() {
     ];
     let config = Config::parse(Flags::parse(&args));
 
-    let actual = match &config.cmd {
+    let actual = match config.cmd.clone() {
         crate::Subcommand::Clippy { allow, deny, warn, forbid, .. } => {
-            get_clippy_rules_in_order(&args, &allow, &deny, &warn, &forbid)
+            let cfg = LintConfig { allow, deny, warn, forbid };
+            get_clippy_rules_in_order(&args, &cfg)
         }
         _ => panic!("invalid subcommand"),
     };
@@ -332,9 +333,10 @@ fn clippy_rule_separate_prefix() {
         vec!["clippy".to_string(), "-A clippy:all".to_string(), "-W clippy::style".to_string()];
     let config = Config::parse(Flags::parse(&args));
 
-    let actual = match &config.cmd {
+    let actual = match config.cmd.clone() {
         crate::Subcommand::Clippy { allow, deny, warn, forbid, .. } => {
-            get_clippy_rules_in_order(&args, &allow, &deny, &warn, &forbid)
+            let cfg = LintConfig { allow, deny, warn, forbid };
+            get_clippy_rules_in_order(&args, &cfg)
         }
         _ => panic!("invalid subcommand"),
     };

diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs
@@ -145,7 +145,7 @@ pub fn symlink_dir(config: &Config, original: &Path, link: &Path) -> io::Result<
 
     #[cfg(windows)]
     fn symlink_dir_inner(target: &Path, junction: &Path) -> io::Result<()> {
-        junction::create(&target, &junction)
+        junction::create(target, junction)
     }
 }
 

diff --git a/src/ci/docker/host-x86_64/mingw-check/Dockerfile b/src/ci/docker/host-x86_64/mingw-check/Dockerfile
@@ -50,9 +50,7 @@ ENV SCRIPT \
            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 && \
            /scripts/check-default-config-profiles.sh && \
            python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu && \
-           python3 ../x.py clippy bootstrap -Dwarnings && \
-           python3 ../x.py clippy library -Aclippy::all -Dclippy::correctness && \
-           python3 ../x.py clippy compiler -Aclippy::all -Dclippy::correctness -Dclippy::clone_on_ref_ptr && \
+           python3 ../x.py clippy ci && \
            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 && \