Skip to content
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

Track clippy.toml and Cargo.toml in file_depinfo #9707

Merged
merged 1 commit into from
Oct 25, 2022
Merged
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
1 change: 0 additions & 1 deletion clippy_dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ indoc = "1.0"
itertools = "0.10.1"
opener = "0.5"
shell-escape = "0.1"
tempfile = "3.2"
walkdir = "2.3"

[features]
Expand Down
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()
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
.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 clippy whenever
// 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