From 560559bafe1d61bca3d815ce80c53ae2d48e1829 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 11 Feb 2020 11:07:38 +0100 Subject: [PATCH 1/6] Make Lint::by_lint_group take impl Iterator as argument --- clippy_dev/src/lib.rs | 9 +++------ clippy_dev/src/main.rs | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 3acecf967910..67a9abd29d67 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -63,11 +63,8 @@ impl Lint { /// Returns the lints in a `HashMap`, grouped by the different lint groups #[must_use] - pub fn by_lint_group(lints: &[Self]) -> HashMap> { - lints - .iter() - .map(|lint| (lint.group.to_string(), lint.clone())) - .into_group_map() + pub fn by_lint_group(lints: impl Iterator) -> HashMap> { + lints.map(|lint| (lint.group.to_string(), lint)).into_group_map() } #[must_use] @@ -449,7 +446,7 @@ fn test_by_lint_group() { "group2".to_string(), vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")], ); - assert_eq!(expected, Lint::by_lint_group(&lints)); + assert_eq!(expected, Lint::by_lint_group(lints.into_iter())); } #[test] diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 154876817429..2808aafc6905 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -136,7 +136,7 @@ fn print_lints() { let lint_list = gather_all(); let usable_lints: Vec = Lint::usable_lints(lint_list).collect(); let lint_count = usable_lints.len(); - let grouped_by_lint_group = Lint::by_lint_group(&usable_lints); + let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter()); for (lint_group, mut lints) in grouped_by_lint_group { if lint_group == "Deprecated" { @@ -267,7 +267,7 @@ fn update_lints(update_mode: UpdateMode) { .changed; // Generate the list of lints for all other lint groups - for (lint_group, lints) in Lint::by_lint_group(&usable_lints) { + for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter()) { file_change |= replace_region_in_file( Path::new("clippy_lints/src/lib.rs"), &format!("store.register_group\\(true, \"clippy::{}\"", lint_group), From 3da2c9183a19c6f11e8bd61f16d03b6830eb3eec Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 11 Feb 2020 11:10:54 +0100 Subject: [PATCH 2/6] Save Lint::module as full path of module --- clippy_dev/src/lib.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 67a9abd29d67..042ce9bc009b 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -170,29 +170,34 @@ pub fn gather_all() -> impl Iterator { fn gather_from_file(dir_entry: &walkdir::DirEntry) -> impl Iterator { let content = fs::read_to_string(dir_entry.path()).unwrap(); - let mut filename = dir_entry.path().file_stem().unwrap().to_str().unwrap(); + let path = dir_entry.path(); + let filename = path.file_stem().unwrap(); + let path_buf = path.with_file_name(filename); + let mut rel_path = path_buf + .strip_prefix(clippy_project_root().join("clippy_lints/src")) + .expect("only files in `clippy_lints/src` should be looked at"); // If the lints are stored in mod.rs, we get the module name from // the containing directory: if filename == "mod" { - filename = dir_entry - .path() - .parent() - .unwrap() - .file_stem() - .unwrap() - .to_str() - .unwrap() + rel_path = rel_path.parent().unwrap(); } - parse_contents(&content, filename) + + let module = rel_path + .components() + .map(|c| c.as_os_str().to_str().unwrap()) + .collect::>() + .join("::"); + + parse_contents(&content, &module) } -fn parse_contents(content: &str, filename: &str) -> impl Iterator { +fn parse_contents(content: &str, module: &str) -> impl Iterator { let lints = DEC_CLIPPY_LINT_RE .captures_iter(content) - .map(|m| Lint::new(&m["name"], &m["cat"], &m["desc"], None, filename)); + .map(|m| Lint::new(&m["name"], &m["cat"], &m["desc"], None, module)); let deprecated = DEC_DEPRECATED_LINT_RE .captures_iter(content) - .map(|m| Lint::new(&m["name"], "Deprecated", &m["desc"], Some(&m["desc"]), filename)); + .map(|m| Lint::new(&m["name"], "Deprecated", &m["desc"], Some(&m["desc"]), module)); // Removing the `.collect::>().into_iter()` causes some lifetime issues due to the map lints.chain(deprecated).collect::>().into_iter() } From 4a9bfe41841ad7299174832cbad450625ef5fc58 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 11 Feb 2020 11:11:43 +0100 Subject: [PATCH 3/6] Let update_lints also generate the internal lints --- clippy_dev/src/lib.rs | 7 ++++++- clippy_dev/src/main.rs | 4 +++- clippy_lints/src/lib.rs | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 042ce9bc009b..8e6479b64c4c 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -61,6 +61,11 @@ impl Lint { lints.filter(|l| l.deprecation.is_none() && !l.is_internal()) } + /// Returns all internal lints (not `internal_warn` lints) + pub fn internal_lints(lints: impl Iterator) -> impl Iterator { + lints.filter(|l| l.group == "internal") + } + /// Returns the lints in a `HashMap`, grouped by the different lint groups #[must_use] pub fn by_lint_group(lints: impl Iterator) -> HashMap> { @@ -79,7 +84,7 @@ pub fn gen_lint_group_list(lints: Vec) -> Vec { lints .into_iter() .filter_map(|l| { - if l.is_internal() || l.deprecation.is_some() { + if l.deprecation.is_some() { None } else { Some(format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase())) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 2808aafc6905..c2c9112e3b86 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -164,6 +164,8 @@ fn print_lints() { fn update_lints(update_mode: UpdateMode) { let lint_list: Vec = gather_all().collect(); + let internal_lints = Lint::internal_lints(lint_list.clone().into_iter()); + let usable_lints: Vec = Lint::usable_lints(lint_list.clone().into_iter()).collect(); let lint_count = usable_lints.len(); @@ -267,7 +269,7 @@ fn update_lints(update_mode: UpdateMode) { .changed; // Generate the list of lints for all other lint groups - for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter()) { + for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter().chain(internal_lints)) { file_change |= replace_region_in_file( Path::new("clippy_lints/src/lib.rs"), &format!("store.register_group\\(true, \"clippy::{}\"", lint_group), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 02cf2d06f04d..a641ff777871 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1105,10 +1105,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![ LintId::of(&utils::internal_lints::CLIPPY_LINTS_INTERNAL), LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS), + LintId::of(&utils::internal_lints::DEFAULT_LINT), LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA), LintId::of(&utils::internal_lints::PRODUCE_ICE), - LintId::of(&utils::internal_lints::DEFAULT_LINT), ]); store.register_group(true, "clippy::all", Some("clippy"), vec![ From 2635a602bc62928fc44b3c7d7c439eb4c788f552 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 11 Feb 2020 11:29:03 +0100 Subject: [PATCH 4/6] Update some documentation --- doc/adding_lints.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/adding_lints.md b/doc/adding_lints.md index cc4b5f7d1623..7e1263ebf22f 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -191,15 +191,15 @@ declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]); impl EarlyLintPass for FooFunctions {} ``` -Don't worry about the `name` method here. As long as it includes the name of the -lint pass it should be fine. - -The new lint automation runs `update_lints`, which automates some things, but it -doesn't automate everything. We will have to register our lint pass manually in -the `register_plugins` function in `clippy_lints/src/lib.rs`: +Normally after declaring the lint, we have to run `cargo dev update_lints`, +which updates some files, so Clippy knows about the new lint. Since we used +`cargo dev new_lint ...` to generate the lint declaration, this was done +automatically. While `update_lints` automates most of the things, it doesn't +automate everything. We will have to register our lint pass manually in the +`register_plugins` function in `clippy_lints/src/lib.rs`: ```rust -reg.register_early_lint_pass(box foo_functions::FooFunctions); +store.register_early_pass(box foo_functions::FooFunctions); ``` This should fix the `unknown clippy lint: clippy::foo_functions` error that we From 07026983f5e46ac5c5caf2dc86014a83297a0c4f Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 12 Feb 2020 09:12:19 +0100 Subject: [PATCH 5/6] Rename lint_count -> usable_lint_count --- clippy_dev/src/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index c2c9112e3b86..ec0c33109ba9 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -135,7 +135,7 @@ fn main() { fn print_lints() { let lint_list = gather_all(); let usable_lints: Vec = Lint::usable_lints(lint_list).collect(); - let lint_count = usable_lints.len(); + let usable_lint_count = usable_lints.len(); let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter()); for (lint_group, mut lints) in grouped_by_lint_group { @@ -157,7 +157,7 @@ fn print_lints() { } } - println!("there are {} lints", lint_count); + println!("there are {} lints", usable_lint_count); } #[allow(clippy::too_many_lines)] @@ -167,7 +167,7 @@ fn update_lints(update_mode: UpdateMode) { let internal_lints = Lint::internal_lints(lint_list.clone().into_iter()); let usable_lints: Vec = Lint::usable_lints(lint_list.clone().into_iter()).collect(); - let lint_count = usable_lints.len(); + let usable_lint_count = usable_lints.len(); let mut sorted_usable_lints = usable_lints.clone(); sorted_usable_lints.sort_by_key(|lint| lint.name.clone()); @@ -200,7 +200,7 @@ fn update_lints(update_mode: UpdateMode) { || { vec![format!( "[There are {} lints included in this crate!]({})", - lint_count, DOCS_LINK + usable_lint_count, DOCS_LINK )] }, ) From 50a2f971fcfb1156dd6bad37f88ba7e651390714 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 14 Feb 2020 14:42:24 +0100 Subject: [PATCH 6/6] Adapt gen_lint_group_list test to also generate internal lints --- clippy_dev/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 8e6479b64c4c..6fe7bb155ac5 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -529,10 +529,11 @@ fn test_gen_lint_group_list() { Lint::new("abc", "group1", "abc", None, "module_name"), Lint::new("should_assert_eq", "group1", "abc", None, "module_name"), Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"), - Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"), + Lint::new("internal", "internal_style", "abc", None, "module_name"), ]; let expected = vec![ " LintId::of(&module_name::ABC),".to_string(), + " LintId::of(&module_name::INTERNAL),".to_string(), " LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(), ]; assert_eq!(expected, gen_lint_group_list(lints));