Skip to content

Commit fe51e04

Browse files
Merge branch 'master' into improve-serve-function
2 parents 48fa2fa + fc42a20 commit fe51e04

File tree

10 files changed

+178
-145
lines changed

10 files changed

+178
-145
lines changed

clippy_dev/src/dogfood.rs

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,28 @@
1-
use crate::utils::exit_if_err;
2-
use std::process::Command;
1+
use crate::utils::{cargo_cmd, run_exit_on_err};
2+
use itertools::Itertools;
33

44
/// # Panics
55
///
66
/// Panics if unable to run the dogfood test
77
#[allow(clippy::fn_params_excessive_bools)]
88
pub fn dogfood(fix: bool, allow_dirty: bool, allow_staged: bool, allow_no_vcs: bool) {
9-
let mut cmd = Command::new("cargo");
10-
11-
cmd.args(["test", "--test", "dogfood"])
12-
.args(["--features", "internal"])
13-
.args(["--", "dogfood_clippy", "--nocapture"]);
14-
15-
let mut dogfood_args = Vec::new();
16-
if fix {
17-
dogfood_args.push("--fix");
18-
}
19-
20-
if allow_dirty {
21-
dogfood_args.push("--allow-dirty");
22-
}
23-
24-
if allow_staged {
25-
dogfood_args.push("--allow-staged");
26-
}
27-
28-
if allow_no_vcs {
29-
dogfood_args.push("--allow-no-vcs");
30-
}
31-
32-
cmd.env("__CLIPPY_DOGFOOD_ARGS", dogfood_args.join(" "));
33-
34-
exit_if_err(cmd.status());
9+
run_exit_on_err(
10+
"cargo test",
11+
cargo_cmd()
12+
.args(["test", "--test", "dogfood"])
13+
.args(["--features", "internal"])
14+
.args(["--", "dogfood_clippy", "--nocapture"])
15+
.env(
16+
"__CLIPPY_DOGFOOD_ARGS",
17+
[
18+
if fix { "--fix" } else { "" },
19+
if allow_dirty { "--allow-dirty" } else { "" },
20+
if allow_staged { "--allow-staged" } else { "" },
21+
if allow_no_vcs { "--allow-no-vcs" } else { "" },
22+
]
23+
.iter()
24+
.filter(|x| !x.is_empty())
25+
.join(" "),
26+
),
27+
);
3528
}

clippy_dev/src/lib.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
)]
1616
#![allow(clippy::missing_panics_doc)]
1717

18-
// The `rustc_driver` crate seems to be required in order to use the `rust_lexer` crate.
19-
#[allow(unused_extern_crates)]
18+
#[expect(unused_extern_crates, reason = "required to link to rustc crates")]
2019
extern crate rustc_driver;
2120
extern crate rustc_lexer;
2221
extern crate rustc_literal_escaper;
@@ -32,4 +31,6 @@ pub mod serve;
3231
pub mod setup;
3332
pub mod sync;
3433
pub mod update_lints;
35-
pub mod utils;
34+
35+
mod utils;
36+
pub use utils::{ClippyInfo, UpdateMode};

clippy_dev/src/lint.rs

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,44 @@
1-
use crate::utils::{cargo_clippy_path, exit_if_err};
2-
use std::process::{self, Command};
1+
use crate::utils::{ErrAction, cargo_cmd, expect_action, run_exit_on_err};
2+
use std::process::Command;
33
use std::{env, fs};
44

5-
pub fn run<'a>(path: &str, edition: &str, args: impl Iterator<Item = &'a String>) {
6-
let is_file = match fs::metadata(path) {
7-
Ok(metadata) => metadata.is_file(),
8-
Err(e) => {
9-
eprintln!("Failed to read {path}: {e:?}");
10-
process::exit(1);
11-
},
12-
};
5+
#[cfg(not(windows))]
6+
static CARGO_CLIPPY_EXE: &str = "cargo-clippy";
7+
#[cfg(windows)]
8+
static CARGO_CLIPPY_EXE: &str = "cargo-clippy.exe";
139

10+
pub fn run<'a>(path: &str, edition: &str, args: impl Iterator<Item = &'a String>) {
11+
let is_file = expect_action(fs::metadata(path), ErrAction::Read, path).is_file();
1412
if is_file {
15-
exit_if_err(
16-
Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into()))
13+
run_exit_on_err(
14+
"cargo run",
15+
cargo_cmd()
1716
.args(["run", "--bin", "clippy-driver", "--"])
1817
.args(["-L", "./target/debug"])
1918
.args(["-Z", "no-codegen"])
2019
.args(["--edition", edition])
2120
.arg(path)
2221
.args(args)
2322
// Prevent rustc from creating `rustc-ice-*` files the console output is enough.
24-
.env("RUSTC_ICE", "0")
25-
.status(),
23+
.env("RUSTC_ICE", "0"),
2624
);
2725
} else {
28-
exit_if_err(
29-
Command::new(env::var("CARGO").unwrap_or_else(|_| "cargo".into()))
30-
.arg("build")
31-
.status(),
32-
);
33-
34-
let status = Command::new(cargo_clippy_path())
35-
.arg("clippy")
36-
.args(args)
37-
// Prevent rustc from creating `rustc-ice-*` files the console output is enough.
38-
.env("RUSTC_ICE", "0")
39-
.current_dir(path)
40-
.status();
26+
// Ideally this would just be `cargo run`, but the working directory needs to be
27+
// set to clippy's directory when building, and the target project's directory
28+
// when running clippy. `cargo` can only set a single working directory for both
29+
// when using `run`.
30+
run_exit_on_err("cargo build", cargo_cmd().arg("build"));
4131

42-
exit_if_err(status);
32+
let mut exe = env::current_exe().expect("failed to get current executable name");
33+
exe.set_file_name(CARGO_CLIPPY_EXE);
34+
run_exit_on_err(
35+
"cargo clippy",
36+
Command::new(exe)
37+
.arg("clippy")
38+
.args(args)
39+
// Prevent rustc from creating `rustc-ice-*` files the console output is enough.
40+
.env("RUSTC_ICE", "0")
41+
.current_dir(path),
42+
);
4343
}
4444
}

clippy_dev/src/main.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44

55
use clap::{Args, Parser, Subcommand};
66
use clippy_dev::{
7-
deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync, update_lints, utils,
7+
ClippyInfo, UpdateMode, deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync,
8+
update_lints,
89
};
910
use std::convert::Infallible;
1011
use std::env;
1112

1213
fn main() {
1314
let dev = Dev::parse();
14-
let clippy = utils::ClippyInfo::search_for_manifest();
15+
let clippy = ClippyInfo::search_for_manifest();
1516
if let Err(e) = env::set_current_dir(&clippy.path) {
1617
panic!("error setting current directory to `{}`: {e}", clippy.path.display());
1718
}
@@ -26,16 +27,16 @@ fn main() {
2627
allow_staged,
2728
allow_no_vcs,
2829
} => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs),
29-
DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)),
30-
DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
30+
DevCommand::Fmt { check } => fmt::run(UpdateMode::from_check(check)),
31+
DevCommand::UpdateLints { check } => update_lints::update(UpdateMode::from_check(check)),
3132
DevCommand::NewLint {
3233
pass,
3334
name,
3435
category,
3536
r#type,
3637
msrv,
3738
} => match new_lint::create(clippy.version, pass, &name, &category, r#type.as_deref(), msrv) {
38-
Ok(()) => update_lints::update(utils::UpdateMode::Change),
39+
Ok(()) => update_lints::update(UpdateMode::Change),
3940
Err(e) => eprintln!("Unable to create lint: {e}"),
4041
},
4142
DevCommand::Setup(SetupCommand { subcommand }) => match subcommand {

clippy_dev/src/serve.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1+
use crate::utils::{ErrAction, cargo_cmd, expect_action};
2+
use core::fmt::Display;
3+
use core::mem;
14
use std::path::Path;
25
use std::process::Command;
36
use std::time::{Duration, SystemTime};
7+
improve-serve-function
48
use std::{env, thread};
59
use std::io;
10+
use std::{fs, thread};
11+
use walkdir::WalkDir;
12+
master
613

714
/// Python binary depending on OS
815
#[cfg(windows)]
@@ -24,6 +31,7 @@ pub fn run(port: u16, lint: Option<String>) -> ! {
2431
None => format!("http://localhost:{port}"),
2532
});
2633

34+
improve-serve-function
2735
let mut server_started = false;
2836

2937
loop {
@@ -72,10 +80,45 @@ pub fn run(port: u16, lint: Option<String>) -> ! {
7280
}
7381
}
7482

83+
84+
let mut last_update = mtime("util/gh-pages/index.html");
85+
loop {
86+
if is_metadata_outdated(mem::replace(&mut last_update, SystemTime::now())) {
87+
// Ignore the command result; we'll fall back to displaying the old metadata.
88+
let _ = expect_action(
89+
cargo_cmd().arg("collect-metadata").status(),
90+
ErrAction::Run,
91+
"cargo collect-metadata",
92+
);
93+
last_update = SystemTime::now();
94+
}
95+
96+
// Only start the web server the first time around.
97+
if let Some(url) = url.take() {
98+
thread::spawn(move || {
99+
let mut child = expect_action(
100+
Command::new(PYTHON)
101+
.args(["-m", "http.server", port.to_string().as_str()])
102+
.current_dir("util/gh-pages")
103+
.spawn(),
104+
ErrAction::Run,
105+
"python -m http.server",
106+
);
107+
// Give some time for python to start
108+
thread::sleep(Duration::from_millis(500));
109+
// Launch browser after first export.py has completed and http.server is up
110+
let _result = opener::open(url);
111+
expect_action(child.wait(), ErrAction::Run, "python -m http.server");
112+
});
113+
}
114+
115+
// Delay to avoid updating the metadata too aggressively.
116+
master
75117
thread::sleep(Duration::from_millis(1000));
76118
}
77119
}
78120

121+
improve-serve-function
79122
/// Get the most recent modification time of a file or directory recursively.
80123
/// Returns `io::Result<SystemTime>`.
81124
fn mtime(path: impl AsRef<Path>) -> io::Result<SystemTime> {
@@ -93,5 +136,49 @@ fn mtime(path: impl AsRef<Path>) -> io::Result<SystemTime> {
93136
Ok(latest)
94137
} else {
95138
Ok(path.metadata()?.modified()?)
139+
140+
fn log_err_and_continue<T>(res: Result<T, impl Display>, path: &Path) -> Option<T> {
141+
match res {
142+
Ok(x) => Some(x),
143+
Err(ref e) => {
144+
eprintln!("error reading `{}`: {e}", path.display());
145+
None
146+
},
147+
master
96148
}
97149
}
150+
151+
fn mtime(path: &str) -> SystemTime {
152+
log_err_and_continue(fs::metadata(path), path.as_ref())
153+
.and_then(|metadata| log_err_and_continue(metadata.modified(), path.as_ref()))
154+
.unwrap_or(SystemTime::UNIX_EPOCH)
155+
}
156+
157+
fn is_metadata_outdated(time: SystemTime) -> bool {
158+
// Ignore all IO errors here. We don't want to stop them from hosting the server.
159+
if time < mtime("util/gh-pages/index_template.html") || time < mtime("tests/compile-test.rs") {
160+
return true;
161+
}
162+
let Some(dir) = log_err_and_continue(fs::read_dir("."), ".".as_ref()) else {
163+
return false;
164+
};
165+
dir.map_while(|e| log_err_and_continue(e, ".".as_ref())).any(|e| {
166+
let name = e.file_name();
167+
let name_bytes = name.as_encoded_bytes();
168+
if (name_bytes.starts_with(b"clippy_lints") && name_bytes != b"clippy_lints_internal")
169+
|| name_bytes == b"clippy_config"
170+
{
171+
WalkDir::new(&name)
172+
.into_iter()
173+
.map_while(|e| log_err_and_continue(e, name.as_ref()))
174+
.filter(|e| e.file_type().is_file())
175+
.filter_map(|e| {
176+
log_err_and_continue(e.metadata(), e.path())
177+
.and_then(|m| log_err_and_continue(m.modified(), e.path()))
178+
})
179+
.any(|ftime| time < ftime)
180+
} else {
181+
false
182+
}
183+
})
184+
}

clippy_dev/src/setup/git_hook.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use std::fs;
22
use std::path::Path;
33

4-
use super::verify_inside_clippy_dir;
5-
64
/// Rusts setup uses `git rev-parse --git-common-dir` to get the root directory of the repo.
75
/// I've decided against this for the sake of simplicity and to make sure that it doesn't install
86
/// 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) {
3533
}
3634

3735
fn check_precondition(force_override: bool) -> bool {
38-
if !verify_inside_clippy_dir() {
39-
return false;
40-
}
41-
4236
// Make sure that we can find the git repository
4337
let git_path = Path::new(REPO_GIT_DIR);
4438
if !git_path.exists() || !git_path.is_dir() {

clippy_dev/src/setup/mod.rs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,3 @@ pub mod git_hook;
22
pub mod intellij;
33
pub mod toolchain;
44
pub mod vscode;
5-
6-
use std::path::Path;
7-
8-
const CLIPPY_DEV_DIR: &str = "clippy_dev";
9-
10-
/// This function verifies that the tool is being executed in the clippy directory.
11-
/// This is useful to ensure that setups only modify Clippy's resources. The verification
12-
/// is done by checking that `clippy_dev` is a sub directory of the current directory.
13-
///
14-
/// It will print an error message and return `false` if the directory could not be
15-
/// verified.
16-
fn verify_inside_clippy_dir() -> bool {
17-
let path = Path::new(CLIPPY_DEV_DIR);
18-
if path.exists() && path.is_dir() {
19-
true
20-
} else {
21-
eprintln!("error: unable to verify that the working directory is clippy's directory");
22-
false
23-
}
24-
}

clippy_dev/src/setup/toolchain.rs

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,12 @@
1+
use crate::utils::{cargo_cmd, run_exit_on_err};
12
use std::env::consts::EXE_SUFFIX;
23
use std::env::current_dir;
34
use std::ffi::OsStr;
45
use std::fs;
56
use std::path::{Path, PathBuf};
6-
use std::process::Command;
77
use walkdir::WalkDir;
88

9-
use crate::utils::exit_if_err;
10-
11-
use super::verify_inside_clippy_dir;
12-
139
pub fn create(standalone: bool, force: bool, release: bool, name: &str) {
14-
if !verify_inside_clippy_dir() {
15-
return;
16-
}
17-
1810
let rustup_home = std::env::var("RUSTUP_HOME").unwrap();
1911
let toolchain = std::env::var("RUSTUP_TOOLCHAIN").unwrap();
2012

@@ -51,11 +43,10 @@ pub fn create(standalone: bool, force: bool, release: bool, name: &str) {
5143
}
5244
}
5345

54-
let status = Command::new("cargo")
55-
.arg("build")
56-
.args(release.then_some("--release"))
57-
.status();
58-
exit_if_err(status);
46+
run_exit_on_err(
47+
"cargo build",
48+
cargo_cmd().arg("build").args(release.then_some("--release")),
49+
);
5950

6051
install_bin("cargo-clippy", &dest, standalone, release);
6152
install_bin("clippy-driver", &dest, standalone, release);

0 commit comments

Comments
 (0)