Skip to content

Commit 40bead0

Browse files
authored
clippy_dev refactor (#14616)
Based on #13223 Various refactoring on `clippy_dev` before upgrading rename and splitting `clippy_lints` into multiple crates. Some improvements: * The working directory is set to the root clippy directory. Running from a subdirectory was kind of supported before sometimes. Now it just works. * File won't be written unless they're actually updated. Most of the time they weren't written, but a few cases slipped through. * Buffers are reused more for the negligible speed boost. changelog: None
2 parents 1a864f3 + 0636121 commit 40bead0

File tree

12 files changed

+1340
-1102
lines changed

12 files changed

+1340
-1102
lines changed

clippy_dev/src/deprecate_lint.rs

+174
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
use crate::update_lints::{
2+
DeprecatedLint, DeprecatedLints, Lint, find_lint_decls, generate_lint_files, read_deprecated_lints,
3+
};
4+
use crate::utils::{UpdateMode, Version};
5+
use std::ffi::OsStr;
6+
use std::path::{Path, PathBuf};
7+
use std::{fs, io};
8+
9+
/// Runs the `deprecate` command
10+
///
11+
/// This does the following:
12+
/// * Adds an entry to `deprecated_lints.rs`.
13+
/// * Removes the lint declaration (and the entire file if applicable)
14+
///
15+
/// # Panics
16+
///
17+
/// If a file path could not read from or written to
18+
pub fn deprecate(clippy_version: Version, name: &str, reason: &str) {
19+
let prefixed_name = if name.starts_with("clippy::") {
20+
name.to_owned()
21+
} else {
22+
format!("clippy::{name}")
23+
};
24+
let stripped_name = &prefixed_name[8..];
25+
26+
let mut lints = find_lint_decls();
27+
let DeprecatedLints {
28+
renamed: renamed_lints,
29+
deprecated: mut deprecated_lints,
30+
file: mut deprecated_file,
31+
contents: mut deprecated_contents,
32+
deprecated_end,
33+
..
34+
} = read_deprecated_lints();
35+
36+
let Some(lint) = lints.iter().find(|l| l.name == stripped_name) else {
37+
eprintln!("error: failed to find lint `{name}`");
38+
return;
39+
};
40+
41+
let mod_path = {
42+
let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module));
43+
if mod_path.is_dir() {
44+
mod_path = mod_path.join("mod");
45+
}
46+
47+
mod_path.set_extension("rs");
48+
mod_path
49+
};
50+
51+
if remove_lint_declaration(stripped_name, &mod_path, &mut lints).unwrap_or(false) {
52+
deprecated_contents.insert_str(
53+
deprecated_end as usize,
54+
&format!(
55+
" #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n",
56+
clippy_version.rust_display(),
57+
prefixed_name,
58+
reason,
59+
),
60+
);
61+
deprecated_file.replace_contents(deprecated_contents.as_bytes());
62+
drop(deprecated_file);
63+
64+
deprecated_lints.push(DeprecatedLint {
65+
name: prefixed_name,
66+
reason: reason.into(),
67+
});
68+
69+
generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints);
70+
println!("info: `{name}` has successfully been deprecated");
71+
println!("note: you must run `cargo uitest` to update the test results");
72+
} else {
73+
eprintln!("error: lint not found");
74+
}
75+
}
76+
77+
fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io::Result<bool> {
78+
fn remove_lint(name: &str, lints: &mut Vec<Lint>) {
79+
lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos));
80+
}
81+
82+
fn remove_test_assets(name: &str) {
83+
let test_file_stem = format!("tests/ui/{name}");
84+
let path = Path::new(&test_file_stem);
85+
86+
// Some lints have their own directories, delete them
87+
if path.is_dir() {
88+
let _ = fs::remove_dir_all(path);
89+
return;
90+
}
91+
92+
// Remove all related test files
93+
let _ = fs::remove_file(path.with_extension("rs"));
94+
let _ = fs::remove_file(path.with_extension("stderr"));
95+
let _ = fs::remove_file(path.with_extension("fixed"));
96+
}
97+
98+
fn remove_impl_lint_pass(lint_name_upper: &str, content: &mut String) {
99+
let impl_lint_pass_start = content.find("impl_lint_pass!").unwrap_or_else(|| {
100+
content
101+
.find("declare_lint_pass!")
102+
.unwrap_or_else(|| panic!("failed to find `impl_lint_pass`"))
103+
});
104+
let mut impl_lint_pass_end = content[impl_lint_pass_start..]
105+
.find(']')
106+
.expect("failed to find `impl_lint_pass` terminator");
107+
108+
impl_lint_pass_end += impl_lint_pass_start;
109+
if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(lint_name_upper) {
110+
let mut lint_name_end = impl_lint_pass_start + (lint_name_pos + lint_name_upper.len());
111+
for c in content[lint_name_end..impl_lint_pass_end].chars() {
112+
// Remove trailing whitespace
113+
if c == ',' || c.is_whitespace() {
114+
lint_name_end += 1;
115+
} else {
116+
break;
117+
}
118+
}
119+
120+
content.replace_range(impl_lint_pass_start + lint_name_pos..lint_name_end, "");
121+
}
122+
}
123+
124+
if path.exists()
125+
&& let Some(lint) = lints.iter().find(|l| l.name == name)
126+
{
127+
if lint.module == name {
128+
// The lint name is the same as the file, we can just delete the entire file
129+
fs::remove_file(path)?;
130+
} else {
131+
// We can't delete the entire file, just remove the declaration
132+
133+
if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) {
134+
// Remove clippy_lints/src/some_mod/some_lint.rs
135+
let mut lint_mod_path = path.to_path_buf();
136+
lint_mod_path.set_file_name(name);
137+
lint_mod_path.set_extension("rs");
138+
139+
let _ = fs::remove_file(lint_mod_path);
140+
}
141+
142+
let mut content =
143+
fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));
144+
145+
eprintln!(
146+
"warn: you will have to manually remove any code related to `{name}` from `{}`",
147+
path.display()
148+
);
149+
150+
assert!(
151+
content[lint.declaration_range.clone()].contains(&name.to_uppercase()),
152+
"error: `{}` does not contain lint `{}`'s declaration",
153+
path.display(),
154+
lint.name
155+
);
156+
157+
// Remove lint declaration (declare_clippy_lint!)
158+
content.replace_range(lint.declaration_range.clone(), "");
159+
160+
// Remove the module declaration (mod xyz;)
161+
let mod_decl = format!("\nmod {name};");
162+
content = content.replacen(&mod_decl, "", 1);
163+
164+
remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content);
165+
fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy()));
166+
}
167+
168+
remove_test_assets(name);
169+
remove_lint(name, lints);
170+
return Ok(true);
171+
}
172+
173+
Ok(false)
174+
}

clippy_dev/src/dogfood.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{clippy_project_root, exit_if_err};
1+
use crate::utils::exit_if_err;
22
use std::process::Command;
33

44
/// # Panics
@@ -8,8 +8,7 @@ use std::process::Command;
88
pub fn dogfood(fix: bool, allow_dirty: bool, allow_staged: bool, allow_no_vcs: bool) {
99
let mut cmd = Command::new("cargo");
1010

11-
cmd.current_dir(clippy_project_root())
12-
.args(["test", "--test", "dogfood"])
11+
cmd.args(["test", "--test", "dogfood"])
1312
.args(["--features", "internal"])
1413
.args(["--", "dogfood_clippy", "--nocapture"]);
1514

clippy_dev/src/fmt.rs

+11-21
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::utils::clippy_project_root;
21
use itertools::Itertools;
32
use rustc_lexer::{TokenKind, tokenize};
43
use shell_escape::escape;
@@ -104,15 +103,8 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
104103
Field,
105104
}
106105

107-
let path: PathBuf = [
108-
clippy_project_root().as_path(),
109-
"clippy_config".as_ref(),
110-
"src".as_ref(),
111-
"conf.rs".as_ref(),
112-
]
113-
.into_iter()
114-
.collect();
115-
let text = fs::read_to_string(&path)?;
106+
let path = "clippy_config/src/conf.rs";
107+
let text = fs::read_to_string(path)?;
116108

117109
let (pre, conf) = text
118110
.split_once("define_Conf! {\n")
@@ -203,7 +195,7 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
203195
| (State::Lints, TokenKind::Comma | TokenKind::OpenParen | TokenKind::CloseParen) => {},
204196
_ => {
205197
return Err(Error::Parse(
206-
path,
198+
PathBuf::from(path),
207199
offset_to_line(&text, conf_offset + i),
208200
format!("unexpected token `{}`", &conf[i..i + t.len as usize]),
209201
));
@@ -213,7 +205,7 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
213205

214206
if !matches!(state, State::Field) {
215207
return Err(Error::Parse(
216-
path,
208+
PathBuf::from(path),
217209
offset_to_line(&text, conf_offset + conf.len()),
218210
"incomplete field".into(),
219211
));
@@ -260,18 +252,16 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
260252
if check {
261253
return Err(Error::CheckFailed);
262254
}
263-
fs::write(&path, new_text.as_bytes())?;
255+
fs::write(path, new_text.as_bytes())?;
264256
}
265257
Ok(())
266258
}
267259

268260
fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {
269-
let project_root = clippy_project_root();
270-
271261
// if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to
272262
// format because rustfmt would also format the entire rustc repo as it is a local
273263
// dependency
274-
if fs::read_to_string(project_root.join("Cargo.toml"))
264+
if fs::read_to_string("Cargo.toml")
275265
.expect("Failed to read clippy Cargo.toml")
276266
.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
277267
{
@@ -280,12 +270,12 @@ fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {
280270

281271
check_for_rustfmt(context)?;
282272

283-
cargo_fmt(context, project_root.as_path())?;
284-
cargo_fmt(context, &project_root.join("clippy_dev"))?;
285-
cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
286-
cargo_fmt(context, &project_root.join("lintcheck"))?;
273+
cargo_fmt(context, ".".as_ref())?;
274+
cargo_fmt(context, "clippy_dev".as_ref())?;
275+
cargo_fmt(context, "rustc_tools_util".as_ref())?;
276+
cargo_fmt(context, "lintcheck".as_ref())?;
287277

288-
let chunks = WalkDir::new(project_root.join("tests"))
278+
let chunks = WalkDir::new("tests")
289279
.into_iter()
290280
.filter_map(|entry| {
291281
let entry = entry.expect("failed to find tests");

clippy_dev/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![feature(rustc_private)]
1+
#![feature(rustc_private, if_let_guard, let_chains)]
22
#![warn(
33
trivial_casts,
44
trivial_numeric_casts,
@@ -14,11 +14,13 @@ extern crate rustc_driver;
1414
extern crate rustc_lexer;
1515
extern crate rustc_literal_escaper;
1616

17+
pub mod deprecate_lint;
1718
pub mod dogfood;
1819
pub mod fmt;
1920
pub mod lint;
2021
pub mod new_lint;
2122
pub mod release;
23+
pub mod rename_lint;
2224
pub mod serve;
2325
pub mod setup;
2426
pub mod sync;

clippy_dev/src/main.rs

+18-19
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@
33
#![warn(rust_2018_idioms, unused_lifetimes)]
44

55
use clap::{Args, Parser, Subcommand};
6-
use clippy_dev::{dogfood, fmt, lint, new_lint, release, serve, setup, sync, update_lints, utils};
6+
use clippy_dev::{
7+
deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync, update_lints, utils,
8+
};
79
use std::convert::Infallible;
10+
use std::env;
811

912
fn main() {
1013
let dev = Dev::parse();
14+
let clippy = utils::ClippyInfo::search_for_manifest();
15+
if let Err(e) = env::set_current_dir(&clippy.path) {
16+
panic!("error setting current directory to `{}`: {e}", clippy.path.display());
17+
}
1118

1219
match dev.command {
1320
DevCommand::Bless => {
@@ -20,22 +27,14 @@ fn main() {
2027
allow_no_vcs,
2128
} => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs),
2229
DevCommand::Fmt { check, verbose } => fmt::run(check, verbose),
23-
DevCommand::UpdateLints { print_only, check } => {
24-
if print_only {
25-
update_lints::print_lints();
26-
} else if check {
27-
update_lints::update(utils::UpdateMode::Check);
28-
} else {
29-
update_lints::update(utils::UpdateMode::Change);
30-
}
31-
},
30+
DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
3231
DevCommand::NewLint {
3332
pass,
3433
name,
3534
category,
3635
r#type,
3736
msrv,
38-
} => match new_lint::create(pass, &name, &category, r#type.as_deref(), msrv) {
37+
} => match new_lint::create(clippy.version, pass, &name, &category, r#type.as_deref(), msrv) {
3938
Ok(()) => update_lints::update(utils::UpdateMode::Change),
4039
Err(e) => eprintln!("Unable to create lint: {e}"),
4140
},
@@ -79,13 +78,18 @@ fn main() {
7978
old_name,
8079
new_name,
8180
uplift,
82-
} => update_lints::rename(&old_name, new_name.as_ref().unwrap_or(&old_name), uplift),
83-
DevCommand::Deprecate { name, reason } => update_lints::deprecate(&name, &reason),
81+
} => rename_lint::rename(
82+
clippy.version,
83+
&old_name,
84+
new_name.as_ref().unwrap_or(&old_name),
85+
uplift,
86+
),
87+
DevCommand::Deprecate { name, reason } => deprecate_lint::deprecate(clippy.version, &name, &reason),
8488
DevCommand::Sync(SyncCommand { subcommand }) => match subcommand {
8589
SyncSubcommand::UpdateNightly => sync::update_nightly(),
8690
},
8791
DevCommand::Release(ReleaseCommand { subcommand }) => match subcommand {
88-
ReleaseSubcommand::BumpVersion => release::bump_version(),
92+
ReleaseSubcommand::BumpVersion => release::bump_version(clippy.version),
8993
},
9094
}
9195
}
@@ -135,11 +139,6 @@ enum DevCommand {
135139
/// * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod` {n}
136140
/// * all lints are registered in the lint store
137141
UpdateLints {
138-
#[arg(long)]
139-
/// Print a table of lints to STDOUT
140-
///
141-
/// This does not include deprecated and internal lints. (Does not modify any files)
142-
print_only: bool,
143142
#[arg(long)]
144143
/// Checks that `cargo dev update_lints` has been run. Used on CI.
145144
check: bool,

0 commit comments

Comments
 (0)