From 6030997800b681cee91acf6ef8aba199d03041a6 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 23 May 2025 08:52:52 -0400 Subject: [PATCH 1/7] Remove `verify_inside_clippy_dir` since we always set the current directory. --- clippy_dev/src/setup/git_hook.rs | 6 ------ clippy_dev/src/setup/mod.rs | 20 -------------------- clippy_dev/src/setup/toolchain.rs | 6 ------ clippy_dev/src/setup/vscode.rs | 6 ------ 4 files changed, 38 deletions(-) diff --git a/clippy_dev/src/setup/git_hook.rs b/clippy_dev/src/setup/git_hook.rs index c7c53bc69d0b..c5a1e8264c7f 100644 --- a/clippy_dev/src/setup/git_hook.rs +++ b/clippy_dev/src/setup/git_hook.rs @@ -1,8 +1,6 @@ use std::fs; use std::path::Path; -use super::verify_inside_clippy_dir; - /// Rusts setup uses `git rev-parse --git-common-dir` to get the root directory of the repo. /// I've decided against this for the sake of simplicity and to make sure that it doesn't install /// the hook if `clippy_dev` would be used in the rust tree. The hook also references this tool @@ -35,10 +33,6 @@ pub fn install_hook(force_override: bool) { } fn check_precondition(force_override: bool) -> bool { - if !verify_inside_clippy_dir() { - return false; - } - // Make sure that we can find the git repository let git_path = Path::new(REPO_GIT_DIR); if !git_path.exists() || !git_path.is_dir() { diff --git a/clippy_dev/src/setup/mod.rs b/clippy_dev/src/setup/mod.rs index b0d318146391..5e938fff126d 100644 --- a/clippy_dev/src/setup/mod.rs +++ b/clippy_dev/src/setup/mod.rs @@ -2,23 +2,3 @@ pub mod git_hook; pub mod intellij; pub mod toolchain; pub mod vscode; - -use std::path::Path; - -const CLIPPY_DEV_DIR: &str = "clippy_dev"; - -/// This function verifies that the tool is being executed in the clippy directory. -/// This is useful to ensure that setups only modify Clippy's resources. The verification -/// is done by checking that `clippy_dev` is a sub directory of the current directory. -/// -/// It will print an error message and return `false` if the directory could not be -/// verified. -fn verify_inside_clippy_dir() -> bool { - let path = Path::new(CLIPPY_DEV_DIR); - if path.exists() && path.is_dir() { - true - } else { - eprintln!("error: unable to verify that the working directory is clippy's directory"); - false - } -} diff --git a/clippy_dev/src/setup/toolchain.rs b/clippy_dev/src/setup/toolchain.rs index ecd80215f7e8..c70b56461a7f 100644 --- a/clippy_dev/src/setup/toolchain.rs +++ b/clippy_dev/src/setup/toolchain.rs @@ -8,13 +8,7 @@ use walkdir::WalkDir; use crate::utils::exit_if_err; -use super::verify_inside_clippy_dir; - pub fn create(standalone: bool, force: bool, release: bool, name: &str) { - if !verify_inside_clippy_dir() { - return; - } - let rustup_home = std::env::var("RUSTUP_HOME").unwrap(); let toolchain = std::env::var("RUSTUP_TOOLCHAIN").unwrap(); diff --git a/clippy_dev/src/setup/vscode.rs b/clippy_dev/src/setup/vscode.rs index a37c873eed4f..a24aef65991f 100644 --- a/clippy_dev/src/setup/vscode.rs +++ b/clippy_dev/src/setup/vscode.rs @@ -1,8 +1,6 @@ use std::fs; use std::path::Path; -use super::verify_inside_clippy_dir; - const VSCODE_DIR: &str = ".vscode"; const TASK_SOURCE_FILE: &str = "util/etc/vscode-tasks.json"; const TASK_TARGET_FILE: &str = ".vscode/tasks.json"; @@ -22,10 +20,6 @@ pub fn install_tasks(force_override: bool) { } fn check_install_precondition(force_override: bool) -> bool { - if !verify_inside_clippy_dir() { - return false; - } - let vs_dir_path = Path::new(VSCODE_DIR); if vs_dir_path.exists() { // verify the target will be valid From fc8bf97095d6578be5464c4f10e1f11971738d80 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 23 May 2025 13:19:51 -0400 Subject: [PATCH 2/7] Rename `exit_if_err` to `run_exit_on_err` --- clippy_dev/src/dogfood.rs | 48 ++++++++++++++----------------- clippy_dev/src/lint.rs | 36 +++++++++++------------ clippy_dev/src/setup/toolchain.rs | 12 ++++---- clippy_dev/src/utils.rs | 26 ++++++++--------- 4 files changed, 55 insertions(+), 67 deletions(-) diff --git a/clippy_dev/src/dogfood.rs b/clippy_dev/src/dogfood.rs index 7e9d92458d05..c1ee3633d3ed 100644 --- a/clippy_dev/src/dogfood.rs +++ b/clippy_dev/src/dogfood.rs @@ -1,4 +1,5 @@ -use crate::utils::exit_if_err; +use crate::utils::run_exit_on_err; +use itertools::Itertools; use std::process::Command; /// # Panics @@ -6,30 +7,23 @@ use std::process::Command; /// Panics if unable to run the dogfood test #[allow(clippy::fn_params_excessive_bools)] pub fn dogfood(fix: bool, allow_dirty: bool, allow_staged: bool, allow_no_vcs: bool) { - let mut cmd = Command::new("cargo"); - - cmd.args(["test", "--test", "dogfood"]) - .args(["--features", "internal"]) - .args(["--", "dogfood_clippy", "--nocapture"]); - - let mut dogfood_args = Vec::new(); - if fix { - dogfood_args.push("--fix"); - } - - if allow_dirty { - dogfood_args.push("--allow-dirty"); - } - - if allow_staged { - dogfood_args.push("--allow-staged"); - } - - if allow_no_vcs { - dogfood_args.push("--allow-no-vcs"); - } - - cmd.env("__CLIPPY_DOGFOOD_ARGS", dogfood_args.join(" ")); - - exit_if_err(cmd.status()); + run_exit_on_err( + "cargo test", + Command::new("cargo") + .args(["test", "--test", "dogfood"]) + .args(["--features", "internal"]) + .args(["--", "dogfood_clippy", "--nocapture"]) + .env( + "__CLIPPY_DOGFOOD_ARGS", + [ + if fix { "--fix" } else { "" }, + if allow_dirty { "--allow-dirty" } else { "" }, + if allow_staged { "--allow-staged" } else { "" }, + if allow_no_vcs { "--allow-no-vcs" } else { "" }, + ] + .iter() + .filter(|x| !x.is_empty()) + .join(" "), + ), + ); } diff --git a/clippy_dev/src/lint.rs b/clippy_dev/src/lint.rs index 0d66f167a386..f450bf5d8a2f 100644 --- a/clippy_dev/src/lint.rs +++ b/clippy_dev/src/lint.rs @@ -1,4 +1,4 @@ -use crate::utils::{cargo_clippy_path, exit_if_err}; +use crate::utils::{cargo_clippy_path, run_exit_on_err}; use std::process::{self, Command}; use std::{env, fs}; @@ -12,8 +12,9 @@ pub fn run<'a>(path: &str, edition: &str, args: impl Iterator }; if is_file { - exit_if_err( - Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())) + run_exit_on_err( + "cargo run", + Command::new(env::var("CARGO").unwrap_or("cargo".into())) .args(["run", "--bin", "clippy-driver", "--"]) .args(["-L", "./target/debug"]) .args(["-Z", "no-codegen"]) @@ -21,24 +22,21 @@ pub fn run<'a>(path: &str, edition: &str, args: impl Iterator .arg(path) .args(args) // Prevent rustc from creating `rustc-ice-*` files the console output is enough. - .env("RUSTC_ICE", "0") - .status(), + .env("RUSTC_ICE", "0"), ); } else { - exit_if_err( - Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())) - .arg("build") - .status(), + run_exit_on_err( + "cargo build", + Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())).arg("build"), + ); + run_exit_on_err( + "cargo clippy", + Command::new(cargo_clippy_path()) + .arg("clippy") + .args(args) + // Prevent rustc from creating `rustc-ice-*` files the console output is enough. + .env("RUSTC_ICE", "0") + .current_dir(path), ); - - let status = Command::new(cargo_clippy_path()) - .arg("clippy") - .args(args) - // Prevent rustc from creating `rustc-ice-*` files the console output is enough. - .env("RUSTC_ICE", "0") - .current_dir(path) - .status(); - - exit_if_err(status); } } diff --git a/clippy_dev/src/setup/toolchain.rs b/clippy_dev/src/setup/toolchain.rs index c70b56461a7f..4c1db4e8a67d 100644 --- a/clippy_dev/src/setup/toolchain.rs +++ b/clippy_dev/src/setup/toolchain.rs @@ -1,3 +1,4 @@ +use crate::utils::run_exit_on_err; use std::env::consts::EXE_SUFFIX; use std::env::current_dir; use std::ffi::OsStr; @@ -6,8 +7,6 @@ use std::path::{Path, PathBuf}; use std::process::Command; use walkdir::WalkDir; -use crate::utils::exit_if_err; - pub fn create(standalone: bool, force: bool, release: bool, name: &str) { let rustup_home = std::env::var("RUSTUP_HOME").unwrap(); let toolchain = std::env::var("RUSTUP_TOOLCHAIN").unwrap(); @@ -45,11 +44,10 @@ pub fn create(standalone: bool, force: bool, release: bool, name: &str) { } } - let status = Command::new("cargo") - .arg("build") - .args(release.then_some("--release")) - .status(); - exit_if_err(status); + run_exit_on_err( + "cargo build", + Command::new("cargo").arg("build").args(release.then_some("--release")), + ); install_bin("cargo-clippy", &dest, standalone, release); install_bin("clippy-driver", &dest, standalone, release); diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index 89962a110341..f518460f5829 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -8,7 +8,7 @@ use std::ffi::OsStr; use std::fs::{self, OpenOptions}; use std::io::{self, Read as _, Seek as _, SeekFrom, Write}; use std::path::{Path, PathBuf}; -use std::process::{self, Command, ExitStatus, Stdio}; +use std::process::{self, Command, Stdio}; use std::{env, thread}; use walkdir::WalkDir; @@ -288,19 +288,6 @@ impl ClippyInfo { } } -/// # Panics -/// Panics if given command result was failed. -pub fn exit_if_err(status: io::Result) { - match status.expect("failed to run command").code() { - Some(0) => {}, - Some(n) => process::exit(n), - None => { - eprintln!("Killed by signal"); - process::exit(1); - }, - } -} - #[derive(Clone, Copy)] pub enum UpdateStatus { Unchanged, @@ -653,6 +640,17 @@ pub fn write_file(path: &Path, contents: &str) { expect_action(fs::write(path, contents), ErrAction::Write, path); } +pub fn run_exit_on_err(path: &(impl AsRef + ?Sized), cmd: &mut Command) { + match expect_action(cmd.status(), ErrAction::Run, path.as_ref()).code() { + Some(0) => {}, + Some(n) => process::exit(n), + None => { + eprintln!("{} killed by signal", path.as_ref().display()); + process::exit(1); + }, + } +} + #[must_use] pub fn run_with_output(path: &(impl AsRef + ?Sized), cmd: &mut Command) -> Vec { fn f(path: &Path, cmd: &mut Command) -> Vec { From ed50c8a0e987511d10ec11936f9e88fb297b18b4 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 23 May 2025 14:22:45 -0400 Subject: [PATCH 3/7] Make `dev serve` actually detect all updates --- clippy_dev/src/serve.rs | 109 ++++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 38 deletions(-) diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index 498ffeba9d67..73d9a1e82373 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -1,7 +1,11 @@ +use crate::utils::{ErrAction, expect_action}; +use core::fmt::Display; +use core::mem; use std::path::Path; use std::process::Command; use std::time::{Duration, SystemTime}; -use std::{env, thread}; +use std::{env, fs, thread}; +use walkdir::WalkDir; #[cfg(windows)] const PYTHON: &str = "python"; @@ -18,56 +22,85 @@ pub fn run(port: u16, lint: Option) -> ! { Some(lint) => format!("http://localhost:{port}/#{lint}"), }); + let mut last_update = mtime("util/gh-pages/index.html"); loop { - let index_time = mtime("util/gh-pages/index.html"); - let times = [ - "clippy_lints/src", - "util/gh-pages/index_template.html", - "tests/compile-test.rs", - ] - .map(mtime); - - if times.iter().any(|&time| index_time < time) { - Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())) - .arg("collect-metadata") - .spawn() - .unwrap() - .wait() - .unwrap(); + if is_metadata_outdated(mem::replace(&mut last_update, SystemTime::now())) { + // Ignore the command result; we'll fall back to displaying the old metadata. + let _ = expect_action( + Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())) + .arg("collect-metadata") + .status(), + ErrAction::Run, + "cargo collect-metadata", + ); + last_update = SystemTime::now(); } + + // Only start the web server the first time around. if let Some(url) = url.take() { thread::spawn(move || { - let mut child = Command::new(PYTHON) - .arg("-m") - .arg("http.server") - .arg(port.to_string()) - .current_dir("util/gh-pages") - .spawn() - .unwrap(); + let mut child = expect_action( + Command::new(PYTHON) + .args(["-m", "http.server", port.to_string().as_str()]) + .current_dir("util/gh-pages") + .spawn(), + ErrAction::Run, + "python -m http.server", + ); // Give some time for python to start thread::sleep(Duration::from_millis(500)); // Launch browser after first export.py has completed and http.server is up let _result = opener::open(url); - child.wait().unwrap(); + expect_action(child.wait(), ErrAction::Run, "python -m http.server"); }); } + + // Delay to avoid updating the metadata too aggressively. thread::sleep(Duration::from_millis(1000)); } } -fn mtime(path: impl AsRef) -> SystemTime { - let path = path.as_ref(); - if path.is_dir() { - path.read_dir() - .into_iter() - .flatten() - .flatten() - .map(|entry| mtime(entry.path())) - .max() - .unwrap_or(SystemTime::UNIX_EPOCH) - } else { - path.metadata() - .and_then(|metadata| metadata.modified()) - .unwrap_or(SystemTime::UNIX_EPOCH) +fn log_err_and_continue(res: Result, path: &Path) -> Option { + match res { + Ok(x) => Some(x), + Err(ref e) => { + eprintln!("error reading `{}`: {e}", path.display()); + None + }, } } + +fn mtime(path: &str) -> SystemTime { + log_err_and_continue(fs::metadata(path), path.as_ref()) + .and_then(|metadata| log_err_and_continue(metadata.modified(), path.as_ref())) + .unwrap_or(SystemTime::UNIX_EPOCH) +} + +fn is_metadata_outdated(time: SystemTime) -> bool { + // Ignore all IO errors here. We don't want to stop them from hosting the server. + if time < mtime("util/gh-pages/index_template.html") || time < mtime("tests/compile-test.rs") { + return true; + } + let Some(dir) = log_err_and_continue(fs::read_dir("."), ".".as_ref()) else { + return false; + }; + dir.map_while(|e| log_err_and_continue(e, ".".as_ref())).any(|e| { + let name = e.file_name(); + let name_bytes = name.as_encoded_bytes(); + if (name_bytes.starts_with(b"clippy_lints") && name_bytes != b"clippy_lints_internal") + || name_bytes == b"clippy_config" + { + WalkDir::new(&name) + .into_iter() + .map_while(|e| log_err_and_continue(e, name.as_ref())) + .filter(|e| e.file_type().is_file()) + .filter_map(|e| { + log_err_and_continue(e.metadata(), e.path()) + .and_then(|m| log_err_and_continue(m.modified(), e.path())) + }) + .any(|ftime| time < ftime) + } else { + false + } + }) +} From 46ff0a85657fcc08f36fec0a06455536c3296121 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 23 May 2025 14:32:10 -0400 Subject: [PATCH 4/7] Add helper function for creating cargo commands. --- clippy_dev/src/dogfood.rs | 5 ++--- clippy_dev/src/lint.rs | 11 ++++------- clippy_dev/src/serve.rs | 8 +++----- clippy_dev/src/setup/toolchain.rs | 5 ++--- clippy_dev/src/utils.rs | 10 ++++++++++ 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/clippy_dev/src/dogfood.rs b/clippy_dev/src/dogfood.rs index c1ee3633d3ed..d0fca952b932 100644 --- a/clippy_dev/src/dogfood.rs +++ b/clippy_dev/src/dogfood.rs @@ -1,6 +1,5 @@ -use crate::utils::run_exit_on_err; +use crate::utils::{cargo_cmd, run_exit_on_err}; use itertools::Itertools; -use std::process::Command; /// # Panics /// @@ -9,7 +8,7 @@ use std::process::Command; pub fn dogfood(fix: bool, allow_dirty: bool, allow_staged: bool, allow_no_vcs: bool) { run_exit_on_err( "cargo test", - Command::new("cargo") + cargo_cmd() .args(["test", "--test", "dogfood"]) .args(["--features", "internal"]) .args(["--", "dogfood_clippy", "--nocapture"]) diff --git a/clippy_dev/src/lint.rs b/clippy_dev/src/lint.rs index f450bf5d8a2f..d8d3e39116b9 100644 --- a/clippy_dev/src/lint.rs +++ b/clippy_dev/src/lint.rs @@ -1,6 +1,6 @@ -use crate::utils::{cargo_clippy_path, run_exit_on_err}; +use crate::utils::{cargo_clippy_path, cargo_cmd, run_exit_on_err}; +use std::fs; use std::process::{self, Command}; -use std::{env, fs}; pub fn run<'a>(path: &str, edition: &str, args: impl Iterator) { let is_file = match fs::metadata(path) { @@ -14,7 +14,7 @@ pub fn run<'a>(path: &str, edition: &str, args: impl Iterator if is_file { run_exit_on_err( "cargo run", - Command::new(env::var("CARGO").unwrap_or("cargo".into())) + cargo_cmd() .args(["run", "--bin", "clippy-driver", "--"]) .args(["-L", "./target/debug"]) .args(["-Z", "no-codegen"]) @@ -25,10 +25,7 @@ pub fn run<'a>(path: &str, edition: &str, args: impl Iterator .env("RUSTC_ICE", "0"), ); } else { - run_exit_on_err( - "cargo build", - Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())).arg("build"), - ); + run_exit_on_err("cargo build", cargo_cmd().arg("build")); run_exit_on_err( "cargo clippy", Command::new(cargo_clippy_path()) diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index 73d9a1e82373..d9e018133813 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -1,10 +1,10 @@ -use crate::utils::{ErrAction, expect_action}; +use crate::utils::{ErrAction, cargo_cmd, expect_action}; use core::fmt::Display; use core::mem; use std::path::Path; use std::process::Command; use std::time::{Duration, SystemTime}; -use std::{env, fs, thread}; +use std::{fs, thread}; use walkdir::WalkDir; #[cfg(windows)] @@ -27,9 +27,7 @@ pub fn run(port: u16, lint: Option) -> ! { if is_metadata_outdated(mem::replace(&mut last_update, SystemTime::now())) { // Ignore the command result; we'll fall back to displaying the old metadata. let _ = expect_action( - Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into())) - .arg("collect-metadata") - .status(), + cargo_cmd().arg("collect-metadata").status(), ErrAction::Run, "cargo collect-metadata", ); diff --git a/clippy_dev/src/setup/toolchain.rs b/clippy_dev/src/setup/toolchain.rs index 4c1db4e8a67d..c64ae4ef3c36 100644 --- a/clippy_dev/src/setup/toolchain.rs +++ b/clippy_dev/src/setup/toolchain.rs @@ -1,10 +1,9 @@ -use crate::utils::run_exit_on_err; +use crate::utils::{cargo_cmd, run_exit_on_err}; use std::env::consts::EXE_SUFFIX; use std::env::current_dir; use std::ffi::OsStr; use std::fs; use std::path::{Path, PathBuf}; -use std::process::Command; use walkdir::WalkDir; pub fn create(standalone: bool, force: bool, release: bool, name: &str) { @@ -46,7 +45,7 @@ pub fn create(standalone: bool, force: bool, release: bool, name: &str) { run_exit_on_err( "cargo build", - Command::new("cargo").arg("build").args(release.then_some("--release")), + cargo_cmd().arg("build").args(release.then_some("--release")), ); install_bin("cargo-clippy", &dest, standalone, release); diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index f518460f5829..4b77a49a9f6e 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -130,6 +130,16 @@ pub fn cargo_clippy_path() -> PathBuf { path } +/// Creates a `Command` for running cargo. +#[must_use] +pub fn cargo_cmd() -> Command { + if let Some(path) = env::var_os("CARGO") { + Command::new(path) + } else { + Command::new("cargo") + } +} + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub struct Version { pub major: u16, From ed1088eec4235e7bb86ff9ea4c231c5b5472f7b7 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 23 May 2025 14:37:52 -0400 Subject: [PATCH 5/7] Use `track_caller` more for better error spans in `clippy_dev` --- clippy_dev/src/utils.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index 4b77a49a9f6e..0c5020c6982b 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -338,6 +338,7 @@ pub struct FileUpdater { dst_buf: String, } impl FileUpdater { + #[track_caller] fn update_file_checked_inner( &mut self, tool: &str, @@ -361,6 +362,7 @@ impl FileUpdater { } } + #[track_caller] fn update_file_inner(&mut self, path: &Path, update: &mut dyn FnMut(&Path, &str, &mut String) -> UpdateStatus) { let mut file = File::open(path, OpenOptions::new().read(true).write(true)); file.read_to_cleared_string(&mut self.src_buf); @@ -370,6 +372,7 @@ impl FileUpdater { } } + #[track_caller] pub fn update_file_checked( &mut self, tool: &str, @@ -380,6 +383,7 @@ impl FileUpdater { self.update_file_checked_inner(tool, mode, path.as_ref(), update); } + #[track_caller] pub fn update_file( &mut self, path: impl AsRef, @@ -598,6 +602,7 @@ impl<'txt> RustSearcher<'txt> { } } +#[track_caller] #[expect(clippy::must_use_candidate)] pub fn try_rename_file(old_name: &Path, new_name: &Path) -> bool { match OpenOptions::new().create_new(true).write(true).open(new_name) { @@ -620,6 +625,7 @@ pub fn try_rename_file(old_name: &Path, new_name: &Path) -> bool { } } +#[track_caller] #[expect(clippy::must_use_candidate)] pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { match fs::create_dir(new_name) { @@ -646,10 +652,12 @@ pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { } } +#[track_caller] pub fn write_file(path: &Path, contents: &str) { expect_action(fs::write(path, contents), ErrAction::Write, path); } +#[track_caller] pub fn run_exit_on_err(path: &(impl AsRef + ?Sized), cmd: &mut Command) { match expect_action(cmd.status(), ErrAction::Run, path.as_ref()).code() { Some(0) => {}, @@ -661,6 +669,7 @@ pub fn run_exit_on_err(path: &(impl AsRef + ?Sized), cmd: &mut Command) { } } +#[track_caller] #[must_use] pub fn run_with_output(path: &(impl AsRef + ?Sized), cmd: &mut Command) -> Vec { fn f(path: &Path, cmd: &mut Command) -> Vec { @@ -746,6 +755,7 @@ pub fn split_args_for_threads( } } +#[track_caller] #[expect(clippy::must_use_candidate)] pub fn delete_file_if_exists(path: &Path) -> bool { match fs::remove_file(path) { @@ -755,6 +765,7 @@ pub fn delete_file_if_exists(path: &Path) -> bool { } } +#[track_caller] pub fn delete_dir_if_exists(path: &Path) { match fs::remove_dir_all(path) { Ok(()) => {}, From e624b7759282d5a76e585ff198e997e7b4ccd072 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 24 May 2025 14:17:33 -0400 Subject: [PATCH 6/7] Inline `cargo_clippy_path` and add a note about removing it when possible --- clippy_dev/src/lint.rs | 29 +++++++++++++++++------------ clippy_dev/src/utils.rs | 17 ----------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/clippy_dev/src/lint.rs b/clippy_dev/src/lint.rs index d8d3e39116b9..2d9f563cdae2 100644 --- a/clippy_dev/src/lint.rs +++ b/clippy_dev/src/lint.rs @@ -1,16 +1,14 @@ -use crate::utils::{cargo_clippy_path, cargo_cmd, run_exit_on_err}; -use std::fs; -use std::process::{self, Command}; +use crate::utils::{ErrAction, cargo_cmd, expect_action, run_exit_on_err}; +use std::process::Command; +use std::{env, fs}; -pub fn run<'a>(path: &str, edition: &str, args: impl Iterator) { - let is_file = match fs::metadata(path) { - Ok(metadata) => metadata.is_file(), - Err(e) => { - eprintln!("Failed to read {path}: {e:?}"); - process::exit(1); - }, - }; +#[cfg(not(windows))] +static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; +#[cfg(windows)] +static CARGO_CLIPPY_EXE: &str = "cargo-clippy.exe"; +pub fn run<'a>(path: &str, edition: &str, args: impl Iterator) { + let is_file = expect_action(fs::metadata(path), ErrAction::Read, path).is_file(); if is_file { run_exit_on_err( "cargo run", @@ -25,10 +23,17 @@ pub fn run<'a>(path: &str, edition: &str, args: impl Iterator .env("RUSTC_ICE", "0"), ); } else { + // Ideally this would just be `cargo run`, but the working directory needs to be + // set to clippy's directory when building, and the target project's directory + // when running clippy. `cargo` can only set a single working directory for both + // when using `run`. run_exit_on_err("cargo build", cargo_cmd().arg("build")); + + let mut exe = env::current_exe().expect("failed to get current executable name"); + exe.set_file_name(CARGO_CLIPPY_EXE); run_exit_on_err( "cargo clippy", - Command::new(cargo_clippy_path()) + Command::new(exe) .arg("clippy") .args(args) // Prevent rustc from creating `rustc-ice-*` files the console output is enough. diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index 0c5020c6982b..dfa2d186097b 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -12,11 +12,6 @@ use std::process::{self, Command, Stdio}; use std::{env, thread}; use walkdir::WalkDir; -#[cfg(not(windows))] -static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; -#[cfg(windows)] -static CARGO_CLIPPY_EXE: &str = "cargo-clippy.exe"; - #[derive(Clone, Copy)] pub enum ErrAction { Open, @@ -118,18 +113,6 @@ impl<'a> File<'a> { } } -/// Returns the path to the `cargo-clippy` binary -/// -/// # Panics -/// -/// Panics if the path of current executable could not be retrieved. -#[must_use] -pub fn cargo_clippy_path() -> PathBuf { - let mut path = env::current_exe().expect("failed to get current executable name"); - path.set_file_name(CARGO_CLIPPY_EXE); - path -} - /// Creates a `Command` for running cargo. #[must_use] pub fn cargo_cmd() -> Command { From 913681464e21e8c7d27ea25b8504710d3fd847d9 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 24 May 2025 14:18:06 -0400 Subject: [PATCH 7/7] Make `utils` module private. --- clippy_dev/src/lib.rs | 7 ++++--- clippy_dev/src/main.rs | 11 ++++++----- clippy_dev/src/utils.rs | 10 ---------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 3361443196ab..eaf9589c8fe2 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -16,8 +16,7 @@ )] #![allow(clippy::missing_panics_doc)] -// The `rustc_driver` crate seems to be required in order to use the `rust_lexer` crate. -#[allow(unused_extern_crates)] +#[expect(unused_extern_crates, reason = "required to link to rustc crates")] extern crate rustc_driver; extern crate rustc_lexer; extern crate rustc_literal_escaper; @@ -33,4 +32,6 @@ pub mod serve; pub mod setup; pub mod sync; pub mod update_lints; -pub mod utils; + +mod utils; +pub use utils::{ClippyInfo, UpdateMode}; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 26aa269fb638..5fef231f6ca1 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -4,14 +4,15 @@ use clap::{Args, Parser, Subcommand}; use clippy_dev::{ - deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync, update_lints, utils, + ClippyInfo, UpdateMode, deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync, + update_lints, }; use std::convert::Infallible; use std::env; fn main() { let dev = Dev::parse(); - let clippy = utils::ClippyInfo::search_for_manifest(); + let clippy = ClippyInfo::search_for_manifest(); if let Err(e) = env::set_current_dir(&clippy.path) { panic!("error setting current directory to `{}`: {e}", clippy.path.display()); } @@ -26,8 +27,8 @@ fn main() { allow_staged, allow_no_vcs, } => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs), - DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)), - DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)), + DevCommand::Fmt { check } => fmt::run(UpdateMode::from_check(check)), + DevCommand::UpdateLints { check } => update_lints::update(UpdateMode::from_check(check)), DevCommand::NewLint { pass, name, @@ -35,7 +36,7 @@ fn main() { r#type, msrv, } => match new_lint::create(clippy.version, pass, &name, &category, r#type.as_deref(), msrv) { - Ok(()) => update_lints::update(utils::UpdateMode::Change), + Ok(()) => update_lints::update(UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {e}"), }, DevCommand::Setup(SetupCommand { subcommand }) => match subcommand { diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index dfa2d186097b..057951d0e33b 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -434,7 +434,6 @@ pub enum Token<'a> { OpenParen, Pound, Semi, - Slash, } pub struct RustSearcher<'txt> { @@ -512,7 +511,6 @@ impl<'txt> RustSearcher<'txt> { | (Token::OpenParen, lexer::TokenKind::OpenParen) | (Token::Pound, lexer::TokenKind::Pound) | (Token::Semi, lexer::TokenKind::Semi) - | (Token::Slash, lexer::TokenKind::Slash) | ( Token::LitStr, lexer::TokenKind::Literal { @@ -586,7 +584,6 @@ impl<'txt> RustSearcher<'txt> { } #[track_caller] -#[expect(clippy::must_use_candidate)] pub fn try_rename_file(old_name: &Path, new_name: &Path) -> bool { match OpenOptions::new().create_new(true).write(true).open(new_name) { Ok(file) => drop(file), @@ -609,7 +606,6 @@ pub fn try_rename_file(old_name: &Path, new_name: &Path) -> bool { } #[track_caller] -#[expect(clippy::must_use_candidate)] pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { match fs::create_dir(new_name) { Ok(()) => {}, @@ -635,11 +631,6 @@ pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { } } -#[track_caller] -pub fn write_file(path: &Path, contents: &str) { - expect_action(fs::write(path, contents), ErrAction::Write, path); -} - #[track_caller] pub fn run_exit_on_err(path: &(impl AsRef + ?Sized), cmd: &mut Command) { match expect_action(cmd.status(), ErrAction::Run, path.as_ref()).code() { @@ -739,7 +730,6 @@ pub fn split_args_for_threads( } #[track_caller] -#[expect(clippy::must_use_candidate)] pub fn delete_file_if_exists(path: &Path) -> bool { match fs::remove_file(path) { Ok(()) => true,