Skip to content

Commit

Permalink
Track clippy.toml and Cargo.toml in file_depinfo
Browse files Browse the repository at this point in the history
Causes cargo to re-run clippy when those paths are modified

Also tracks the path to `clippy-driver` in debug mode to remove the
workarounds in `cargo dev lint` and `lintcheck`
  • Loading branch information
Alexendoo committed Oct 24, 2022
1 parent 5b09d4e commit 468ba1e
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 58 deletions.
8 changes: 0 additions & 8 deletions clippy_dev/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,12 @@ pub fn run<'a>(path: &str, args: impl Iterator<Item = &'a String>) {
} else {
exit_if_err(Command::new("cargo").arg("build").status());

// Run in a tempdir as changes to clippy do not retrigger linting
let target = tempfile::Builder::new()
.prefix("clippy")
.tempdir()
.expect("failed to create tempdir");

let status = Command::new(cargo_clippy_path())
.arg("clippy")
.args(args)
.current_dir(path)
.env("CARGO_TARGET_DIR", target.as_ref())
.status();

target.close().expect("failed to remove tempdir");
exit_if_err(status);
}
}
11 changes: 7 additions & 4 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ extern crate clippy_utils;
#[macro_use]
extern crate declare_clippy_lint;

use std::io;
use std::path::PathBuf;

use clippy_utils::parse_msrv;
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::{Lint, LintId};
Expand Down Expand Up @@ -303,8 +306,8 @@ mod zero_div_zero;
mod zero_sized_map_values;
// end lints modules, do not remove this comment, it’s used in `update_lints`

pub use crate::utils::conf::Conf;
use crate::utils::conf::{format_error, TryConf};
pub use crate::utils::conf::{lookup_conf_file, Conf};

/// Register all pre expansion lints
///
Expand Down Expand Up @@ -361,8 +364,8 @@ fn read_msrv(conf: &Conf, sess: &Session) -> Option<RustcVersion> {
}

#[doc(hidden)]
pub fn read_conf(sess: &Session) -> Conf {
let file_name = match utils::conf::lookup_conf_file() {
pub fn read_conf(sess: &Session, path: &io::Result<Option<PathBuf>>) -> Conf {
let file_name = match path {
Ok(Some(path)) => path,
Ok(None) => return Conf::default(),
Err(error) => {
Expand All @@ -372,7 +375,7 @@ pub fn read_conf(sess: &Session) -> Conf {
},
};

let TryConf { conf, errors, warnings } = utils::conf::read(&file_name);
let TryConf { conf, errors, warnings } = utils::conf::read(file_name);
// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
sess.err(format!(
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,10 @@ define_Conf! {
}

/// Search for the configuration file.
///
/// # Errors
///
/// Returns any unexpected filesystem error encountered when searching for the config file
pub fn lookup_conf_file() -> io::Result<Option<PathBuf>> {
/// Possible filename to search for.
const CONFIG_FILE_NAMES: [&str; 2] = [".clippy.toml", "clippy.toml"];
Expand Down
45 changes: 0 additions & 45 deletions lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,34 +544,6 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String,
(stats_string, counter)
}

/// check if the latest modification of the logfile is older than the modification date of the
/// clippy binary, if this is true, we should clean the lintchec shared target directory and recheck
fn lintcheck_needs_rerun(lintcheck_logs_path: &Path, paths: [&Path; 2]) -> bool {
if !lintcheck_logs_path.exists() {
return true;
}

let clippy_modified: std::time::SystemTime = {
let [cargo, driver] = paths.map(|p| {
std::fs::metadata(p)
.expect("failed to get metadata of file")
.modified()
.expect("failed to get modification date")
});
// the oldest modification of either of the binaries
std::cmp::max(cargo, driver)
};

let logs_modified: std::time::SystemTime = std::fs::metadata(lintcheck_logs_path)
.expect("failed to get metadata of file")
.modified()
.expect("failed to get modification date");

// time is represented in seconds since X
// logs_modified 2 and clippy_modified 5 means clippy binary is older and we need to recheck
logs_modified < clippy_modified
}

#[allow(clippy::too_many_lines)]
fn main() {
// We're being executed as a `RUSTC_WRAPPER` as part of `--recursive`
Expand All @@ -594,23 +566,6 @@ fn main() {
let cargo_clippy_path = fs::canonicalize(format!("target/debug/cargo-clippy{EXE_SUFFIX}")).unwrap();
let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap();

// if the clippy bin is newer than our logs, throw away target dirs to force clippy to
// refresh the logs
if lintcheck_needs_rerun(
&config.lintcheck_results_path,
[&cargo_clippy_path, &clippy_driver_path],
) {
let shared_target_dir = "target/lintcheck/shared_target_dir";
// if we get an Err here, the shared target dir probably does simply not exist
if let Ok(metadata) = std::fs::metadata(shared_target_dir) {
if metadata.is_dir() {
println!("Clippy is newer than lint check logs, clearing lintcheck shared target dir...");
std::fs::remove_dir_all(shared_target_dir)
.expect("failed to remove target/lintcheck/shared_target_dir");
}
}
}

// assert that clippy is found
assert!(
cargo_clippy_path.is_file(),
Expand Down
37 changes: 36 additions & 1 deletion src/driver.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![feature(rustc_private)]
#![feature(let_chains)]
#![feature(once_cell)]
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
// warn on lints, that are included in `rust-lang/rust`s bootstrap
Expand Down Expand Up @@ -71,6 +72,32 @@ fn track_clippy_args(parse_sess: &mut ParseSess, args_env_var: &Option<String>)
));
}

/// Track files that may be accessed at runtime in `file_depinfo` so that cargo will re-run clippy
/// when any of them are modified
fn track_files(parse_sess: &mut ParseSess, conf_path_string: Option<String>) {
let file_depinfo = parse_sess.file_depinfo.get_mut();

// Used by `clippy::cargo` lints and to determine the MSRV. `cargo clippy` executes `clippy-driver`
// with the current directory set to `CARGO_MANIFEST_DIR` so a relative path is fine
if Path::new("Cargo.toml").exists() {
file_depinfo.insert(Symbol::intern("Cargo.toml"));
}

// `clippy.toml`
if let Some(path) = conf_path_string {
file_depinfo.insert(Symbol::intern(&path));
}

// During development track the clippy-driver executable so that cargo will re-run every time it is
// rebuilt
if cfg!(debug_assertions)
&& let Ok(current_exe) = env::current_exe()
&& let Some(current_exe) = current_exe.to_str()
{
file_depinfo.insert(Symbol::intern(current_exe));
}
}

struct DefaultCallbacks;
impl rustc_driver::Callbacks for DefaultCallbacks {}

Expand All @@ -97,10 +124,18 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
// JUSTIFICATION: necessary in clippy driver to set `mir_opt_level`
#[allow(rustc::bad_opt_access)]
fn config(&mut self, config: &mut interface::Config) {
let conf_path = clippy_lints::lookup_conf_file();
let conf_path_string = if let Ok(Some(path)) = &conf_path {
path.to_str().map(String::from)
} else {
None
};

let previous = config.register_lints.take();
let clippy_args_var = self.clippy_args_var.take();
config.parse_sess_created = Some(Box::new(move |parse_sess| {
track_clippy_args(parse_sess, &clippy_args_var);
track_files(parse_sess, conf_path_string);
}));
config.register_lints = Some(Box::new(move |sess, lint_store| {
// technically we're ~guaranteed that this is none but might as well call anything that
Expand All @@ -109,7 +144,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
(previous)(sess, lint_store);
}

let conf = clippy_lints::read_conf(sess);
let conf = clippy_lints::read_conf(sess, &conf_path);
clippy_lints::register_plugins(lint_store, sess, &conf);
clippy_lints::register_pre_expansion_lints(lint_store, sess, &conf);
clippy_lints::register_renamed(lint_store);
Expand Down

0 comments on commit 468ba1e

Please sign in to comment.