Improve cargo dev rename_lint

* Rename the module containing the lint if it has the same name
* Check for identifier boundaries when replacing the lint name
* Don't require a new name when uplifting lints

Author: Jarcho

clippy_dev/Cargo.toml

@@ -4,6 +4,7 @@ version = "0.0.1"
 edition = "2021"
 
 [dependencies]
+aho-corasick = "0.7"
 clap = "2.33"
 indoc = "1.0"
 itertools = "0.10.1"

clippy_dev/src/lib.rs

@@ -1,3 +1,4 @@
+#![feature(let_chains)]
 #![feature(let_else)]
 #![feature(once_cell)]
 #![feature(rustc_private)]

clippy_dev/src/main.rs

@@ -61,7 +61,7 @@ fn main() {
         },
         ("rename_lint", Some(matches)) => {
             let old_name = matches.value_of("old_name").unwrap();
-            let new_name = matches.value_of("new_name").unwrap();
+            let new_name = matches.value_of("new_name").unwrap_or(old_name);
             let uplift = matches.is_present("uplift");
             update_lints::rename(old_name, new_name, uplift);
         },
@@ -250,13 +250,13 @@ fn get_clap_config<'a>() -> ArgMatches<'a> {
                 .arg(
                     Arg::with_name("new_name")
                         .index(2)
-                        .required(true)
+                        .required_unless("uplift")
                         .help("The new name of the lint"),
                 )
                 .arg(
                     Arg::with_name("uplift")
                         .long("uplift")
-                        .help("This lint was uplifted into rustc"),
+                        .help("This lint will be uplifted into rustc"),
                 ),
         )
         .get_matches()

clippy_dev/src/update_lints.rs

@@ -1,9 +1,11 @@
-use core::fmt::Write;
+use aho_corasick::AhoCorasickBuilder;
+use core::fmt::Write as _;
 use itertools::Itertools;
 use rustc_lexer::{tokenize, unescape, LiteralKind, TokenKind};
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsStr;
 use std::fs;
+use std::io::Write as _;
 use std::path::{Path, PathBuf};
 use walkdir::{DirEntry, WalkDir};
 
@@ -143,12 +145,20 @@ pub fn print_lints() {
 
 /// 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.
+#[allow(clippy::too_many_lines)]
 pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
     if let Some((prefix, _)) = old_name.split_once("::") {
         panic!("`{}` should not contain the `{}` prefix", old_name, prefix);
@@ -181,17 +191,17 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
     // Renamed lints and deprecated lints shouldn't have been found in the lint list, but check just in
     // case.)
     assert!(
-        renamed_lints.iter().any(|l| lint.old_name == l.old_name),
+        !renamed_lints.iter().any(|l| lint.old_name == l.old_name),
         "`{}` has already been renamed",
         old_name
     );
     assert!(
-        deprecated_lints.iter().any(|l| lint.old_name == l.name),
+        !deprecated_lints.iter().any(|l| lint.old_name == l.name),
         "`{}` has already been deprecated",
         old_name
     );
 
-    // Update all lint level attributes.
+    // Update all lint level attributes. (`clippy::lint_name`)
     for file in WalkDir::new(clippy_project_root())
         .into_iter()
         .map(Result::unwrap)
@@ -205,8 +215,8 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
         let path = file.path();
         let contents =
             fs::read_to_string(path).unwrap_or_else(|e| panic!("Cannot read from `{}`: {}", path.display(), e));
-        if contents.contains(&lint.old_name) {
-            fs::write(path, contents.replace(&lint.old_name, &lint.new_name).as_bytes())
+        if let Some(contents) = replace_ident_like(&contents, &[(&lint.old_name, &lint.new_name)]) {
+            fs::write(path, contents.as_bytes())
                 .unwrap_or_else(|e| panic!("Cannot write to `{}`: {}", path.display(), e));
         }
     }
@@ -222,24 +232,53 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
 
     if found_new_name {
         println!(
-            "`{}` is already defined. Removal the old lint code in `clippy_lints` will have to be done manually",
+            "`{}` is already defined. The old linting code inside `clippy_lints` has to be updated/removed manually",
             new_name
         );
+        // Still update the test file
         let contents = gen_renamed_lints_test(&renamed_lints);
         fs::write("tests/ui/rename.rs", contents.as_bytes())
             .unwrap_or_else(|e| panic!("error writing file `tests/ui/rename.rs`: {}", e));
     } else {
-        // Update the lint struct name
-        lints[old_lint_index].name = new_name.into();
-        let old_name = old_name.to_uppercase();
-        let new_name = new_name.to_uppercase();
+        // Rename the lint struct and the containing module if it shares the lint name across all files.
+        let lint = &mut lints[old_lint_index];
+        let old_name_upper = old_name.to_uppercase();
+        let new_name_upper = new_name.to_uppercase();
+        let new_module_path = PathBuf::from(format!("clippy_lints/src/{}.rs", new_name));
+        let replacements;
+
+        let (replacements, mut renamed_file) = if lint.module == lint.name
+            // Only rename the module if the file doesn't already exist.
+            && let Ok(file) = fs::OpenOptions::new().create_new(true).write(true).open(&new_module_path)
+        {
+            lint.module = new_name.into();
+            replacements = [(&*old_name_upper, &*new_name_upper), (old_name, new_name)];
+            (replacements.as_slice(), Some(file))
+        } else {
+            replacements = [(&*old_name_upper, &*new_name_upper), ("", "")];
+            (&replacements[0..1], None)
+        };
+        lint.name = new_name.into();
 
-        for (_, file) in clippy_lints_src_files() {
+        for (rel_path, file) in clippy_lints_src_files() {
             let path = file.path();
             let contents =
                 fs::read_to_string(path).unwrap_or_else(|e| panic!("Cannot read from `{}`: {}", path.display(), e));
-            if contents.contains(&old_name) {
-                fs::write(path, contents.replace(&old_name, &new_name).as_bytes())
+            let new_contents = replace_ident_like(&contents, replacements);
+
+            if let Some(ref mut file) = renamed_file
+                // Make sure the file is directly in the src directory and not a sub-directory.
+                && rel_path.parent().map_or(false, |p| p.as_os_str().is_empty())
+                && rel_path.file_stem() == Some(OsStr::new(old_name))
+            {
+                let contents = new_contents.as_ref().unwrap_or(&contents);
+                file.write_all(contents.as_bytes())
+                    .unwrap_or_else(|e| panic!("Cannot write to `{}`: {}", new_module_path.display(), e));
+                renamed_file = None;
+                fs::remove_file(path)
+                    .unwrap_or_else(|e| panic!("Cannot delete file `{}`: {}", path.display(), e));
+            } else if let Some(contents) = new_contents {
+                fs::write(path, contents.as_bytes())
                     .unwrap_or_else(|e| panic!("Cannot write to `{}`: {}", path.display(), e));
             }
         }
@@ -252,6 +291,41 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
         .unwrap_or_else(|e| panic!("error writing file `clippy_lints/src/renamed_lints.rs`: {}", e));
 }
 
+/// Replace substrings if they aren't bordered by identifier characters. Returns `None` if there
+/// were no replacements.
+fn replace_ident_like(contents: &str, replacements: &[(&str, &str)]) -> Option<String> {
+    fn is_ident_char(c: u8) -> bool {
+        matches!(c, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_')
+    }
+
+    let searcher = AhoCorasickBuilder::new()
+        .dfa(true)
+        .match_kind(aho_corasick::MatchKind::LeftmostLongest)
+        .build_with_size::<u16, _, _>(replacements.iter().map(|&(x, _)| x.as_bytes()))
+        .unwrap();
+
+    let mut result = String::with_capacity(contents.len() + 1024);
+    let mut pos = 0;
+    let mut edited = false;
+    for m in searcher.find_iter(contents) {
+        let (old, new) = replacements[m.pattern()];
+        result.push_str(&contents[pos..m.start()]);
+        result.push_str(
+            if !is_ident_char(contents.as_bytes().get(m.start().wrapping_sub(1)).copied().unwrap_or(0))
+                && !is_ident_char(contents.as_bytes().get(m.end()).copied().unwrap_or(0))
+            {
+                edited = true;
+                new
+            } else {
+                old
+            },
+        );
+        pos = m.end();
+    }
+    result.push_str(&contents[pos..]);
+    edited.then(|| result)
+}
+
 fn round_to_fifty(count: usize) -> usize {
     count / 50 * 50
 }
@@ -442,7 +516,12 @@ fn gen_renamed_lints_test(lints: &[RenamedLint]) -> String {
 }
 
 fn gen_renamed_lints_list(lints: &[RenamedLint]) -> String {
-    let mut res = String::from("pub static RENAMED_LINTS: &[(&str, &str)] = &[\n");
+    const HEADER: &str = "\
+        // This file is managed by `cargo dev rename_lint`. Prefer using that when possible.\n\n\
+        #[rustfmt::skip]\n\
+        pub static RENAMED_LINTS: &[(&str, &str)] = &[\n";
+
+    let mut res = String::from(HEADER);
     for lint in lints {
         writeln!(res, "    (\"{}\", \"{}\"),", lint.old_name, lint.new_name).unwrap();
     }

clippy_lints/src/renamed_lints.rs

@@ -1,3 +1,6 @@
+// This file is managed by `cargo dev rename_lint`. Prefer using that when possible.
+
+#[rustfmt::skip]
 pub static RENAMED_LINTS: &[(&str, &str)] = &[
     ("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions"),
     ("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions"),