Fix lint register code format

Also change the generation functions to return `String` instead of
`Vec<String>`. This makes sense now as the updates aren't line oriented
anymore.

Author: Michael Wright

clippy_dev/src/lib.rs

@@ -5,7 +5,7 @@
 
 use itertools::Itertools;
 use regex::Regex;
-use std::collections::HashMap;
+use std::collections::{BTreeSet, HashMap};
 use std::ffi::OsStr;
 use std::fs;
 use std::lazy::SyncLazy;
@@ -19,6 +19,10 @@ pub mod serve;
 pub mod setup;
 pub mod update_lints;
 
+const GENERATED_FILE_COMMENT: &str = "// This file was generated by `cargo dev update_lints`.\n\
+     // Use that command to update this file and do not edit by hand.\n\
+     // Manual edits will be overwritten.\n\n";
+
 static DEC_CLIPPY_LINT_RE: SyncLazy<Regex> = SyncLazy::new(|| {
     Regex::new(
         r#"(?x)
@@ -98,37 +102,35 @@ impl Lint {
     }
 }
 
-/// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`.
-#[must_use]
-pub fn gen_lint_group_list<'a>(group_name: &str, lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
-    let header = format!(
-        r#"store.register_group(true, "clippy::{0}", Some("clippy_{0}"), vec!["#,
-        group_name
-    );
-    let footer = "])".to_string();
-
-    let mut result = vec![header];
+/// Generates the code for registering a group
+pub fn gen_lint_group_list<'a>(group_name: &str, lints: impl Iterator<Item = &'a Lint>) -> String {
+    let mut details: Vec<_> = lints.map(|l| (&l.module, l.name.to_uppercase())).collect();
+    details.sort_unstable();
 
-    result.extend(
-        lints
-            .map(|l| format!("LintId::of({}::{}),", l.module, l.name.to_uppercase()))
-            .sorted(),
-    );
+    let mut output = GENERATED_FILE_COMMENT.to_string();
 
-    result.push(footer);
+    output.push_str(&format!(
+        "store.register_group(true, \"clippy::{0}\", Some(\"clippy_{0}\"), vec![\n",
+        group_name
+    ));
+    for (module, name) in details {
+        output.push_str(&format!("    LintId::of({}::{}),\n", module, name));
+    }
+    output.push_str("])\n");
 
-    result
+    output
 }
 
-/// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`.
+/// Generates the module declarations for `lints`
 #[must_use]
-pub fn gen_modules_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
-    lints
-        .map(|l| &l.module)
-        .unique()
-        .map(|module| format!("mod {};", module))
-        .sorted()
-        .collect::<Vec<String>>()
+pub fn gen_modules_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> String {
+    let module_names: BTreeSet<_> = lints.map(|l| &l.module).collect();
+
+    let mut output = GENERATED_FILE_COMMENT.to_string();
+    for name in module_names {
+        output.push_str(&format!("mod {};\n", name));
+    }
+    output
 }
 
 /// Generates the list of lint links at the bottom of the README
@@ -140,52 +142,52 @@ pub fn gen_changelog_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec
         .collect()
 }
 
-/// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`.
+/// Generates the `register_removed` code
 #[must_use]
-pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
-    let mut result = vec!["{".to_string()];
-    result.extend(lints.flat_map(|l| {
-        l.deprecation
-            .clone()
-            .map(|depr_text| {
-                vec![
-                    "    store.register_removed(".to_string(),
-                    format!("        \"clippy::{}\",", l.name),
-                    format!("        \"{}\",", depr_text),
-                    "    );".to_string(),
-                ]
-            })
-            .expect("only deprecated lints should be passed")
-    }));
-    result.push("}".to_string());
-    result
+pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> String {
+    let mut output = GENERATED_FILE_COMMENT.to_string();
+    output.push_str("{\n");
+    for Lint { name, deprecation, .. } in lints {
+        output.push_str(&format!(
+            concat!(
+                "    store.register_removed(\n",
+                "        \"clippy::{}\",\n",
+                "        \"{}\",\n",
+                "    );\n"
+            ),
+            name,
+            deprecation.as_ref().expect("`lints` are deprecated")
+        ));
+    }
+    output.push_str("}\n");
+
+    output
 }
 
+/// Generates the code for registering lints
 #[must_use]
 pub fn gen_register_lint_list<'a>(
     internal_lints: impl Iterator<Item = &'a Lint>,
     usable_lints: impl Iterator<Item = &'a Lint>,
-) -> Vec<String> {
-    let header = "    store.register_lints(&[".to_string();
-    let footer = "    ])".to_string();
-    let internal_lints = internal_lints
-        .sorted_by_key(|l| format!("        {}::{},", l.module, l.name.to_uppercase()))
-        .map(|l| {
-            format!(
-                "        #[cfg(feature = \"internal-lints\")]\n        {}::{},",
-                l.module,
-                l.name.to_uppercase()
-            )
-        });
-    let other_lints = usable_lints
-        .sorted_by_key(|l| format!("        {}::{},", l.module, l.name.to_uppercase()))
-        .map(|l| format!("        {}::{},", l.module, l.name.to_uppercase()))
-        .sorted();
-    let mut lint_list = vec![header];
-    lint_list.extend(internal_lints);
-    lint_list.extend(other_lints);
-    lint_list.push(footer);
-    lint_list
+) -> String {
+    let mut details: Vec<_> = internal_lints
+        .map(|l| (false, &l.module, l.name.to_uppercase()))
+        .chain(usable_lints.map(|l| (true, &l.module, l.name.to_uppercase())))
+        .collect();
+    details.sort_unstable();
+
+    let mut output = GENERATED_FILE_COMMENT.to_string();
+    output.push_str("store.register_lints(&[\n");
+
+    for (is_public, module_name, lint_name) in details {
+        if !is_public {
+            output.push_str("    #[cfg(feature = \"internal-lints\")]\n");
+        }
+        output.push_str(&format!("    {}::{},\n", module_name, lint_name));
+    }
+    output.push_str("])\n");
+
+    output
 }
 
 /// Gathers all files in `src/clippy_lints` and gathers all lints inside
@@ -524,21 +526,23 @@ fn test_gen_deprecated() {
             "module_name",
         ),
     ];
-    let expected: Vec<String> = vec![
-        "{",
-        "    store.register_removed(",
-        "        \"clippy::should_assert_eq\",",
-        "        \"has been superseded by should_assert_eq2\",",
-        "    );",
-        "    store.register_removed(",
-        "        \"clippy::another_deprecated\",",
-        "        \"will be removed\",",
-        "    );",
-        "}",
-    ]
-    .into_iter()
-    .map(String::from)
-    .collect();
+
+    let expected = GENERATED_FILE_COMMENT.to_string()
+        + &[
+            "{",
+            "    store.register_removed(",
+            "        \"clippy::should_assert_eq\",",
+            "        \"has been superseded by should_assert_eq2\",",
+            "    );",
+            "    store.register_removed(",
+            "        \"clippy::another_deprecated\",",
+            "        \"will be removed\",",
+            "    );",
+            "}",
+        ]
+        .join("\n")
+        + "\n";
+
     assert_eq!(expected, gen_deprecated(lints.iter()));
 }
 
@@ -555,7 +559,7 @@ fn test_gen_modules_list() {
         Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
         Lint::new("incorrect_stuff", "group3", "abc", None, "another_module"),
     ];
-    let expected = vec!["mod another_module;".to_string(), "mod module_name;".to_string()];
+    let expected = GENERATED_FILE_COMMENT.to_string() + &["mod another_module;", "mod module_name;"].join("\n") + "\n";
     assert_eq!(expected, gen_modules_list(lints.iter()));
 }
 
@@ -566,12 +570,18 @@ fn test_gen_lint_group_list() {
         Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
         Lint::new("internal", "internal_style", "abc", None, "module_name"),
     ];
-    let expected = vec![
-        "store.register_group(true, \"clippy::group1\", Some(\"clippy_group1\"), vec![",
-        "LintId::of(module_name::ABC),",
-        "LintId::of(module_name::INTERNAL),",
-        "LintId::of(module_name::SHOULD_ASSERT_EQ),",
-        "])",
-    ];
-    assert_eq!(expected, gen_lint_group_list("group1", lints.iter()));
+    let expected = GENERATED_FILE_COMMENT.to_string()
+        + &[
+            "store.register_group(true, \"clippy::group1\", Some(\"clippy_group1\"), vec![",
+            "    LintId::of(module_name::ABC),",
+            "    LintId::of(module_name::INTERNAL),",
+            "    LintId::of(module_name::SHOULD_ASSERT_EQ),",
+            "])",
+        ]
+        .join("\n")
+        + "\n";
+
+    let result = gen_lint_group_list("group1", lints.iter());
+
+    assert_eq!(expected, result);
 }

clippy_dev/src/update_lints.rs

@@ -66,32 +66,37 @@ pub fn run(update_mode: UpdateMode) {
         exit_with_failure();
     }
 
-    for (name, lines) in [
-        ("mods", gen_modules_list(usable_lints.iter())),
-        ("deprecated", gen_deprecated(deprecated_lints.iter())),
-        (
-            "register_lints",
-            gen_register_lint_list(internal_lints.iter(), usable_lints.iter()),
-        ),
-        ("register_all", {
-            let all_group_lints = usable_lints.iter().filter(|l| {
-                matches!(
-                    &*l.group,
-                    "correctness" | "suspicious" | "style" | "complexity" | "perf"
-                )
-            });
-
-            gen_lint_group_list("all", all_group_lints)
-        }),
-    ] {
-        process_file(&format!("clippy_lints/src/lib.{}.rs", name), update_mode, &lines[..]);
-    }
+    process_file(
+        "clippy_lints/src/lib.register_lints.rs",
+        update_mode,
+        &gen_register_lint_list(internal_lints.iter(), usable_lints.iter()),
+    );
+    process_file(
+        "clippy_lints/src/lib.deprecated.rs",
+        update_mode,
+        &gen_deprecated(deprecated_lints.iter()),
+    );
+    process_file(
+        "clippy_lints/src/lib.mods.rs",
+        update_mode,
+        &gen_modules_list(usable_lints.iter()),
+    );
+
+    let all_group_lints = usable_lints.iter().filter(|l| {
+        matches!(
+            &*l.group,
+            "correctness" | "suspicious" | "style" | "complexity" | "perf"
+        )
+    });
+    let content = gen_lint_group_list("all", all_group_lints);
+    process_file("clippy_lints/src/lib.register_all.rs", update_mode, &content);
 
     for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter().chain(internal_lints)) {
+        let content = gen_lint_group_list(&lint_group, lints.iter());
         process_file(
             &format!("clippy_lints/src/lib.register_{}.rs", lint_group),
             update_mode,
-            &gen_lint_group_list(&lints.get(0).expect("group non-empty").group, lints.iter())[..],
+            &content,
         );
     }
 }
@@ -122,21 +127,15 @@ fn round_to_fifty(count: usize) -> usize {
     count / 50 * 50
 }
 
-fn process_file(path: impl AsRef<Path>, update_mode: UpdateMode, new_lines: &[String]) {
-    let mut new_content = "// This file was generated by `cargo dev update_lints`.\n\
-                       // Use that command to update this file and do not edit by hand.\n\
-                       // Manual edits will be overwritten.\n\n"
-        .to_string();
-    new_content.push_str(&new_lines.join("\n"));
-
+fn process_file(path: impl AsRef<Path>, update_mode: UpdateMode, content: &str) {
     if update_mode == UpdateMode::Check {
         let old_content =
             fs::read_to_string(&path).unwrap_or_else(|e| panic!("Cannot read from {}: {}", path.as_ref().display(), e));
-        if new_content != old_content {
+        if content != old_content {
             exit_with_failure();
         }
     } else {
-        fs::write(&path, new_content.as_bytes())
+        fs::write(&path, content.as_bytes())
             .unwrap_or_else(|e| panic!("Cannot write to {}: {}", path.as_ref().display(), e));
     }
 }

clippy_lints/src/lib.deprecated.rs

@@ -67,4 +67,4 @@
         "clippy::wrong_pub_self_convention",
         "set the `avoid-breaking-exported-api` config option to `false` to enable the `wrong_self_convention` lint for public items",
     );
-}
+}

clippy_lints/src/lib.mods.rs

@@ -229,4 +229,4 @@ mod wildcard_dependencies;
 mod wildcard_imports;
 mod write;
 mod zero_div_zero;
-mod zero_sized_map_values;
+mod zero_sized_map_values;