From be82601e854a37b687bd80a994a903f62ffe54de Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 11 Oct 2025 23:48:20 -0400 Subject: [PATCH 1/7] `clippy_dev`: Move lint uplifting into its own command. --- clippy_dev/src/deprecate_lint.rs | 54 ++++++++++++++++++++++++++++- clippy_dev/src/main.rs | 36 +++++++++----------- clippy_dev/src/rename_lint.rs | 58 +++++--------------------------- 3 files changed, 78 insertions(+), 70 deletions(-) diff --git a/clippy_dev/src/deprecate_lint.rs b/clippy_dev/src/deprecate_lint.rs index 0401cfda7080..bee7508dabb9 100644 --- a/clippy_dev/src/deprecate_lint.rs +++ b/clippy_dev/src/deprecate_lint.rs @@ -1,4 +1,4 @@ -use crate::parse::{DeprecatedLint, Lint, ParseCx}; +use crate::parse::{DeprecatedLint, Lint, ParseCx, RenamedLint}; use crate::update_lints::generate_lint_files; use crate::utils::{UpdateMode, Version}; use std::ffi::OsStr; @@ -61,6 +61,58 @@ pub fn deprecate<'cx>(cx: ParseCx<'cx>, clippy_version: Version, name: &'cx str, } } +pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { + let mut lints = cx.find_lint_decls(); + let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); + + let Some(lint) = lints.iter().find(|l| l.name == old_name) else { + eprintln!("error: failed to find lint `{old_name}`"); + return; + }; + + let old_name_prefixed = cx.str_buf.with(|buf| { + buf.extend(["clippy::", old_name]); + cx.arena.alloc_str(buf) + }); + for lint in &mut renamed_lints { + if lint.new_name == old_name_prefixed { + lint.new_name = new_name; + } + } + match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { + Ok(_) => { + println!("`{old_name}` is already deprecated"); + return; + }, + Err(idx) => renamed_lints.insert( + idx, + RenamedLint { + old_name: old_name_prefixed, + new_name, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }, + ), + } + + let mod_path = { + let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module)); + if mod_path.is_dir() { + mod_path = mod_path.join("mod"); + } + + mod_path.set_extension("rs"); + mod_path + }; + + if remove_lint_declaration(old_name, &mod_path, &mut lints).unwrap_or(false) { + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + println!("info: `{old_name}` has successfully been uplifted"); + println!("note: you must run `cargo uitest` to update the test results"); + } else { + eprintln!("error: lint not found"); + } +} + fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec>) -> io::Result { fn remove_lint(name: &str, lints: &mut Vec>) { lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos)); diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 392c3aabf193..9571dfde1877 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -74,18 +74,11 @@ fn main() { }, DevCommand::Serve { port, lint } => serve::run(port, lint), DevCommand::Lint { path, edition, args } => lint::run(&path, &edition, args.iter()), - DevCommand::RenameLint { - old_name, - new_name, - uplift, - } => new_parse_cx(|cx| { - rename_lint::rename( - cx, - clippy.version, - &old_name, - new_name.as_ref().unwrap_or(&old_name), - uplift, - ); + DevCommand::RenameLint { old_name, new_name } => new_parse_cx(|cx| { + rename_lint::rename(cx, clippy.version, &old_name, &new_name); + }), + DevCommand::Uplift { old_name, new_name } => new_parse_cx(|cx| { + deprecate_lint::uplift(cx, clippy.version, &old_name, new_name.as_deref().unwrap_or(&old_name)); }), DevCommand::Deprecate { name, reason } => { new_parse_cx(|cx| deprecate_lint::deprecate(cx, clippy.version, &name, &reason)); @@ -243,15 +236,9 @@ enum DevCommand { /// The name of the lint to rename #[arg(value_parser = lint_name)] old_name: String, - #[arg( - required_unless_present = "uplift", - value_parser = lint_name, - )] + #[arg(value_parser = lint_name)] /// The new name of the lint - new_name: Option, - #[arg(long)] - /// This lint will be uplifted into rustc - uplift: bool, + new_name: String, }, /// Deprecate the given lint Deprecate { @@ -266,6 +253,15 @@ enum DevCommand { Sync(SyncCommand), /// Manage Clippy releases Release(ReleaseCommand), + /// Marks a lint as uplifted into rustc and removes its code + Uplift { + /// The name of the lint to uplift + #[arg(value_parser = lint_name)] + old_name: String, + /// The name of the lint in rustc + #[arg(value_parser = lint_name)] + new_name: Option, + }, } #[derive(Args)] diff --git a/clippy_dev/src/rename_lint.rs b/clippy_dev/src/rename_lint.rs index 8e30eb7ce95b..9be4f2bdc970 100644 --- a/clippy_dev/src/rename_lint.rs +++ b/clippy_dev/src/rename_lint.rs @@ -25,8 +25,7 @@ use std::path::Path; /// * If either lint name has a prefix /// * If `old_name` doesn't name an existing lint. /// * If `old_name` names a deprecated or renamed lint. -#[expect(clippy::too_many_lines)] -pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str, new_name: &'cx str, uplift: bool) { +pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str, new_name: &'cx str) { let mut updater = FileUpdater::default(); let mut lints = cx.find_lint_decls(); let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); @@ -34,20 +33,15 @@ pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str let Ok(lint_idx) = lints.binary_search_by(|x| x.name.cmp(old_name)) else { panic!("could not find lint `{old_name}`"); }; - let lint = &lints[lint_idx]; let old_name_prefixed = cx.str_buf.with(|buf| { buf.extend(["clippy::", old_name]); cx.arena.alloc_str(buf) }); - let new_name_prefixed = if uplift { - new_name - } else { - cx.str_buf.with(|buf| { - buf.extend(["clippy::", new_name]); - cx.arena.alloc_str(buf) - }) - }; + let new_name_prefixed = cx.str_buf.with(|buf| { + buf.extend(["clippy::", new_name]); + cx.arena.alloc_str(buf) + }); for lint in &mut renamed_lints { if lint.new_name == old_name_prefixed { @@ -77,31 +71,7 @@ pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str let change_prefixed_tests = lints.get(lint_idx + 1).is_none_or(|l| !l.name.starts_with(old_name)); let mut mod_edit = ModEdit::None; - if uplift { - let is_unique_mod = lints[..lint_idx].iter().any(|l| l.module == lint.module) - || lints[lint_idx + 1..].iter().any(|l| l.module == lint.module); - if is_unique_mod { - if delete_file_if_exists(lint.path.as_ref()) { - mod_edit = ModEdit::Delete; - } - } else { - updater.update_file(&lint.path, &mut |_, src, dst| -> UpdateStatus { - let mut start = &src[..lint.declaration_range.start]; - if start.ends_with("\n\n") { - start = &start[..start.len() - 1]; - } - let mut end = &src[lint.declaration_range.end..]; - if end.starts_with("\n\n") { - end = &end[1..]; - } - dst.push_str(start); - dst.push_str(end); - UpdateStatus::Changed - }); - } - delete_test_files(old_name, change_prefixed_tests); - lints.remove(lint_idx); - } else if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { + if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { let lint = &mut lints[lint_idx]; if lint.module.ends_with(old_name) && lint @@ -139,19 +109,9 @@ pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str } generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); - if uplift { - println!("Uplifted `clippy::{old_name}` as `{new_name}`"); - if matches!(mod_edit, ModEdit::None) { - println!("Only the rename has been registered, the code will need to be edited manually"); - } else { - println!("All the lint's code has been deleted"); - println!("Make sure to inspect the results as some things may have been missed"); - } - } else { - println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); - println!("All code referencing the old name has been updated"); - println!("Make sure to inspect the results as some things may have been missed"); - } + println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); + println!("All code referencing the old name has been updated"); + println!("Make sure to inspect the results as some things may have been missed"); println!("note: `cargo uibless` still needs to be run to update the test results"); } From b4c3f17a5aa0a072de6dac94348d36e478281821 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 12 Oct 2025 14:43:21 -0400 Subject: [PATCH 2/7] `clippy_dev`: Rename `deprecate_lint` module to `edit_lints`. --- clippy_dev/src/edit_lints.rs | 213 +++++++++++++++++++++++++++++++++++ clippy_dev/src/lib.rs | 2 +- clippy_dev/src/main.rs | 8 +- 3 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 clippy_dev/src/edit_lints.rs diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs new file mode 100644 index 000000000000..573bd1e44535 --- /dev/null +++ b/clippy_dev/src/edit_lints.rs @@ -0,0 +1,213 @@ +use crate::parse::{DeprecatedLint, Lint, ParseCx, RenamedLint}; +use crate::update_lints::generate_lint_files; +use crate::utils::{UpdateMode, Version}; +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; +use std::{fs, io}; + +/// Runs the `deprecate` command +/// +/// This does the following: +/// * Adds an entry to `deprecated_lints.rs`. +/// * Removes the lint declaration (and the entire file if applicable) +/// +/// # Panics +/// +/// If a file path could not read from or written to +pub fn deprecate<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, name: &'env str, reason: &'env str) { + let mut lints = cx.find_lint_decls(); + let (mut deprecated_lints, renamed_lints) = cx.read_deprecated_lints(); + + let Some(lint) = lints.iter().find(|l| l.name == name) else { + eprintln!("error: failed to find lint `{name}`"); + return; + }; + + let prefixed_name = cx.str_buf.with(|buf| { + buf.extend(["clippy::", name]); + cx.arena.alloc_str(buf) + }); + match deprecated_lints.binary_search_by(|x| x.name.cmp(prefixed_name)) { + Ok(_) => { + println!("`{name}` is already deprecated"); + return; + }, + Err(idx) => deprecated_lints.insert( + idx, + DeprecatedLint { + name: prefixed_name, + reason, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }, + ), + } + + let mod_path = { + let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module)); + if mod_path.is_dir() { + mod_path = mod_path.join("mod"); + } + + mod_path.set_extension("rs"); + mod_path + }; + + if remove_lint_declaration(name, &mod_path, &mut lints).unwrap_or(false) { + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + println!("info: `{name}` has successfully been deprecated"); + println!("note: you must run `cargo uitest` to update the test results"); + } else { + eprintln!("error: lint not found"); + } +} + +pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { + let mut lints = cx.find_lint_decls(); + let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); + + let Some(lint) = lints.iter().find(|l| l.name == old_name) else { + eprintln!("error: failed to find lint `{old_name}`"); + return; + }; + + let old_name_prefixed = cx.str_buf.with(|buf| { + buf.extend(["clippy::", old_name]); + cx.arena.alloc_str(buf) + }); + for lint in &mut renamed_lints { + if lint.new_name == old_name_prefixed { + lint.new_name = new_name; + } + } + match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { + Ok(_) => { + println!("`{old_name}` is already deprecated"); + return; + }, + Err(idx) => renamed_lints.insert( + idx, + RenamedLint { + old_name: old_name_prefixed, + new_name, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }, + ), + } + + let mod_path = { + let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module)); + if mod_path.is_dir() { + mod_path = mod_path.join("mod"); + } + + mod_path.set_extension("rs"); + mod_path + }; + + if remove_lint_declaration(old_name, &mod_path, &mut lints).unwrap_or(false) { + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + println!("info: `{old_name}` has successfully been uplifted"); + println!("note: you must run `cargo uitest` to update the test results"); + } else { + eprintln!("error: lint not found"); + } +} + +fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec>) -> io::Result { + fn remove_lint(name: &str, lints: &mut Vec>) { + lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos)); + } + + fn remove_test_assets(name: &str) { + let test_file_stem = format!("tests/ui/{name}"); + let path = Path::new(&test_file_stem); + + // Some lints have their own directories, delete them + if path.is_dir() { + let _ = fs::remove_dir_all(path); + return; + } + + // Remove all related test files + let _ = fs::remove_file(path.with_extension("rs")); + let _ = fs::remove_file(path.with_extension("stderr")); + let _ = fs::remove_file(path.with_extension("fixed")); + } + + fn remove_impl_lint_pass(lint_name_upper: &str, content: &mut String) { + let impl_lint_pass_start = content.find("impl_lint_pass!").unwrap_or_else(|| { + content + .find("declare_lint_pass!") + .unwrap_or_else(|| panic!("failed to find `impl_lint_pass`")) + }); + let mut impl_lint_pass_end = content[impl_lint_pass_start..] + .find(']') + .expect("failed to find `impl_lint_pass` terminator"); + + impl_lint_pass_end += impl_lint_pass_start; + if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(lint_name_upper) { + let mut lint_name_end = impl_lint_pass_start + (lint_name_pos + lint_name_upper.len()); + for c in content[lint_name_end..impl_lint_pass_end].chars() { + // Remove trailing whitespace + if c == ',' || c.is_whitespace() { + lint_name_end += 1; + } else { + break; + } + } + + content.replace_range(impl_lint_pass_start + lint_name_pos..lint_name_end, ""); + } + } + + if path.exists() + && let Some(lint) = lints.iter().find(|l| l.name == name) + { + if lint.module == name { + // The lint name is the same as the file, we can just delete the entire file + fs::remove_file(path)?; + } else { + // We can't delete the entire file, just remove the declaration + + if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) { + // Remove clippy_lints/src/some_mod/some_lint.rs + let mut lint_mod_path = path.to_path_buf(); + lint_mod_path.set_file_name(name); + lint_mod_path.set_extension("rs"); + + let _ = fs::remove_file(lint_mod_path); + } + + let mut content = + fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy())); + + eprintln!( + "warn: you will have to manually remove any code related to `{name}` from `{}`", + path.display() + ); + + assert!( + content[lint.declaration_range].contains(&name.to_uppercase()), + "error: `{}` does not contain lint `{}`'s declaration", + path.display(), + lint.name + ); + + // Remove lint declaration (declare_clippy_lint!) + content.replace_range(lint.declaration_range, ""); + + // Remove the module declaration (mod xyz;) + let mod_decl = format!("\nmod {name};"); + content = content.replacen(&mod_decl, "", 1); + + remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content); + fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy())); + } + + remove_test_assets(name); + remove_lint(name, lints); + return Ok(true); + } + + Ok(false) +} diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index dcca08aee7e6..4a93860121fa 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -24,8 +24,8 @@ extern crate rustc_driver; extern crate rustc_lexer; extern crate rustc_literal_escaper; -pub mod deprecate_lint; pub mod dogfood; +pub mod edit_lints; pub mod fmt; pub mod lint; pub mod new_lint; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 9571dfde1877..e4e19a100c6e 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -4,8 +4,8 @@ use clap::{Args, Parser, Subcommand}; use clippy_dev::{ - ClippyInfo, UpdateMode, deprecate_lint, dogfood, fmt, lint, new_lint, new_parse_cx, release, rename_lint, serve, - setup, sync, update_lints, + ClippyInfo, UpdateMode, dogfood, edit_lints, fmt, lint, new_lint, new_parse_cx, release, rename_lint, serve, setup, + sync, update_lints, }; use std::env; @@ -78,10 +78,10 @@ fn main() { rename_lint::rename(cx, clippy.version, &old_name, &new_name); }), DevCommand::Uplift { old_name, new_name } => new_parse_cx(|cx| { - deprecate_lint::uplift(cx, clippy.version, &old_name, new_name.as_deref().unwrap_or(&old_name)); + edit_lints::uplift(cx, clippy.version, &old_name, new_name.as_deref().unwrap_or(&old_name)); }), DevCommand::Deprecate { name, reason } => { - new_parse_cx(|cx| deprecate_lint::deprecate(cx, clippy.version, &name, &reason)); + new_parse_cx(|cx| edit_lints::deprecate(cx, clippy.version, &name, &reason)); }, DevCommand::Sync(SyncCommand { subcommand }) => match subcommand { SyncSubcommand::UpdateNightly => sync::update_nightly(), From 53ca7d8dae08cfb42555bb552334e1c88080e8e8 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 12 Oct 2025 14:44:25 -0400 Subject: [PATCH 3/7] `clippy_dev`: Move `rename_lint` command to `edit_lints` module. --- clippy_dev/src/edit_lints.rs | 351 ++++++++++++++++++++++++++++++++- clippy_dev/src/lib.rs | 1 - clippy_dev/src/main.rs | 6 +- clippy_dev/src/rename_lint.rs | 353 ---------------------------------- 4 files changed, 352 insertions(+), 359 deletions(-) delete mode 100644 clippy_dev/src/rename_lint.rs diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs index 573bd1e44535..8a5ba26fd215 100644 --- a/clippy_dev/src/edit_lints.rs +++ b/clippy_dev/src/edit_lints.rs @@ -1,7 +1,12 @@ +use crate::parse::cursor::{self, Capture, Cursor}; use crate::parse::{DeprecatedLint, Lint, ParseCx, RenamedLint}; use crate::update_lints::generate_lint_files; -use crate::utils::{UpdateMode, Version}; -use std::ffi::OsStr; +use crate::utils::{ + ErrAction, FileUpdater, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, + expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, +}; +use rustc_lexer::TokenKind; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::{fs, io}; @@ -113,6 +118,111 @@ pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam } } +/// Runs the `rename_lint` command. +/// +/// This does the following: +/// * Adds an entry to `renamed_lints.rs`. +/// * Renames all lint attributes to the new name (e.g. `#[allow(clippy::lint_name)]`). +/// * Renames the lint struct to the new name. +/// * Renames the module containing the lint struct to the new name if it shares a name with the +/// lint. +/// +/// # Panics +/// Panics for the following conditions: +/// * If a file path could not read from or then written to +/// * If either lint name has a prefix +/// * If `old_name` doesn't name an existing lint. +/// * If `old_name` names a deprecated or renamed lint. +pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { + let mut updater = FileUpdater::default(); + let mut lints = cx.find_lint_decls(); + let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); + + let Ok(lint_idx) = lints.binary_search_by(|x| x.name.cmp(old_name)) else { + panic!("could not find lint `{old_name}`"); + }; + + let old_name_prefixed = cx.str_buf.with(|buf| { + buf.extend(["clippy::", old_name]); + cx.arena.alloc_str(buf) + }); + let new_name_prefixed = cx.str_buf.with(|buf| { + buf.extend(["clippy::", new_name]); + cx.arena.alloc_str(buf) + }); + + for lint in &mut renamed_lints { + if lint.new_name == old_name_prefixed { + lint.new_name = new_name_prefixed; + } + } + match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { + Ok(_) => { + println!("`{old_name}` already has a rename registered"); + return; + }, + Err(idx) => { + renamed_lints.insert( + idx, + RenamedLint { + old_name: old_name_prefixed, + new_name: new_name_prefixed, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }, + ); + }, + } + + // Some tests are named `lint_name_suffix` which should also be renamed, + // but we can't do that if the renamed lint's name overlaps with another + // lint. e.g. renaming 'foo' to 'bar' when a lint 'foo_bar' also exists. + let change_prefixed_tests = lints.get(lint_idx + 1).is_none_or(|l| !l.name.starts_with(old_name)); + + let mut mod_edit = ModEdit::None; + if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { + let lint = &mut lints[lint_idx]; + if lint.module.ends_with(old_name) + && lint + .path + .file_stem() + .is_some_and(|x| x.as_encoded_bytes() == old_name.as_bytes()) + { + let mut new_path = lint.path.with_file_name(new_name).into_os_string(); + new_path.push(".rs"); + if try_rename_file(lint.path.as_ref(), new_path.as_ref()) { + mod_edit = ModEdit::Rename; + } + + lint.module = cx.str_buf.with(|buf| { + buf.push_str(&lint.module[..lint.module.len() - old_name.len()]); + buf.push_str(new_name); + cx.arena.alloc_str(buf) + }); + } + rename_test_files(old_name, new_name, change_prefixed_tests); + lints[lint_idx].name = new_name; + lints.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); + } else { + println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); + println!("Since `{new_name}` already exists the existing code has not been changed"); + return; + } + + let mut update_fn = file_update_fn(old_name, new_name, mod_edit); + for e in walk_dir_no_dot_or_target(".") { + let e = expect_action(e, ErrAction::Read, "."); + if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { + updater.update_file(e.path(), &mut update_fn); + } + } + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + + println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); + println!("All code referencing the old name has been updated"); + println!("Make sure to inspect the results as some things may have been missed"); + println!("note: `cargo uibless` still needs to be run to update the test results"); +} + fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec>) -> io::Result { fn remove_lint(name: &str, lints: &mut Vec>) { lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos)); @@ -211,3 +321,240 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec>) - Ok(false) } + +#[derive(Clone, Copy)] +enum ModEdit { + None, + Delete, + Rename, +} + +fn collect_ui_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { + for e in fs::read_dir("tests/ui").expect("error reading `tests/ui`") { + let e = e.expect("error reading `tests/ui`"); + let name = e.file_name(); + if let Some((name_only, _)) = name.as_encoded_bytes().split_once(|&x| x == b'.') { + if name_only.starts_with(lint.as_bytes()) && (rename_prefixed || name_only.len() == lint.len()) { + dst.push((name, true)); + } + } else if name.as_encoded_bytes().starts_with(lint.as_bytes()) && (rename_prefixed || name.len() == lint.len()) + { + dst.push((name, false)); + } + } +} + +fn collect_ui_toml_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { + if rename_prefixed { + for e in fs::read_dir("tests/ui-toml").expect("error reading `tests/ui-toml`") { + let e = e.expect("error reading `tests/ui-toml`"); + let name = e.file_name(); + if name.as_encoded_bytes().starts_with(lint.as_bytes()) && e.file_type().is_ok_and(|ty| ty.is_dir()) { + dst.push((name, false)); + } + } + } else { + dst.push((lint.into(), false)); + } +} + +/// Renames all test files for the given lint. +/// +/// If `rename_prefixed` is `true` this will also rename tests which have the lint name as a prefix. +fn rename_test_files(old_name: &str, new_name: &str, rename_prefixed: bool) { + let mut tests = Vec::new(); + + let mut old_buf = OsString::from("tests/ui/"); + let mut new_buf = OsString::from("tests/ui/"); + collect_ui_test_names(old_name, rename_prefixed, &mut tests); + for &(ref name, is_file) in &tests { + old_buf.push(name); + new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); + if is_file { + try_rename_file(old_buf.as_ref(), new_buf.as_ref()); + } else { + try_rename_dir(old_buf.as_ref(), new_buf.as_ref()); + } + old_buf.truncate("tests/ui/".len()); + new_buf.truncate("tests/ui/".len()); + } + + tests.clear(); + old_buf.truncate("tests/ui".len()); + new_buf.truncate("tests/ui".len()); + old_buf.push("-toml/"); + new_buf.push("-toml/"); + collect_ui_toml_test_names(old_name, rename_prefixed, &mut tests); + for (name, _) in &tests { + old_buf.push(name); + new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); + try_rename_dir(old_buf.as_ref(), new_buf.as_ref()); + old_buf.truncate("tests/ui/".len()); + new_buf.truncate("tests/ui/".len()); + } +} + +fn delete_test_files(lint: &str, rename_prefixed: bool) { + let mut tests = Vec::new(); + + let mut buf = OsString::from("tests/ui/"); + collect_ui_test_names(lint, rename_prefixed, &mut tests); + for &(ref name, is_file) in &tests { + buf.push(name); + if is_file { + delete_file_if_exists(buf.as_ref()); + } else { + delete_dir_if_exists(buf.as_ref()); + } + buf.truncate("tests/ui/".len()); + } + + buf.truncate("tests/ui".len()); + buf.push("-toml/"); + + tests.clear(); + collect_ui_toml_test_names(lint, rename_prefixed, &mut tests); + for (name, _) in &tests { + buf.push(name); + delete_dir_if_exists(buf.as_ref()); + buf.truncate("tests/ui/".len()); + } +} + +fn snake_to_pascal(s: &str) -> String { + let mut dst = Vec::with_capacity(s.len()); + let mut iter = s.bytes(); + || -> Option<()> { + dst.push(iter.next()?.to_ascii_uppercase()); + while let Some(c) = iter.next() { + if c == b'_' { + dst.push(iter.next()?.to_ascii_uppercase()); + } else { + dst.push(c); + } + } + Some(()) + }(); + String::from_utf8(dst).unwrap() +} + +#[expect(clippy::too_many_lines)] +fn file_update_fn<'a, 'b>( + old_name: &'a str, + new_name: &'b str, + mod_edit: ModEdit, +) -> impl use<'a, 'b> + FnMut(&Path, &str, &mut String) -> UpdateStatus { + let old_name_pascal = snake_to_pascal(old_name); + let new_name_pascal = snake_to_pascal(new_name); + let old_name_upper = old_name.to_ascii_uppercase(); + let new_name_upper = new_name.to_ascii_uppercase(); + move |_, src, dst| { + let mut copy_pos = 0u32; + let mut changed = false; + let mut cursor = Cursor::new(src); + let mut captures = [Capture::EMPTY]; + loop { + match cursor.peek() { + TokenKind::Eof => break, + TokenKind::Ident => { + let match_start = cursor.pos(); + let text = cursor.peek_text(); + cursor.step(); + match text { + // clippy::line_name or clippy::lint-name + "clippy" => { + if cursor.match_all(&[cursor::Pat::DoubleColon, cursor::Pat::CaptureIdent], &mut captures) + && cursor.get_text(captures[0]) == old_name + { + dst.push_str(&src[copy_pos as usize..captures[0].pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + changed = true; + } + }, + // mod lint_name + "mod" => { + if !matches!(mod_edit, ModEdit::None) + && let Some(pos) = cursor.find_ident(old_name) + { + match mod_edit { + ModEdit::Rename => { + dst.push_str(&src[copy_pos as usize..pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + changed = true; + }, + ModEdit::Delete if cursor.match_pat(cursor::Pat::Semi) => { + let mut start = &src[copy_pos as usize..match_start as usize]; + if start.ends_with("\n\n") { + start = &start[..start.len() - 1]; + } + dst.push_str(start); + copy_pos = cursor.pos(); + if src[copy_pos as usize..].starts_with("\n\n") { + copy_pos += 1; + } + changed = true; + }, + ModEdit::Delete | ModEdit::None => {}, + } + } + }, + // lint_name:: + name if matches!(mod_edit, ModEdit::Rename) && name == old_name => { + let name_end = cursor.pos(); + if cursor.match_pat(cursor::Pat::DoubleColon) { + dst.push_str(&src[copy_pos as usize..match_start as usize]); + dst.push_str(new_name); + copy_pos = name_end; + changed = true; + } + }, + // LINT_NAME or LintName + name => { + let replacement = if name == old_name_upper { + &new_name_upper + } else if name == old_name_pascal { + &new_name_pascal + } else { + continue; + }; + dst.push_str(&src[copy_pos as usize..match_start as usize]); + dst.push_str(replacement); + copy_pos = cursor.pos(); + changed = true; + }, + } + }, + // //~ lint_name + TokenKind::LineComment { doc_style: None } => { + let text = cursor.peek_text(); + if text.starts_with("//~") + && let Some(text) = text.strip_suffix(old_name) + && !text.ends_with(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '_')) + { + dst.push_str(&src[copy_pos as usize..cursor.pos() as usize + text.len()]); + dst.push_str(new_name); + copy_pos = cursor.pos() + cursor.peek_len(); + changed = true; + } + cursor.step(); + }, + // ::lint_name + TokenKind::Colon + if cursor.match_all(&[cursor::Pat::DoubleColon, cursor::Pat::CaptureIdent], &mut captures) + && cursor.get_text(captures[0]) == old_name => + { + dst.push_str(&src[copy_pos as usize..captures[0].pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + changed = true; + }, + _ => cursor.step(), + } + } + + dst.push_str(&src[copy_pos as usize..]); + UpdateStatus::from_changed(changed) + } +} diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 4a93860121fa..fdab4235a97c 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -30,7 +30,6 @@ pub mod fmt; pub mod lint; pub mod new_lint; pub mod release; -pub mod rename_lint; pub mod serve; pub mod setup; pub mod sync; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index e4e19a100c6e..8dc2290df8e4 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -4,8 +4,8 @@ use clap::{Args, Parser, Subcommand}; use clippy_dev::{ - ClippyInfo, UpdateMode, dogfood, edit_lints, fmt, lint, new_lint, new_parse_cx, release, rename_lint, serve, setup, - sync, update_lints, + ClippyInfo, UpdateMode, dogfood, edit_lints, fmt, lint, new_lint, new_parse_cx, release, serve, setup, sync, + update_lints, }; use std::env; @@ -75,7 +75,7 @@ fn main() { DevCommand::Serve { port, lint } => serve::run(port, lint), DevCommand::Lint { path, edition, args } => lint::run(&path, &edition, args.iter()), DevCommand::RenameLint { old_name, new_name } => new_parse_cx(|cx| { - rename_lint::rename(cx, clippy.version, &old_name, &new_name); + edit_lints::rename(cx, clippy.version, &old_name, &new_name); }), DevCommand::Uplift { old_name, new_name } => new_parse_cx(|cx| { edit_lints::uplift(cx, clippy.version, &old_name, new_name.as_deref().unwrap_or(&old_name)); diff --git a/clippy_dev/src/rename_lint.rs b/clippy_dev/src/rename_lint.rs deleted file mode 100644 index 9be4f2bdc970..000000000000 --- a/clippy_dev/src/rename_lint.rs +++ /dev/null @@ -1,353 +0,0 @@ -use crate::parse::cursor::{self, Capture, Cursor}; -use crate::parse::{ParseCx, RenamedLint}; -use crate::update_lints::generate_lint_files; -use crate::utils::{ - ErrAction, FileUpdater, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, - expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, -}; -use rustc_lexer::TokenKind; -use std::ffi::OsString; -use std::fs; -use std::path::Path; - -/// Runs the `rename_lint` command. -/// -/// This does the following: -/// * Adds an entry to `renamed_lints.rs`. -/// * Renames all lint attributes to the new name (e.g. `#[allow(clippy::lint_name)]`). -/// * Renames the lint struct to the new name. -/// * Renames the module containing the lint struct to the new name if it shares a name with the -/// lint. -/// -/// # Panics -/// Panics for the following conditions: -/// * If a file path could not read from or then written to -/// * If either lint name has a prefix -/// * If `old_name` doesn't name an existing lint. -/// * If `old_name` names a deprecated or renamed lint. -pub fn rename<'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'cx str, new_name: &'cx str) { - let mut updater = FileUpdater::default(); - let mut lints = cx.find_lint_decls(); - let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); - - let Ok(lint_idx) = lints.binary_search_by(|x| x.name.cmp(old_name)) else { - panic!("could not find lint `{old_name}`"); - }; - - let old_name_prefixed = cx.str_buf.with(|buf| { - buf.extend(["clippy::", old_name]); - cx.arena.alloc_str(buf) - }); - let new_name_prefixed = cx.str_buf.with(|buf| { - buf.extend(["clippy::", new_name]); - cx.arena.alloc_str(buf) - }); - - for lint in &mut renamed_lints { - if lint.new_name == old_name_prefixed { - lint.new_name = new_name_prefixed; - } - } - match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { - Ok(_) => { - println!("`{old_name}` already has a rename registered"); - return; - }, - Err(idx) => { - renamed_lints.insert( - idx, - RenamedLint { - old_name: old_name_prefixed, - new_name: new_name_prefixed, - version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), - }, - ); - }, - } - - // Some tests are named `lint_name_suffix` which should also be renamed, - // but we can't do that if the renamed lint's name overlaps with another - // lint. e.g. renaming 'foo' to 'bar' when a lint 'foo_bar' also exists. - let change_prefixed_tests = lints.get(lint_idx + 1).is_none_or(|l| !l.name.starts_with(old_name)); - - let mut mod_edit = ModEdit::None; - if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { - let lint = &mut lints[lint_idx]; - if lint.module.ends_with(old_name) - && lint - .path - .file_stem() - .is_some_and(|x| x.as_encoded_bytes() == old_name.as_bytes()) - { - let mut new_path = lint.path.with_file_name(new_name).into_os_string(); - new_path.push(".rs"); - if try_rename_file(lint.path.as_ref(), new_path.as_ref()) { - mod_edit = ModEdit::Rename; - } - - lint.module = cx.str_buf.with(|buf| { - buf.push_str(&lint.module[..lint.module.len() - old_name.len()]); - buf.push_str(new_name); - cx.arena.alloc_str(buf) - }); - } - rename_test_files(old_name, new_name, change_prefixed_tests); - lints[lint_idx].name = new_name; - lints.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); - } else { - println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); - println!("Since `{new_name}` already exists the existing code has not been changed"); - return; - } - - let mut update_fn = file_update_fn(old_name, new_name, mod_edit); - for e in walk_dir_no_dot_or_target(".") { - let e = expect_action(e, ErrAction::Read, "."); - if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { - updater.update_file(e.path(), &mut update_fn); - } - } - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); - - println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); - println!("All code referencing the old name has been updated"); - println!("Make sure to inspect the results as some things may have been missed"); - println!("note: `cargo uibless` still needs to be run to update the test results"); -} - -#[derive(Clone, Copy)] -enum ModEdit { - None, - Delete, - Rename, -} - -fn collect_ui_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { - for e in fs::read_dir("tests/ui").expect("error reading `tests/ui`") { - let e = e.expect("error reading `tests/ui`"); - let name = e.file_name(); - if let Some((name_only, _)) = name.as_encoded_bytes().split_once(|&x| x == b'.') { - if name_only.starts_with(lint.as_bytes()) && (rename_prefixed || name_only.len() == lint.len()) { - dst.push((name, true)); - } - } else if name.as_encoded_bytes().starts_with(lint.as_bytes()) && (rename_prefixed || name.len() == lint.len()) - { - dst.push((name, false)); - } - } -} - -fn collect_ui_toml_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { - if rename_prefixed { - for e in fs::read_dir("tests/ui-toml").expect("error reading `tests/ui-toml`") { - let e = e.expect("error reading `tests/ui-toml`"); - let name = e.file_name(); - if name.as_encoded_bytes().starts_with(lint.as_bytes()) && e.file_type().is_ok_and(|ty| ty.is_dir()) { - dst.push((name, false)); - } - } - } else { - dst.push((lint.into(), false)); - } -} - -/// Renames all test files for the given lint. -/// -/// If `rename_prefixed` is `true` this will also rename tests which have the lint name as a prefix. -fn rename_test_files(old_name: &str, new_name: &str, rename_prefixed: bool) { - let mut tests = Vec::new(); - - let mut old_buf = OsString::from("tests/ui/"); - let mut new_buf = OsString::from("tests/ui/"); - collect_ui_test_names(old_name, rename_prefixed, &mut tests); - for &(ref name, is_file) in &tests { - old_buf.push(name); - new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); - if is_file { - try_rename_file(old_buf.as_ref(), new_buf.as_ref()); - } else { - try_rename_dir(old_buf.as_ref(), new_buf.as_ref()); - } - old_buf.truncate("tests/ui/".len()); - new_buf.truncate("tests/ui/".len()); - } - - tests.clear(); - old_buf.truncate("tests/ui".len()); - new_buf.truncate("tests/ui".len()); - old_buf.push("-toml/"); - new_buf.push("-toml/"); - collect_ui_toml_test_names(old_name, rename_prefixed, &mut tests); - for (name, _) in &tests { - old_buf.push(name); - new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); - try_rename_dir(old_buf.as_ref(), new_buf.as_ref()); - old_buf.truncate("tests/ui/".len()); - new_buf.truncate("tests/ui/".len()); - } -} - -fn delete_test_files(lint: &str, rename_prefixed: bool) { - let mut tests = Vec::new(); - - let mut buf = OsString::from("tests/ui/"); - collect_ui_test_names(lint, rename_prefixed, &mut tests); - for &(ref name, is_file) in &tests { - buf.push(name); - if is_file { - delete_file_if_exists(buf.as_ref()); - } else { - delete_dir_if_exists(buf.as_ref()); - } - buf.truncate("tests/ui/".len()); - } - - buf.truncate("tests/ui".len()); - buf.push("-toml/"); - - tests.clear(); - collect_ui_toml_test_names(lint, rename_prefixed, &mut tests); - for (name, _) in &tests { - buf.push(name); - delete_dir_if_exists(buf.as_ref()); - buf.truncate("tests/ui/".len()); - } -} - -fn snake_to_pascal(s: &str) -> String { - let mut dst = Vec::with_capacity(s.len()); - let mut iter = s.bytes(); - || -> Option<()> { - dst.push(iter.next()?.to_ascii_uppercase()); - while let Some(c) = iter.next() { - if c == b'_' { - dst.push(iter.next()?.to_ascii_uppercase()); - } else { - dst.push(c); - } - } - Some(()) - }(); - String::from_utf8(dst).unwrap() -} - -#[expect(clippy::too_many_lines)] -fn file_update_fn<'a, 'b>( - old_name: &'a str, - new_name: &'b str, - mod_edit: ModEdit, -) -> impl use<'a, 'b> + FnMut(&Path, &str, &mut String) -> UpdateStatus { - let old_name_pascal = snake_to_pascal(old_name); - let new_name_pascal = snake_to_pascal(new_name); - let old_name_upper = old_name.to_ascii_uppercase(); - let new_name_upper = new_name.to_ascii_uppercase(); - move |_, src, dst| { - let mut copy_pos = 0u32; - let mut changed = false; - let mut cursor = Cursor::new(src); - let mut captures = [Capture::EMPTY]; - loop { - match cursor.peek() { - TokenKind::Eof => break, - TokenKind::Ident => { - let match_start = cursor.pos(); - let text = cursor.peek_text(); - cursor.step(); - match text { - // clippy::line_name or clippy::lint-name - "clippy" => { - if cursor.match_all(&[cursor::Pat::DoubleColon, cursor::Pat::CaptureIdent], &mut captures) - && cursor.get_text(captures[0]) == old_name - { - dst.push_str(&src[copy_pos as usize..captures[0].pos as usize]); - dst.push_str(new_name); - copy_pos = cursor.pos(); - changed = true; - } - }, - // mod lint_name - "mod" => { - if !matches!(mod_edit, ModEdit::None) - && let Some(pos) = cursor.find_ident(old_name) - { - match mod_edit { - ModEdit::Rename => { - dst.push_str(&src[copy_pos as usize..pos as usize]); - dst.push_str(new_name); - copy_pos = cursor.pos(); - changed = true; - }, - ModEdit::Delete if cursor.match_pat(cursor::Pat::Semi) => { - let mut start = &src[copy_pos as usize..match_start as usize]; - if start.ends_with("\n\n") { - start = &start[..start.len() - 1]; - } - dst.push_str(start); - copy_pos = cursor.pos(); - if src[copy_pos as usize..].starts_with("\n\n") { - copy_pos += 1; - } - changed = true; - }, - ModEdit::Delete | ModEdit::None => {}, - } - } - }, - // lint_name:: - name if matches!(mod_edit, ModEdit::Rename) && name == old_name => { - let name_end = cursor.pos(); - if cursor.match_pat(cursor::Pat::DoubleColon) { - dst.push_str(&src[copy_pos as usize..match_start as usize]); - dst.push_str(new_name); - copy_pos = name_end; - changed = true; - } - }, - // LINT_NAME or LintName - name => { - let replacement = if name == old_name_upper { - &new_name_upper - } else if name == old_name_pascal { - &new_name_pascal - } else { - continue; - }; - dst.push_str(&src[copy_pos as usize..match_start as usize]); - dst.push_str(replacement); - copy_pos = cursor.pos(); - changed = true; - }, - } - }, - // //~ lint_name - TokenKind::LineComment { doc_style: None } => { - let text = cursor.peek_text(); - if text.starts_with("//~") - && let Some(text) = text.strip_suffix(old_name) - && !text.ends_with(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '_')) - { - dst.push_str(&src[copy_pos as usize..cursor.pos() as usize + text.len()]); - dst.push_str(new_name); - copy_pos = cursor.pos() + cursor.peek_len(); - changed = true; - } - cursor.step(); - }, - // ::lint_name - TokenKind::Colon - if cursor.match_all(&[cursor::Pat::DoubleColon, cursor::Pat::CaptureIdent], &mut captures) - && cursor.get_text(captures[0]) == old_name => - { - dst.push_str(&src[copy_pos as usize..captures[0].pos as usize]); - dst.push_str(new_name); - copy_pos = cursor.pos(); - changed = true; - }, - _ => cursor.step(), - } - } - - dst.push_str(&src[copy_pos as usize..]); - UpdateStatus::from_changed(changed) - } -} From 0eb8a65f6f88cc37cc8d1c6b17ce9f6c83f1f668 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 12 Oct 2025 22:42:33 -0400 Subject: [PATCH 4/7] `clippy_dev`: When renaming a lint better handle the case where the renamed lint is a prefix of another lint name. --- clippy_dev/src/edit_lints.rs | 73 ++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 33 deletions(-) diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs index 8a5ba26fd215..14dd8b21df9a 100644 --- a/clippy_dev/src/edit_lints.rs +++ b/clippy_dev/src/edit_lints.rs @@ -173,11 +173,6 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam }, } - // Some tests are named `lint_name_suffix` which should also be renamed, - // but we can't do that if the renamed lint's name overlaps with another - // lint. e.g. renaming 'foo' to 'bar' when a lint 'foo_bar' also exists. - let change_prefixed_tests = lints.get(lint_idx + 1).is_none_or(|l| !l.name.starts_with(old_name)); - let mut mod_edit = ModEdit::None; if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { let lint = &mut lints[lint_idx]; @@ -199,7 +194,16 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam cx.arena.alloc_str(buf) }); } - rename_test_files(old_name, new_name, change_prefixed_tests); + + rename_test_files( + old_name, + new_name, + &lints[lint_idx + 1..] + .iter() + .map(|l| l.name) + .take_while(|&n| n.starts_with(old_name)) + .collect::>(), + ); lints[lint_idx].name = new_name; lints.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); } else { @@ -329,44 +333,45 @@ enum ModEdit { Rename, } -fn collect_ui_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { +fn collect_ui_test_names(lint: &str, ignored_prefixes: &[&str], dst: &mut Vec<(OsString, bool)>) { for e in fs::read_dir("tests/ui").expect("error reading `tests/ui`") { let e = e.expect("error reading `tests/ui`"); let name = e.file_name(); - if let Some((name_only, _)) = name.as_encoded_bytes().split_once(|&x| x == b'.') { - if name_only.starts_with(lint.as_bytes()) && (rename_prefixed || name_only.len() == lint.len()) { - dst.push((name, true)); - } - } else if name.as_encoded_bytes().starts_with(lint.as_bytes()) && (rename_prefixed || name.len() == lint.len()) + if name.as_encoded_bytes().starts_with(lint.as_bytes()) + && !ignored_prefixes + .iter() + .any(|&pre| name.as_encoded_bytes().starts_with(pre.as_bytes())) + && let Ok(ty) = e.file_type() + && (ty.is_file() || ty.is_dir()) { - dst.push((name, false)); + dst.push((name, ty.is_file())); } } } -fn collect_ui_toml_test_names(lint: &str, rename_prefixed: bool, dst: &mut Vec<(OsString, bool)>) { - if rename_prefixed { - for e in fs::read_dir("tests/ui-toml").expect("error reading `tests/ui-toml`") { - let e = e.expect("error reading `tests/ui-toml`"); - let name = e.file_name(); - if name.as_encoded_bytes().starts_with(lint.as_bytes()) && e.file_type().is_ok_and(|ty| ty.is_dir()) { - dst.push((name, false)); - } +fn collect_ui_toml_test_names(lint: &str, ignored_prefixes: &[&str], dst: &mut Vec<(OsString, bool)>) { + for e in fs::read_dir("tests/ui-toml").expect("error reading `tests/ui-toml`") { + let e = e.expect("error reading `tests/ui-toml`"); + let name = e.file_name(); + if name.as_encoded_bytes().starts_with(lint.as_bytes()) + && !ignored_prefixes + .iter() + .any(|&pre| name.as_encoded_bytes().starts_with(pre.as_bytes())) + && e.file_type().is_ok_and(|ty| ty.is_dir()) + { + dst.push((name, false)); } - } else { - dst.push((lint.into(), false)); } } -/// Renames all test files for the given lint. -/// -/// If `rename_prefixed` is `true` this will also rename tests which have the lint name as a prefix. -fn rename_test_files(old_name: &str, new_name: &str, rename_prefixed: bool) { - let mut tests = Vec::new(); +/// Renames all test files for the given lint where the file name does not start with any +/// of the given prefixes. +fn rename_test_files(old_name: &str, new_name: &str, ignored_prefixes: &[&str]) { + let mut tests: Vec<(OsString, bool)> = Vec::new(); let mut old_buf = OsString::from("tests/ui/"); let mut new_buf = OsString::from("tests/ui/"); - collect_ui_test_names(old_name, rename_prefixed, &mut tests); + collect_ui_test_names(old_name, ignored_prefixes, &mut tests); for &(ref name, is_file) in &tests { old_buf.push(name); new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); @@ -384,7 +389,7 @@ fn rename_test_files(old_name: &str, new_name: &str, rename_prefixed: bool) { new_buf.truncate("tests/ui".len()); old_buf.push("-toml/"); new_buf.push("-toml/"); - collect_ui_toml_test_names(old_name, rename_prefixed, &mut tests); + collect_ui_toml_test_names(old_name, ignored_prefixes, &mut tests); for (name, _) in &tests { old_buf.push(name); new_buf.extend([new_name.as_ref(), name.slice_encoded_bytes(old_name.len()..)]); @@ -394,11 +399,13 @@ fn rename_test_files(old_name: &str, new_name: &str, rename_prefixed: bool) { } } -fn delete_test_files(lint: &str, rename_prefixed: bool) { +/// Deletes all test files for the given lint where the file name does not start with any +/// of the given prefixes. +fn delete_test_files(lint: &str, ignored_prefixes: &[&str]) { let mut tests = Vec::new(); let mut buf = OsString::from("tests/ui/"); - collect_ui_test_names(lint, rename_prefixed, &mut tests); + collect_ui_test_names(lint, ignored_prefixes, &mut tests); for &(ref name, is_file) in &tests { buf.push(name); if is_file { @@ -413,7 +420,7 @@ fn delete_test_files(lint: &str, rename_prefixed: bool) { buf.push("-toml/"); tests.clear(); - collect_ui_toml_test_names(lint, rename_prefixed, &mut tests); + collect_ui_toml_test_names(lint, ignored_prefixes, &mut tests); for (name, _) in &tests { buf.push(name); delete_dir_if_exists(buf.as_ref()); From 18f4ce438a248287513b582778810fe744dd0efd Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 13 Oct 2025 17:31:51 -0400 Subject: [PATCH 5/7] `clippy_dev`: Re-enable deleting the module declaration on deprecation and uplifting. --- clippy_dev/src/edit_lints.rs | 273 ++++++++++++--------------------- clippy_dev/src/parse/cursor.rs | 16 ++ 2 files changed, 117 insertions(+), 172 deletions(-) diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs index 14dd8b21df9a..fb1c1458c50c 100644 --- a/clippy_dev/src/edit_lints.rs +++ b/clippy_dev/src/edit_lints.rs @@ -6,9 +6,9 @@ use crate::utils::{ expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, }; use rustc_lexer::TokenKind; -use std::ffi::{OsStr, OsString}; -use std::path::{Path, PathBuf}; -use std::{fs, io}; +use std::ffi::OsString; +use std::fs; +use std::path::Path; /// Runs the `deprecate` command /// @@ -23,7 +23,7 @@ pub fn deprecate<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, name let mut lints = cx.find_lint_decls(); let (mut deprecated_lints, renamed_lints) = cx.read_deprecated_lints(); - let Some(lint) = lints.iter().find(|l| l.name == name) else { + let Some(lint_idx) = lints.iter().position(|l| l.name == name) else { eprintln!("error: failed to find lint `{name}`"); return; }; @@ -47,30 +47,17 @@ pub fn deprecate<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, name ), } - let mod_path = { - let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module)); - if mod_path.is_dir() { - mod_path = mod_path.join("mod"); - } - - mod_path.set_extension("rs"); - mod_path - }; - - if remove_lint_declaration(name, &mod_path, &mut lints).unwrap_or(false) { - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); - println!("info: `{name}` has successfully been deprecated"); - println!("note: you must run `cargo uitest` to update the test results"); - } else { - eprintln!("error: lint not found"); - } + remove_lint_declaration(lint_idx, &mut lints, &mut FileUpdater::default()); + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + println!("info: `{name}` has successfully been deprecated"); + println!("note: you must run `cargo uitest` to update the test results"); } pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { let mut lints = cx.find_lint_decls(); let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); - let Some(lint) = lints.iter().find(|l| l.name == old_name) else { + let Some(lint_idx) = lints.iter().position(|l| l.name == old_name) else { eprintln!("error: failed to find lint `{old_name}`"); return; }; @@ -99,23 +86,18 @@ pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam ), } - let mod_path = { - let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module)); - if mod_path.is_dir() { - mod_path = mod_path.join("mod"); + let mut updater = FileUpdater::default(); + let remove_mod = remove_lint_declaration(lint_idx, &mut lints, &mut updater); + let mut update_fn = uplift_update_fn(old_name, new_name, remove_mod); + for e in walk_dir_no_dot_or_target(".") { + let e = expect_action(e, ErrAction::Read, "."); + if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { + updater.update_file(e.path(), &mut update_fn); } - - mod_path.set_extension("rs"); - mod_path - }; - - if remove_lint_declaration(old_name, &mod_path, &mut lints).unwrap_or(false) { - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); - println!("info: `{old_name}` has successfully been uplifted"); - println!("note: you must run `cargo uitest` to update the test results"); - } else { - eprintln!("error: lint not found"); } + generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + println!("info: `{old_name}` has successfully been uplifted as `{new_name}`"); + println!("note: you must run `cargo uitest` to update the test results"); } /// Runs the `rename_lint` command. @@ -173,7 +155,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam }, } - let mut mod_edit = ModEdit::None; + let mut rename_mod = false; if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { let lint = &mut lints[lint_idx]; if lint.module.ends_with(old_name) @@ -185,7 +167,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam let mut new_path = lint.path.with_file_name(new_name).into_os_string(); new_path.push(".rs"); if try_rename_file(lint.path.as_ref(), new_path.as_ref()) { - mod_edit = ModEdit::Rename; + rename_mod = true; } lint.module = cx.str_buf.with(|buf| { @@ -212,7 +194,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam return; } - let mut update_fn = file_update_fn(old_name, new_name, mod_edit); + let mut update_fn = rename_update_fn(old_name, new_name, rename_mod); for e in walk_dir_no_dot_or_target(".") { let e = expect_action(e, ErrAction::Read, "."); if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { @@ -227,110 +209,38 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam println!("note: `cargo uibless` still needs to be run to update the test results"); } -fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec>) -> io::Result { - fn remove_lint(name: &str, lints: &mut Vec>) { - lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos)); - } - - fn remove_test_assets(name: &str) { - let test_file_stem = format!("tests/ui/{name}"); - let path = Path::new(&test_file_stem); - - // Some lints have their own directories, delete them - if path.is_dir() { - let _ = fs::remove_dir_all(path); - return; - } - - // Remove all related test files - let _ = fs::remove_file(path.with_extension("rs")); - let _ = fs::remove_file(path.with_extension("stderr")); - let _ = fs::remove_file(path.with_extension("fixed")); - } - - fn remove_impl_lint_pass(lint_name_upper: &str, content: &mut String) { - let impl_lint_pass_start = content.find("impl_lint_pass!").unwrap_or_else(|| { - content - .find("declare_lint_pass!") - .unwrap_or_else(|| panic!("failed to find `impl_lint_pass`")) - }); - let mut impl_lint_pass_end = content[impl_lint_pass_start..] - .find(']') - .expect("failed to find `impl_lint_pass` terminator"); - - impl_lint_pass_end += impl_lint_pass_start; - if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(lint_name_upper) { - let mut lint_name_end = impl_lint_pass_start + (lint_name_pos + lint_name_upper.len()); - for c in content[lint_name_end..impl_lint_pass_end].chars() { - // Remove trailing whitespace - if c == ',' || c.is_whitespace() { - lint_name_end += 1; - } else { - break; - } +/// Removes a lint's declaration and test files. Returns whether the module containing the +/// lint was deleted. +fn remove_lint_declaration(lint_idx: usize, lints: &mut Vec>, updater: &mut FileUpdater) -> bool { + let lint = lints.remove(lint_idx); + let delete_mod = if lints.iter().all(|l| l.module != lint.module) { + delete_file_if_exists(lint.path.as_ref()) + } else { + updater.update_file(&lint.path, &mut |_, src, dst| -> UpdateStatus { + let mut start = &src[..lint.declaration_range.start]; + if start.ends_with("\n\n") { + start = &start[..start.len() - 1]; } - - content.replace_range(impl_lint_pass_start + lint_name_pos..lint_name_end, ""); - } - } - - if path.exists() - && let Some(lint) = lints.iter().find(|l| l.name == name) - { - if lint.module == name { - // The lint name is the same as the file, we can just delete the entire file - fs::remove_file(path)?; - } else { - // We can't delete the entire file, just remove the declaration - - if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) { - // Remove clippy_lints/src/some_mod/some_lint.rs - let mut lint_mod_path = path.to_path_buf(); - lint_mod_path.set_file_name(name); - lint_mod_path.set_extension("rs"); - - let _ = fs::remove_file(lint_mod_path); + let mut end = &src[lint.declaration_range.end..]; + if end.starts_with("\n\n") { + end = &end[1..]; } - - let mut content = - fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy())); - - eprintln!( - "warn: you will have to manually remove any code related to `{name}` from `{}`", - path.display() - ); - - assert!( - content[lint.declaration_range].contains(&name.to_uppercase()), - "error: `{}` does not contain lint `{}`'s declaration", - path.display(), - lint.name - ); - - // Remove lint declaration (declare_clippy_lint!) - content.replace_range(lint.declaration_range, ""); - - // Remove the module declaration (mod xyz;) - let mod_decl = format!("\nmod {name};"); - content = content.replacen(&mod_decl, "", 1); - - remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content); - fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy())); - } - - remove_test_assets(name); - remove_lint(name, lints); - return Ok(true); - } - - Ok(false) -} - -#[derive(Clone, Copy)] -enum ModEdit { - None, - Delete, - Rename, + dst.push_str(start); + dst.push_str(end); + UpdateStatus::Changed + }); + false + }; + delete_test_files( + lint.name, + &lints[lint_idx..] + .iter() + .map(|l| l.name) + .take_while(|&n| n.starts_with(lint.name)) + .collect::>(), + ); + + delete_mod } fn collect_ui_test_names(lint: &str, ignored_prefixes: &[&str], dst: &mut Vec<(OsString, bool)>) { @@ -445,12 +355,50 @@ fn snake_to_pascal(s: &str) -> String { String::from_utf8(dst).unwrap() } -#[expect(clippy::too_many_lines)] -fn file_update_fn<'a, 'b>( +/// Creates an update function which replaces all instances of `clippy::old_name` with +/// `new_name`. +fn uplift_update_fn<'a>( + old_name: &'a str, + new_name: &'a str, + remove_mod: bool, +) -> impl use<'a> + FnMut(&Path, &str, &mut String) -> UpdateStatus { + move |_, src, dst| { + let mut copy_pos = 0u32; + let mut changed = false; + let mut cursor = Cursor::new(src); + while let Some(ident) = cursor.find_any_ident() { + match cursor.get_text(ident) { + "mod" + if remove_mod && cursor.match_all(&[cursor::Pat::Ident(old_name), cursor::Pat::Semi], &mut []) => + { + dst.push_str(&src[copy_pos as usize..ident.pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + if src[copy_pos as usize..].starts_with('\n') { + copy_pos += 1; + } + changed = true; + }, + "clippy" if cursor.match_all(&[cursor::Pat::DoubleColon, cursor::Pat::Ident(old_name)], &mut []) => { + dst.push_str(&src[copy_pos as usize..ident.pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + changed = true; + }, + + _ => {}, + } + } + dst.push_str(&src[copy_pos as usize..]); + UpdateStatus::from_changed(changed) + } +} + +fn rename_update_fn<'a>( old_name: &'a str, - new_name: &'b str, - mod_edit: ModEdit, -) -> impl use<'a, 'b> + FnMut(&Path, &str, &mut String) -> UpdateStatus { + new_name: &'a str, + rename_mod: bool, +) -> impl use<'a> + FnMut(&Path, &str, &mut String) -> UpdateStatus { let old_name_pascal = snake_to_pascal(old_name); let new_name_pascal = snake_to_pascal(new_name); let old_name_upper = old_name.to_ascii_uppercase(); @@ -481,34 +429,15 @@ fn file_update_fn<'a, 'b>( }, // mod lint_name "mod" => { - if !matches!(mod_edit, ModEdit::None) - && let Some(pos) = cursor.find_ident(old_name) - { - match mod_edit { - ModEdit::Rename => { - dst.push_str(&src[copy_pos as usize..pos as usize]); - dst.push_str(new_name); - copy_pos = cursor.pos(); - changed = true; - }, - ModEdit::Delete if cursor.match_pat(cursor::Pat::Semi) => { - let mut start = &src[copy_pos as usize..match_start as usize]; - if start.ends_with("\n\n") { - start = &start[..start.len() - 1]; - } - dst.push_str(start); - copy_pos = cursor.pos(); - if src[copy_pos as usize..].starts_with("\n\n") { - copy_pos += 1; - } - changed = true; - }, - ModEdit::Delete | ModEdit::None => {}, - } + if rename_mod && let Some(pos) = cursor.match_ident(old_name) { + dst.push_str(&src[copy_pos as usize..pos as usize]); + dst.push_str(new_name); + copy_pos = cursor.pos(); + changed = true; } }, // lint_name:: - name if matches!(mod_edit, ModEdit::Rename) && name == old_name => { + name if rename_mod && name == old_name => { let name_end = cursor.pos(); if cursor.match_pat(cursor::Pat::DoubleColon) { dst.push_str(&src[copy_pos as usize..match_start as usize]); diff --git a/clippy_dev/src/parse/cursor.rs b/clippy_dev/src/parse/cursor.rs index 6dc003f326de..2c142af4883a 100644 --- a/clippy_dev/src/parse/cursor.rs +++ b/clippy_dev/src/parse/cursor.rs @@ -219,6 +219,22 @@ impl<'txt> Cursor<'txt> { } } + /// Consume the returns the position of the next non-whitespace token if it's an + /// identifier. Returns `None` otherwise. + pub fn match_ident(&mut self, s: &str) -> Option { + loop { + match self.next_token.kind { + TokenKind::Ident if s == self.peek_text() => { + let pos = self.pos; + self.step(); + return Some(pos); + }, + TokenKind::Whitespace => self.step(), + _ => return None, + } + } + } + /// Continually attempt to match the pattern on subsequent tokens until a match is /// found. Returns whether the pattern was successfully matched. /// From f7f3328b7ab8ea06292caa25e790c87faf7b0faa Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Thu, 16 Oct 2025 01:31:12 -0400 Subject: [PATCH 6/7] `clippy_dev`: Stores parsed lints in a map indexed by name. --- clippy_dev/src/edit_lints.rs | 221 ++++++++++++++--------------- clippy_dev/src/lib.rs | 1 + clippy_dev/src/parse.rs | 106 +++++++++----- clippy_dev/src/update_lints.rs | 134 +++++++++-------- clippy_lints/src/declared_lints.rs | 30 ++-- 5 files changed, 262 insertions(+), 230 deletions(-) diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs index fb1c1458c50c..35459a1bbd70 100644 --- a/clippy_dev/src/edit_lints.rs +++ b/clippy_dev/src/edit_lints.rs @@ -1,11 +1,13 @@ use crate::parse::cursor::{self, Capture, Cursor}; -use crate::parse::{DeprecatedLint, Lint, ParseCx, RenamedLint}; +use crate::parse::{ActiveLint, DeprecatedLint, Lint, LintData, ParseCx, RenamedLint}; use crate::update_lints::generate_lint_files; use crate::utils::{ ErrAction, FileUpdater, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, }; +use core::mem; use rustc_lexer::TokenKind; +use std::collections::hash_map::Entry; use std::ffi::OsString; use std::fs; use std::path::Path; @@ -20,74 +22,58 @@ use std::path::Path; /// /// If a file path could not read from or written to pub fn deprecate<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, name: &'env str, reason: &'env str) { - let mut lints = cx.find_lint_decls(); - let (mut deprecated_lints, renamed_lints) = cx.read_deprecated_lints(); + let mut data = cx.parse_lint_decls(); - let Some(lint_idx) = lints.iter().position(|l| l.name == name) else { + let Entry::Occupied(mut lint) = data.lints.entry(name) else { eprintln!("error: failed to find lint `{name}`"); return; }; + let Lint::Active(prev_lint) = mem::replace( + lint.get_mut(), + Lint::Deprecated(DeprecatedLint { + reason, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }), + ) else { + eprintln!("error: `{name}` is already deprecated"); + return; + }; - let prefixed_name = cx.str_buf.with(|buf| { - buf.extend(["clippy::", name]); - cx.arena.alloc_str(buf) - }); - match deprecated_lints.binary_search_by(|x| x.name.cmp(prefixed_name)) { - Ok(_) => { - println!("`{name}` is already deprecated"); - return; - }, - Err(idx) => deprecated_lints.insert( - idx, - DeprecatedLint { - name: prefixed_name, - reason, - version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), - }, - ), - } - - remove_lint_declaration(lint_idx, &mut lints, &mut FileUpdater::default()); - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + remove_lint_declaration(name, &prev_lint, &data, &mut FileUpdater::default()); + generate_lint_files(UpdateMode::Change, &data); println!("info: `{name}` has successfully been deprecated"); println!("note: you must run `cargo uitest` to update the test results"); } pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { - let mut lints = cx.find_lint_decls(); - let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); + let mut data = cx.parse_lint_decls(); + + update_rename_targets( + &mut data, + cx.str_buf.with(|buf| { + buf.extend(["clippy::", old_name]); + cx.arena.alloc_str(buf) + }), + new_name, + ); - let Some(lint_idx) = lints.iter().position(|l| l.name == old_name) else { + let Entry::Occupied(mut lint) = data.lints.entry(old_name) else { eprintln!("error: failed to find lint `{old_name}`"); return; }; - - let old_name_prefixed = cx.str_buf.with(|buf| { - buf.extend(["clippy::", old_name]); - cx.arena.alloc_str(buf) - }); - for lint in &mut renamed_lints { - if lint.new_name == old_name_prefixed { - lint.new_name = new_name; - } - } - match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { - Ok(_) => { - println!("`{old_name}` is already deprecated"); - return; - }, - Err(idx) => renamed_lints.insert( - idx, - RenamedLint { - old_name: old_name_prefixed, - new_name, - version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), - }, - ), - } + let Lint::Active(prev_lint) = mem::replace( + lint.get_mut(), + Lint::Renamed(RenamedLint { + new_name, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }), + ) else { + eprintln!("error: `{old_name}` is already deprecated"); + return; + }; let mut updater = FileUpdater::default(); - let remove_mod = remove_lint_declaration(lint_idx, &mut lints, &mut updater); + let remove_mod = remove_lint_declaration(old_name, &prev_lint, &data, &mut updater); let mut update_fn = uplift_update_fn(old_name, new_name, remove_mod); for e in walk_dir_no_dot_or_target(".") { let e = expect_action(e, ErrAction::Read, "."); @@ -95,7 +81,7 @@ pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam updater.update_file(e.path(), &mut update_fn); } } - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + generate_lint_files(UpdateMode::Change, &data); println!("info: `{old_name}` has successfully been uplifted as `{new_name}`"); println!("note: you must run `cargo uitest` to update the test results"); } @@ -117,77 +103,59 @@ pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam /// * If `old_name` names a deprecated or renamed lint. pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { let mut updater = FileUpdater::default(); - let mut lints = cx.find_lint_decls(); - let (deprecated_lints, mut renamed_lints) = cx.read_deprecated_lints(); + let mut data = cx.parse_lint_decls(); - let Ok(lint_idx) = lints.binary_search_by(|x| x.name.cmp(old_name)) else { - panic!("could not find lint `{old_name}`"); - }; - - let old_name_prefixed = cx.str_buf.with(|buf| { - buf.extend(["clippy::", old_name]); - cx.arena.alloc_str(buf) - }); let new_name_prefixed = cx.str_buf.with(|buf| { buf.extend(["clippy::", new_name]); cx.arena.alloc_str(buf) }); + update_rename_targets( + &mut data, + cx.str_buf.with(|buf| { + buf.extend(["clippy::", old_name]); + cx.arena.alloc_str(buf) + }), + new_name_prefixed, + ); - for lint in &mut renamed_lints { - if lint.new_name == old_name_prefixed { - lint.new_name = new_name_prefixed; - } - } - match renamed_lints.binary_search_by(|x| x.old_name.cmp(old_name_prefixed)) { - Ok(_) => { - println!("`{old_name}` already has a rename registered"); - return; - }, - Err(idx) => { - renamed_lints.insert( - idx, - RenamedLint { - old_name: old_name_prefixed, - new_name: new_name_prefixed, - version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), - }, - ); - }, - } + let Entry::Occupied(mut lint) = data.lints.entry(old_name) else { + eprintln!("error: failed to find lint `{old_name}`"); + return; + }; + let Lint::Active(mut prev_lint) = mem::replace( + lint.get_mut(), + Lint::Renamed(RenamedLint { + new_name: new_name_prefixed, + version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), + }), + ) else { + eprintln!("error: `{old_name}` is already deprecated"); + return; + }; let mut rename_mod = false; - if lints.binary_search_by(|x| x.name.cmp(new_name)).is_err() { - let lint = &mut lints[lint_idx]; - if lint.module.ends_with(old_name) - && lint + if let Entry::Vacant(e) = data.lints.entry(new_name) { + if prev_lint.module.ends_with(old_name) + && prev_lint .path .file_stem() .is_some_and(|x| x.as_encoded_bytes() == old_name.as_bytes()) { - let mut new_path = lint.path.with_file_name(new_name).into_os_string(); + let mut new_path = prev_lint.path.with_file_name(new_name).into_os_string(); new_path.push(".rs"); - if try_rename_file(lint.path.as_ref(), new_path.as_ref()) { + if try_rename_file(prev_lint.path.as_ref(), new_path.as_ref()) { rename_mod = true; } - lint.module = cx.str_buf.with(|buf| { - buf.push_str(&lint.module[..lint.module.len() - old_name.len()]); + prev_lint.module = cx.str_buf.with(|buf| { + buf.push_str(&prev_lint.module[..prev_lint.module.len() - old_name.len()]); buf.push_str(new_name); cx.arena.alloc_str(buf) }); } + e.insert(Lint::Active(prev_lint)); - rename_test_files( - old_name, - new_name, - &lints[lint_idx + 1..] - .iter() - .map(|l| l.name) - .take_while(|&n| n.starts_with(old_name)) - .collect::>(), - ); - lints[lint_idx].name = new_name; - lints.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); + rename_test_files(old_name, new_name, &create_ignored_prefixes(old_name, &data)); } else { println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); println!("Since `{new_name}` already exists the existing code has not been changed"); @@ -201,7 +169,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam updater.update_file(e.path(), &mut update_fn); } } - generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); + generate_lint_files(UpdateMode::Change, &data); println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); println!("All code referencing the old name has been updated"); @@ -211,9 +179,14 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam /// Removes a lint's declaration and test files. Returns whether the module containing the /// lint was deleted. -fn remove_lint_declaration(lint_idx: usize, lints: &mut Vec>, updater: &mut FileUpdater) -> bool { - let lint = lints.remove(lint_idx); - let delete_mod = if lints.iter().all(|l| l.module != lint.module) { +fn remove_lint_declaration(name: &str, lint: &ActiveLint<'_>, data: &LintData<'_>, updater: &mut FileUpdater) -> bool { + let delete_mod = if data.lints.iter().all(|(_, l)| { + if let Lint::Active(l) = l { + l.module != lint.module + } else { + true + } + }) { delete_file_if_exists(lint.path.as_ref()) } else { updater.update_file(&lint.path, &mut |_, src, dst| -> UpdateStatus { @@ -231,18 +204,34 @@ fn remove_lint_declaration(lint_idx: usize, lints: &mut Vec>, updater: }); false }; - delete_test_files( - lint.name, - &lints[lint_idx..] - .iter() - .map(|l| l.name) - .take_while(|&n| n.starts_with(lint.name)) - .collect::>(), - ); + delete_test_files(name, &create_ignored_prefixes(name, data)); delete_mod } +/// Updates all renames to the old name to be renames to the new name. +/// +/// This is needed because rustc doesn't allow a lint to be renamed to a lint that has +/// also been renamed. +fn update_rename_targets<'cx>(data: &mut LintData<'cx>, old_name: &str, new_name: &'cx str) { + for lint in data.lints.values_mut() { + if let Lint::Renamed(lint) = lint + && lint.new_name == old_name + { + lint.new_name = new_name; + } + } +} + +/// Creates a list of prefixes to ignore when +fn create_ignored_prefixes<'cx>(name: &str, data: &LintData<'cx>) -> Vec<&'cx str> { + data.lints + .keys() + .copied() + .filter(|&x| x.len() > name.len() && x.starts_with(name)) + .collect() +} + fn collect_ui_test_names(lint: &str, ignored_prefixes: &[&str], dst: &mut Vec<(OsString, bool)>) { for e in fs::read_dir("tests/ui").expect("error reading `tests/ui`") { let e = e.expect("error reading `tests/ui`"); diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index fdab4235a97c..acd0c2f382e8 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -19,6 +19,7 @@ #![allow(clippy::missing_panics_doc)] extern crate rustc_arena; +extern crate rustc_data_structures; #[expect(unused_extern_crates, reason = "required to link to rustc crates")] extern crate rustc_driver; extern crate rustc_lexer; diff --git a/clippy_dev/src/parse.rs b/clippy_dev/src/parse.rs index de5caf4e1ef6..566eea336f0d 100644 --- a/clippy_dev/src/parse.rs +++ b/clippy_dev/src/parse.rs @@ -5,6 +5,7 @@ use crate::utils::{ErrAction, File, Scoped, expect_action, walk_dir_no_dot_or_ta use core::fmt::{Display, Write as _}; use core::range::Range; use rustc_arena::DroplessArena; +use rustc_data_structures::fx::FxHashMap; use std::fs; use std::path::{self, Path, PathBuf}; use std::str::pattern::Pattern; @@ -81,8 +82,7 @@ impl StrBuf { } } -pub struct Lint<'cx> { - pub name: &'cx str, +pub struct ActiveLint<'cx> { pub group: &'cx str, pub module: &'cx str, pub path: PathBuf, @@ -90,22 +90,34 @@ pub struct Lint<'cx> { } pub struct DeprecatedLint<'cx> { - pub name: &'cx str, pub reason: &'cx str, pub version: &'cx str, } pub struct RenamedLint<'cx> { - pub old_name: &'cx str, pub new_name: &'cx str, pub version: &'cx str, } +pub enum Lint<'cx> { + Active(ActiveLint<'cx>), + Deprecated(DeprecatedLint<'cx>), + Renamed(RenamedLint<'cx>), +} + +pub struct LintData<'cx> { + pub lints: FxHashMap<&'cx str, Lint<'cx>>, +} + impl<'cx> ParseCxImpl<'cx> { - /// Finds all lint declarations (`declare_clippy_lint!`) + /// Finds and parses all lint declarations. #[must_use] - pub fn find_lint_decls(&mut self) -> Vec> { - let mut lints = Vec::with_capacity(1000); + pub fn parse_lint_decls(&mut self) -> LintData<'cx> { + let mut data = LintData { + #[expect(clippy::default_trait_access)] + lints: FxHashMap::with_capacity_and_hasher(1000, Default::default()), + }; + let mut contents = String::new(); for e in expect_action(fs::read_dir("."), ErrAction::Read, ".") { let e = expect_action(e, ErrAction::Read, "."); @@ -143,17 +155,18 @@ impl<'cx> ParseCxImpl<'cx> { e.path(), File::open_read_to_cleared_string(e.path(), &mut contents), module, - &mut lints, + &mut data, ); } } } - lints.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); - lints + + self.read_deprecated_lints(&mut data); + data } /// Parse a source file looking for `declare_clippy_lint` macro invocations. - fn parse_clippy_lint_decls(&mut self, path: &Path, contents: &str, module: &'cx str, lints: &mut Vec>) { + fn parse_clippy_lint_decls(&mut self, path: &Path, contents: &str, module: &'cx str, data: &mut LintData<'cx>) { #[allow(clippy::enum_glob_use)] use cursor::Pat::*; #[rustfmt::skip] @@ -170,19 +183,24 @@ impl<'cx> ParseCxImpl<'cx> { let mut captures = [Capture::EMPTY; 2]; while let Some(start) = cursor.find_ident("declare_clippy_lint") { if cursor.match_all(DECL_TOKENS, &mut captures) && cursor.find_pat(CloseBrace) { - lints.push(Lint { - name: self.str_buf.alloc_ascii_lower(self.arena, cursor.get_text(captures[0])), - group: self.arena.alloc_str(cursor.get_text(captures[1])), - module, - path: path.into(), - declaration_range: start as usize..cursor.pos() as usize, - }); + assert!( + data.lints + .insert( + self.str_buf.alloc_ascii_lower(self.arena, cursor.get_text(captures[0])), + Lint::Active(ActiveLint { + group: self.arena.alloc_str(cursor.get_text(captures[1])), + module, + path: path.into(), + declaration_range: start as usize..cursor.pos() as usize, + }), + ) + .is_none() + ); } } } - #[must_use] - pub fn read_deprecated_lints(&mut self) -> (Vec>, Vec>) { + fn read_deprecated_lints(&mut self, data: &mut LintData<'cx>) { #[allow(clippy::enum_glob_use)] use cursor::Pat::*; #[rustfmt::skip] @@ -204,8 +222,6 @@ impl<'cx> ParseCxImpl<'cx> { ]; let path = "clippy_lints/src/deprecated_lints.rs"; - let mut deprecated = Vec::with_capacity(30); - let mut renamed = Vec::with_capacity(80); let mut contents = String::new(); File::open_read_to_cleared_string(path, &mut contents); @@ -220,11 +236,17 @@ impl<'cx> ParseCxImpl<'cx> { if cursor.find_ident("declare_with_version").is_some() && cursor.match_all(DEPRECATED_TOKENS, &mut []) { while cursor.match_all(DECL_TOKENS, &mut captures) { - deprecated.push(DeprecatedLint { - name: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[1])), - reason: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[2])), - version: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[0])), - }); + assert!( + data.lints + .insert( + self.parse_clippy_lint_name(path.as_ref(), cursor.get_text(captures[1])), + Lint::Deprecated(DeprecatedLint { + reason: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[2])), + version: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[0])), + }), + ) + .is_none() + ); } } else { panic!("error reading deprecated lints"); @@ -232,19 +254,21 @@ impl<'cx> ParseCxImpl<'cx> { if cursor.find_ident("declare_with_version").is_some() && cursor.match_all(RENAMED_TOKENS, &mut []) { while cursor.match_all(DECL_TOKENS, &mut captures) { - renamed.push(RenamedLint { - old_name: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[1])), - new_name: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[2])), - version: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[0])), - }); + assert!( + data.lints + .insert( + self.parse_clippy_lint_name(path.as_ref(), cursor.get_text(captures[1])), + Lint::Renamed(RenamedLint { + new_name: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[2])), + version: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[0])), + }), + ) + .is_none() + ); } } else { panic!("error reading renamed lints"); } - - deprecated.sort_by(|lhs, rhs| lhs.name.cmp(rhs.name)); - renamed.sort_by(|lhs, rhs| lhs.old_name.cmp(rhs.old_name)); - (deprecated, renamed) } /// Removes the line splices and surrounding quotes from a string literal @@ -282,4 +306,14 @@ impl<'cx> ParseCxImpl<'cx> { ); value } + + fn parse_clippy_lint_name(&mut self, path: &Path, s: &str) -> &'cx str { + match self.parse_str_single_line(path, s).strip_prefix("clippy::") { + Some(x) => x, + None => panic!( + "error parsing `{}`: `{s}` should be a string starting with `clippy::`", + path.display() + ), + } + } } diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 3d0da6846114..ccc7a1d95355 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -1,5 +1,5 @@ use crate::parse::cursor::Cursor; -use crate::parse::{DeprecatedLint, Lint, ParseCx, RenamedLint}; +use crate::parse::{Lint, LintData, ParseCx}; use crate::utils::{FileUpdater, UpdateMode, UpdateStatus, update_text_region_fn}; use itertools::Itertools; use std::collections::HashSet; @@ -22,55 +22,62 @@ const DOCS_LINK: &str = "https://rust-lang.github.io/rust-clippy/master/index.ht /// /// Panics if a file path could not read from or then written to pub fn update(cx: ParseCx<'_>, update_mode: UpdateMode) { - let lints = cx.find_lint_decls(); - let (deprecated, renamed) = cx.read_deprecated_lints(); - generate_lint_files(update_mode, &lints, &deprecated, &renamed); + let data = cx.parse_lint_decls(); + generate_lint_files(update_mode, &data); } #[expect(clippy::too_many_lines)] -pub fn generate_lint_files( - update_mode: UpdateMode, - lints: &[Lint<'_>], - deprecated: &[DeprecatedLint<'_>], - renamed: &[RenamedLint<'_>], -) { +pub fn generate_lint_files(update_mode: UpdateMode, data: &LintData<'_>) { let mut updater = FileUpdater::default(); + + let mut lints: Vec<_> = data.lints.iter().map(|(&x, y)| (x, y)).collect(); + lints.sort_by_key(|&(x, _)| x); updater.update_file_checked( "cargo dev update_lints", update_mode, - "README.md", - &mut update_text_region_fn("[There are over ", " lints included in this crate!]", |dst| { - write!(dst, "{}", round_to_fifty(lints.len())).unwrap(); - }), + "CHANGELOG.md", + &mut update_text_region_fn( + "\n", + "", + |dst| { + for &(lint, _) in &lints { + writeln!(dst, "[`{lint}`]: {DOCS_LINK}#{lint}").unwrap(); + } + }, + ), ); + + let mut active = Vec::with_capacity(lints.len()); + let mut deprecated = Vec::with_capacity(lints.len() / 8); + let mut renamed = Vec::with_capacity(lints.len() / 8); + for &(name, lint) in &lints { + match lint { + Lint::Active(lint) => active.push((name, lint)), + Lint::Deprecated(lint) => deprecated.push((name, lint)), + Lint::Renamed(lint) => renamed.push((name, lint)), + } + } + active.sort_by_key(|&(_, lint)| lint.module); + + // Round to avoid updating the readme every time a lint is added/deprecated. + let lint_count = active.len() / 50 * 50; updater.update_file_checked( "cargo dev update_lints", update_mode, - "book/src/README.md", + "README.md", &mut update_text_region_fn("[There are over ", " lints included in this crate!]", |dst| { - write!(dst, "{}", round_to_fifty(lints.len())).unwrap(); + write!(dst, "{lint_count}").unwrap(); }), ); updater.update_file_checked( "cargo dev update_lints", update_mode, - "CHANGELOG.md", - &mut update_text_region_fn( - "\n", - "", - |dst| { - for lint in lints - .iter() - .map(|l| l.name) - .chain(deprecated.iter().filter_map(|l| l.name.strip_prefix("clippy::"))) - .chain(renamed.iter().filter_map(|l| l.old_name.strip_prefix("clippy::"))) - .sorted() - { - writeln!(dst, "[`{lint}`]: {DOCS_LINK}#{lint}").unwrap(); - } - }, - ), + "book/src/README.md", + &mut update_text_region_fn("[There are over ", " lints included in this crate!]", |dst| { + write!(dst, "{lint_count}").unwrap(); + }), ); + updater.update_file_checked( "cargo dev update_lints", update_mode, @@ -84,11 +91,11 @@ pub fn generate_lint_files( ); dst.push_str(&src[..cursor.pos() as usize]); dst.push_str("! { DEPRECATED(DEPRECATED_VERSION) = [\n"); - for lint in deprecated { + for &(name, data) in &deprecated { write!( dst, - " #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n", - lint.version, lint.name, lint.reason, + " #[clippy::version = \"{}\"]\n (\"clippy::{name}\", \"{}\"),\n", + data.version, data.reason, ) .unwrap(); } @@ -98,11 +105,11 @@ pub fn generate_lint_files( declare_with_version! { RENAMED(RENAMED_VERSION) = [\n\ ", ); - for lint in renamed { + for &(name, data) in &renamed { write!( dst, - " #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n", - lint.version, lint.old_name, lint.new_name, + " #[clippy::version = \"{}\"]\n (\"clippy::{name}\", \"{}\"),\n", + data.version, data.new_name, ) .unwrap(); } @@ -116,8 +123,8 @@ pub fn generate_lint_files( "tests/ui/deprecated.rs", &mut |_, src, dst| { dst.push_str(GENERATED_FILE_COMMENT); - for lint in deprecated { - writeln!(dst, "#![warn({})] //~ ERROR: lint `{}`", lint.name, lint.name).unwrap(); + for &(lint, _) in &deprecated { + writeln!(dst, "#![warn(clippy::{lint})] //~ ERROR: lint `clippy::{lint}`").unwrap(); } dst.push_str("\nfn main() {}\n"); UpdateStatus::from_changed(src != dst) @@ -131,25 +138,28 @@ pub fn generate_lint_files( let mut seen_lints = HashSet::new(); dst.push_str(GENERATED_FILE_COMMENT); dst.push_str("#![allow(clippy::duplicated_attributes)]\n"); - for lint in renamed { + for &(_, lint) in &renamed { if seen_lints.insert(lint.new_name) { writeln!(dst, "#![allow({})]", lint.new_name).unwrap(); } } seen_lints.clear(); - for lint in renamed { - if seen_lints.insert(lint.old_name) { - writeln!(dst, "#![warn({})] //~ ERROR: lint `{}`", lint.old_name, lint.old_name).unwrap(); + for &(lint, _) in &renamed { + if seen_lints.insert(lint) { + writeln!(dst, "#![warn(clippy::{lint})] //~ ERROR: lint `clippy::{lint}`").unwrap(); } } dst.push_str("\nfn main() {}\n"); UpdateStatus::from_changed(src != dst) }, ); - for (crate_name, lints) in lints.iter().into_group_map_by(|&l| { - let Some(path::Component::Normal(name)) = l.path.components().next() else { + for (crate_name, lints) in active.iter().copied().into_group_map_by(|&(_, lint)| { + let Some(path::Component::Normal(name)) = lint.path.components().next() else { // All paths should start with `{crate_name}/src` when parsed from `find_lint_decls` - panic!("internal error: can't read crate name from path `{}`", l.path.display()); + panic!( + "internal error: can't read crate name from path `{}`", + lint.path.display() + ); }; name }) { @@ -161,14 +171,12 @@ pub fn generate_lint_files( "// begin lints modules, do not remove this comment, it's used in `update_lints`\n", "// end lints modules, do not remove this comment, it's used in `update_lints`", |dst| { - for lint_mod in lints - .iter() - .filter(|l| !l.module.is_empty()) - .map(|l| l.module.split_once("::").map_or(l.module, |x| x.0)) - .sorted() - .dedup() - { - writeln!(dst, "mod {lint_mod};").unwrap(); + let mut prev = ""; + for &(_, lint) in &lints { + if lint.module != prev { + writeln!(dst, "mod {};", lint.module).unwrap(); + prev = lint.module; + } } }, ), @@ -180,11 +188,15 @@ pub fn generate_lint_files( &mut |_, src, dst| { dst.push_str(GENERATED_FILE_COMMENT); dst.push_str("pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[\n"); - for (module_path, lint_name) in lints.iter().map(|l| (&l.module, l.name.to_uppercase())).sorted() { - if module_path.is_empty() { - writeln!(dst, " crate::{lint_name}_INFO,").unwrap(); + let mut buf = String::new(); + for &(name, lint) in &lints { + buf.clear(); + buf.push_str(name); + buf.make_ascii_uppercase(); + if lint.module.is_empty() { + writeln!(dst, " crate::{buf}_INFO,").unwrap(); } else { - writeln!(dst, " crate::{module_path}::{lint_name}_INFO,").unwrap(); + writeln!(dst, " crate::{}::{buf}_INFO,", lint.module).unwrap(); } } dst.push_str("];\n"); @@ -193,7 +205,3 @@ pub fn generate_lint_files( ); } } - -fn round_to_fifty(count: usize) -> usize { - count / 50 * 50 -} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bd2971fa150d..f603866cf7b9 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -99,9 +99,9 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::dereference::NEEDLESS_BORROW_INFO, crate::dereference::REF_BINDING_TO_REFERENCE_INFO, crate::derivable_impls::DERIVABLE_IMPLS_INFO, - crate::derive::DERIVED_HASH_WITH_MANUAL_EQ_INFO, crate::derive::DERIVE_ORD_XOR_PARTIAL_ORD_INFO, crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO, + crate::derive::DERIVED_HASH_WITH_MANUAL_EQ_INFO, crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO, crate::derive::UNSAFE_DERIVE_DESERIALIZE_INFO, crate::disallowed_macros::DISALLOWED_MACROS_INFO, @@ -201,8 +201,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::if_not_else::IF_NOT_ELSE_INFO, crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO, crate::ifs::BRANCHES_SHARING_CODE_INFO, - crate::ifs::IFS_SAME_COND_INFO, crate::ifs::IF_SAME_THEN_ELSE_INFO, + crate::ifs::IFS_SAME_COND_INFO, crate::ifs::SAME_FUNCTIONS_IN_IF_CONDITION_INFO, crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO, crate::impl_hash_with_borrow_str_and_bytes::IMPL_HASH_BORROW_WITH_STR_AND_BYTES_INFO, @@ -333,8 +333,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::matches::MATCH_SAME_ARMS_INFO, crate::matches::MATCH_SINGLE_BINDING_INFO, crate::matches::MATCH_STR_CASE_MISMATCH_INFO, - crate::matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS_INFO, crate::matches::MATCH_WILD_ERR_ARM_INFO, + crate::matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS_INFO, crate::matches::NEEDLESS_MATCH_INFO, crate::matches::REDUNDANT_GUARDS_INFO, crate::matches::REDUNDANT_PATTERN_MATCHING_INFO, @@ -356,9 +356,9 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::CHARS_LAST_CMP_INFO, crate::methods::CHARS_NEXT_CMP_INFO, crate::methods::CLEAR_WITH_DRAIN_INFO, - crate::methods::CLONED_INSTEAD_OF_COPIED_INFO, crate::methods::CLONE_ON_COPY_INFO, crate::methods::CLONE_ON_REF_PTR_INFO, + crate::methods::CLONED_INSTEAD_OF_COPIED_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, crate::methods::CONST_IS_EMPTY_INFO, crate::methods::DOUBLE_ENDED_ITERATOR_LAST_INFO, @@ -386,7 +386,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::IO_OTHER_ERROR_INFO, crate::methods::IP_CONSTANT_INFO, crate::methods::IS_DIGIT_ASCII_RADIX_INFO, - crate::methods::ITERATOR_STEP_BY_ZERO_INFO, crate::methods::ITER_CLONED_COLLECT_INFO, crate::methods::ITER_COUNT_INFO, crate::methods::ITER_FILTER_IS_OK_INFO, @@ -402,9 +401,10 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::ITER_SKIP_NEXT_INFO, crate::methods::ITER_SKIP_ZERO_INFO, crate::methods::ITER_WITH_DRAIN_INFO, + crate::methods::ITERATOR_STEP_BY_ZERO_INFO, crate::methods::JOIN_ABSOLUTE_PATHS_INFO, - crate::methods::MANUAL_CONTAINS_INFO, crate::methods::MANUAL_C_STR_LITERALS_INFO, + crate::methods::MANUAL_CONTAINS_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, crate::methods::MANUAL_INSPECT_INFO, @@ -433,8 +433,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::NEEDLESS_OPTION_TAKE_INFO, crate::methods::NEEDLESS_SPLITN_INFO, crate::methods::NEW_RET_NO_SELF_INFO, - crate::methods::NONSENSICAL_OPEN_OPTIONS_INFO, crate::methods::NO_EFFECT_REPLACE_INFO, + crate::methods::NONSENSICAL_OPEN_OPTIONS_INFO, crate::methods::OBFUSCATED_IF_ELSE_INFO, crate::methods::OK_EXPECT_INFO, crate::methods::OPTION_AS_REF_CLONED_INFO, @@ -447,8 +447,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::PATH_ENDS_WITH_EXT_INFO, crate::methods::PTR_OFFSET_WITH_CAST_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, - crate::methods::READONLY_WRITE_LOCK_INFO, crate::methods::READ_LINE_WITHOUT_TRIM_INFO, + crate::methods::READONLY_WRITE_LOCK_INFO, crate::methods::REDUNDANT_AS_STR_INFO, crate::methods::REDUNDANT_ITER_CLONED_INFO, crate::methods::REPEAT_ONCE_INFO, @@ -463,9 +463,9 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::SKIP_WHILE_NEXT_INFO, crate::methods::SLICED_STRING_AS_BYTES_INFO, crate::methods::STABLE_SORT_PRIMITIVE_INFO, + crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, crate::methods::STRING_LIT_CHARS_ANY_INFO, - crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, crate::methods::SUSPICIOUS_OPEN_OPTIONS_INFO, @@ -633,8 +633,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::ranges::RANGE_MINUS_ONE_INFO, crate::ranges::RANGE_PLUS_ONE_INFO, crate::ranges::REVERSED_EMPTY_RANGES_INFO, - crate::raw_strings::NEEDLESS_RAW_STRINGS_INFO, crate::raw_strings::NEEDLESS_RAW_STRING_HASHES_INFO, + crate::raw_strings::NEEDLESS_RAW_STRINGS_INFO, crate::rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT_INFO, crate::read_zero_byte_vec::READ_ZERO_BYTE_VEC_INFO, crate::redundant_async_block::REDUNDANT_ASYNC_BLOCK_INFO, @@ -686,12 +686,12 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO, crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO, crate::string_patterns::SINGLE_CHAR_PATTERN_INFO, + crate::strings::STR_TO_STRING_INFO, crate::strings::STRING_ADD_INFO, crate::strings::STRING_ADD_ASSIGN_INFO, crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO, crate::strings::STRING_LIT_AS_BYTES_INFO, crate::strings::STRING_SLICE_INFO, - crate::strings::STR_TO_STRING_INFO, crate::strings::TRIM_SPLIT_WHITESPACE_INFO, crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO, crate::suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS_INFO, @@ -715,7 +715,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::transmute::CROSSPOINTER_TRANSMUTE_INFO, crate::transmute::EAGER_TRANSMUTE_INFO, crate::transmute::MISSING_TRANSMUTE_ANNOTATIONS_INFO, - crate::transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS_INFO, crate::transmute::TRANSMUTE_BYTES_TO_STR_INFO, crate::transmute::TRANSMUTE_INT_TO_BOOL_INFO, crate::transmute::TRANSMUTE_INT_TO_NON_ZERO_INFO, @@ -723,6 +722,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::transmute::TRANSMUTE_PTR_TO_PTR_INFO, crate::transmute::TRANSMUTE_PTR_TO_REF_INFO, crate::transmute::TRANSMUTE_UNDEFINED_REPR_INFO, + crate::transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS_INFO, crate::transmute::TRANSMUTING_NULL_INFO, crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO, crate::transmute::USELESS_TRANSMUTE_INFO, @@ -780,20 +780,20 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::vec::USELESS_VEC_INFO, crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO, crate::visibility::NEEDLESS_PUB_SELF_INFO, - crate::visibility::PUB_WITHOUT_SHORTHAND_INFO, crate::visibility::PUB_WITH_SHORTHAND_INFO, + crate::visibility::PUB_WITHOUT_SHORTHAND_INFO, crate::volatile_composites::VOLATILE_COMPOSITES_INFO, crate::wildcard_imports::ENUM_GLOB_USE_INFO, crate::wildcard_imports::WILDCARD_IMPORTS_INFO, - crate::write::PRINTLN_EMPTY_STRING_INFO, crate::write::PRINT_LITERAL_INFO, crate::write::PRINT_STDERR_INFO, crate::write::PRINT_STDOUT_INFO, crate::write::PRINT_WITH_NEWLINE_INFO, + crate::write::PRINTLN_EMPTY_STRING_INFO, crate::write::USE_DEBUG_INFO, - crate::write::WRITELN_EMPTY_STRING_INFO, crate::write::WRITE_LITERAL_INFO, crate::write::WRITE_WITH_NEWLINE_INFO, + crate::write::WRITELN_EMPTY_STRING_INFO, crate::zero_div_zero::ZERO_DIVIDED_BY_ZERO_INFO, crate::zero_repeat_side_effects::ZERO_REPEAT_SIDE_EFFECTS_INFO, crate::zero_sized_map_values::ZERO_SIZED_MAP_VALUES_INFO, From 19ec3f72f194b4e3f02a4cab070a69f3c22800fc Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 20 Oct 2025 15:58:23 -0400 Subject: [PATCH 7/7] `clippy_dev`: Use types to differentiate rustc and clippy lints. --- clippy_dev/src/edit_lints.rs | 35 ++++++--------------- clippy_dev/src/parse.rs | 57 ++++++++++++++++++++++++++++++++-- clippy_dev/src/update_lints.rs | 5 +-- 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/clippy_dev/src/edit_lints.rs b/clippy_dev/src/edit_lints.rs index 35459a1bbd70..411bf530b22b 100644 --- a/clippy_dev/src/edit_lints.rs +++ b/clippy_dev/src/edit_lints.rs @@ -1,5 +1,5 @@ use crate::parse::cursor::{self, Capture, Cursor}; -use crate::parse::{ActiveLint, DeprecatedLint, Lint, LintData, ParseCx, RenamedLint}; +use crate::parse::{ActiveLint, DeprecatedLint, Lint, LintData, LintName, ParseCx, RenamedLint}; use crate::update_lints::generate_lint_files; use crate::utils::{ ErrAction, FileUpdater, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, @@ -48,14 +48,7 @@ pub fn deprecate<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, name pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_name: &'env str, new_name: &'env str) { let mut data = cx.parse_lint_decls(); - update_rename_targets( - &mut data, - cx.str_buf.with(|buf| { - buf.extend(["clippy::", old_name]); - cx.arena.alloc_str(buf) - }), - new_name, - ); + update_rename_targets(&mut data, old_name, LintName::new_rustc(new_name)); let Entry::Occupied(mut lint) = data.lints.entry(old_name) else { eprintln!("error: failed to find lint `{old_name}`"); @@ -64,7 +57,7 @@ pub fn uplift<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam let Lint::Active(prev_lint) = mem::replace( lint.get_mut(), Lint::Renamed(RenamedLint { - new_name, + new_name: LintName::new_rustc(new_name), version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), }), ) else { @@ -105,18 +98,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam let mut updater = FileUpdater::default(); let mut data = cx.parse_lint_decls(); - let new_name_prefixed = cx.str_buf.with(|buf| { - buf.extend(["clippy::", new_name]); - cx.arena.alloc_str(buf) - }); - update_rename_targets( - &mut data, - cx.str_buf.with(|buf| { - buf.extend(["clippy::", old_name]); - cx.arena.alloc_str(buf) - }), - new_name_prefixed, - ); + update_rename_targets(&mut data, old_name, LintName::new_clippy(new_name)); let Entry::Occupied(mut lint) = data.lints.entry(old_name) else { eprintln!("error: failed to find lint `{old_name}`"); @@ -125,7 +107,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam let Lint::Active(mut prev_lint) = mem::replace( lint.get_mut(), Lint::Renamed(RenamedLint { - new_name: new_name_prefixed, + new_name: LintName::new_clippy(new_name), version: cx.str_buf.alloc_display(cx.arena, clippy_version.rust_display()), }), ) else { @@ -157,7 +139,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam rename_test_files(old_name, new_name, &create_ignored_prefixes(old_name, &data)); } else { - println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); + println!("Renamed `{old_name}` to `{new_name}`"); println!("Since `{new_name}` already exists the existing code has not been changed"); return; } @@ -171,7 +153,7 @@ pub fn rename<'cx, 'env: 'cx>(cx: ParseCx<'cx>, clippy_version: Version, old_nam } generate_lint_files(UpdateMode::Change, &data); - println!("Renamed `clippy::{old_name}` to `clippy::{new_name}`"); + println!("Renamed `{old_name}` to `{new_name}`"); println!("All code referencing the old name has been updated"); println!("Make sure to inspect the results as some things may have been missed"); println!("note: `cargo uibless` still needs to be run to update the test results"); @@ -213,7 +195,8 @@ fn remove_lint_declaration(name: &str, lint: &ActiveLint<'_>, data: &LintData<'_ /// /// This is needed because rustc doesn't allow a lint to be renamed to a lint that has /// also been renamed. -fn update_rename_targets<'cx>(data: &mut LintData<'cx>, old_name: &str, new_name: &'cx str) { +fn update_rename_targets<'cx>(data: &mut LintData<'cx>, old_name: &str, new_name: LintName<'cx>) { + let old_name = LintName::new_clippy(old_name); for lint in data.lints.values_mut() { if let Lint::Renamed(lint) = lint && lint.new_name == old_name diff --git a/clippy_dev/src/parse.rs b/clippy_dev/src/parse.rs index 566eea336f0d..ffb50784a9a7 100644 --- a/clippy_dev/src/parse.rs +++ b/clippy_dev/src/parse.rs @@ -2,7 +2,7 @@ pub mod cursor; use self::cursor::{Capture, Cursor}; use crate::utils::{ErrAction, File, Scoped, expect_action, walk_dir_no_dot_or_target}; -use core::fmt::{Display, Write as _}; +use core::fmt::{self, Display, Write as _}; use core::range::Range; use rustc_arena::DroplessArena; use rustc_data_structures::fx::FxHashMap; @@ -82,6 +82,48 @@ impl StrBuf { } } +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub enum LintTool { + Rustc, + Clippy, +} +impl LintTool { + /// Gets the namespace prefix to use when naming a lint including the `::`. + pub fn prefix(self) -> &'static str { + match self { + Self::Rustc => "", + Self::Clippy => "clippy::", + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub struct LintName<'cx> { + pub name: &'cx str, + pub tool: LintTool, +} +impl<'cx> LintName<'cx> { + pub fn new_rustc(name: &'cx str) -> Self { + Self { + name, + tool: LintTool::Rustc, + } + } + + pub fn new_clippy(name: &'cx str) -> Self { + Self { + name, + tool: LintTool::Clippy, + } + } +} +impl Display for LintName<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.tool.prefix())?; + f.write_str(self.name) + } +} + pub struct ActiveLint<'cx> { pub group: &'cx str, pub module: &'cx str, @@ -95,7 +137,7 @@ pub struct DeprecatedLint<'cx> { } pub struct RenamedLint<'cx> { - pub new_name: &'cx str, + pub new_name: LintName<'cx>, pub version: &'cx str, } @@ -259,7 +301,7 @@ impl<'cx> ParseCxImpl<'cx> { .insert( self.parse_clippy_lint_name(path.as_ref(), cursor.get_text(captures[1])), Lint::Renamed(RenamedLint { - new_name: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[2])), + new_name: self.parse_lint_name(path.as_ref(), cursor.get_text(captures[2])), version: self.parse_str_single_line(path.as_ref(), cursor.get_text(captures[0])), }), ) @@ -316,4 +358,13 @@ impl<'cx> ParseCxImpl<'cx> { ), } } + + fn parse_lint_name(&mut self, path: &Path, s: &str) -> LintName<'cx> { + let s = self.parse_str_single_line(path, s); + let (name, tool) = match s.strip_prefix("clippy::") { + Some(s) => (s, LintTool::Clippy), + None => (s, LintTool::Rustc), + }; + LintName { name, tool } + } } diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index ccc7a1d95355..a4cf15058986 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -143,11 +143,8 @@ pub fn generate_lint_files(update_mode: UpdateMode, data: &LintData<'_>) { writeln!(dst, "#![allow({})]", lint.new_name).unwrap(); } } - seen_lints.clear(); for &(lint, _) in &renamed { - if seen_lints.insert(lint) { - writeln!(dst, "#![warn(clippy::{lint})] //~ ERROR: lint `clippy::{lint}`").unwrap(); - } + writeln!(dst, "#![warn(clippy::{lint})] //~ ERROR: lint `clippy::{lint}`").unwrap(); } dst.push_str("\nfn main() {}\n"); UpdateStatus::from_changed(src != dst)