From c7aa411fabb88f0340257d2f31de2226f9433475 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 5 Oct 2024 02:23:59 +0200 Subject: [PATCH] add a book page for lighter, more granular lint groups --- Cargo.toml | 1 + book/src/SUMMARY.md | 1 + book/src/granular_lint_groups.md | 219 +++++++++++++++++++++++++++++++ tests/lint-groups.rs | 132 +++++++++++++++++++ 4 files changed, 353 insertions(+) create mode 100644 book/src/granular_lint_groups.md create mode 100644 tests/lint-groups.rs diff --git a/Cargo.toml b/Cargo.toml index ddc27179ef2f..f5bae35a5b22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ serde_json = "1.0.122" toml = "0.7.3" walkdir = "2.3" filetime = "0.2.9" +indoc = "1.0" itertools = "0.12" # UI test dependencies diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index be13fcbe260f..040eb7db02ed 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -7,6 +7,7 @@ - [Configuration](configuration.md) - [Lint Configuration](lint_configuration.md) - [Clippy's Lints](lints.md) +- [More granular lint groups](granular_lint_groups.md) - [Continuous Integration](continuous_integration/README.md) - [GitHub Actions](continuous_integration/github_actions.md) - [GitLab CI](continuous_integration/gitlab.md) diff --git a/book/src/granular_lint_groups.md b/book/src/granular_lint_groups.md new file mode 100644 index 000000000000..cd645c22b7e2 --- /dev/null +++ b/book/src/granular_lint_groups.md @@ -0,0 +1,219 @@ +# More granular lint groups + +Clippy groups its lints into [9 primary categories](lints.md), +two of which are allow-by-default (pedantic and restriction). + +One downside with having such few but broad categories for allow-by-default lints +is that it significantly decreases discoverability, as `restriction` often acts as a blanket category +for any lint that most users likely would not want enforced on their codebase. + +This page should help with that, by defining more granular, unofficial lint groups. +For example, some people might not be interested in all the style-related `pedantic` lints, +but *are* interested in the `perf`-related ones, so it can be worth going through some of these. + + + +## Perf-pedantic + +These are `pedantic` lints that look for code patterns that could be expressed in a more efficient way. +These would be candidates for the `perf` category, however suggestions made by them can also sometimes hurt readability +and obfuscate the meaning, so occasional `#[allow]`s are expected to be used. + + +Lints: `assigning_clones`, `inefficient_to_string`, `naive_bytecount`, `needless_bitwise_bool`, `trivially_copy_pass_by_ref`, `unnecessary_join`, `unnecessary_box_returns` + +
+#![warn] attribute + +``` +#![warn( + clippy::assigning_clones, + clippy::inefficient_to_string, + clippy::naive_bytecount, + clippy::needless_bitwise_bool, + clippy::trivially_copy_pass_by_ref, + clippy::unnecessary_join, + clippy::unnecessary_box_returns +)] +``` +
+ +
+Lint table + +``` +[lints.clippy] +assigning_clones = "warn" +inefficient_to_string = "warn" +naive_bytecount = "warn" +needless_bitwise_bool = "warn" +trivially_copy_pass_by_ref = "warn" +unnecessary_join = "warn" +unnecessary_box_returns = "warn" +``` +
+ + + +## Perf-restriction + +These are `restriction` lints that can improve the performance of code, but are very specific +and sometimes *significantly* hurt readability with very little gain in the usual case. +These should ideally only be applied to specific functions or modules that were profiled +and where it is very clear that any performance gain matters. + +As always (but especially here), you should double-check that applying these actually helps +and that any performance wins are worth the introduced complexity. + + +Lints: `format_push_string`, `missing_asserts_for_indexing` + +
+#![warn] attribute + +``` +#![warn( + clippy::format_push_string, + clippy::missing_asserts_for_indexing +)] +``` +
+ +
+Lint table + +``` +[lints.clippy] +format_push_string = "warn" +missing_asserts_for_indexing = "warn" +``` +
+ + +## Perf-nursery + +These are `nursery` lints that either were previously in the `perf` category or are intended to be in `perf` +but have too many false positives. +Some of them may also be simply wrong in certain situations and end up slower, +so you should make sure to read the description to learn about possible edge cases. + + +Lints: `redundant_clone`, `iter_with_drain`, `mutex_integer`, `or_fun_call`, `significant_drop_tightening`, `trivial_regex`, `needless_collect` + +
+#![warn] attribute + +``` +#![warn( + clippy::redundant_clone, + clippy::iter_with_drain, + clippy::mutex_integer, + clippy::or_fun_call, + clippy::significant_drop_tightening, + clippy::trivial_regex, + clippy::needless_collect +)] +``` +
+ +
+Lint table + +``` +[lints.clippy] +redundant_clone = "warn" +iter_with_drain = "warn" +mutex_integer = "warn" +or_fun_call = "warn" +significant_drop_tightening = "warn" +trivial_regex = "warn" +needless_collect = "warn" +``` +
+ + +## Panicking + +These are `restriction` lints that look for patterns that can introduce panics. + +Usually panics are not something that one should want to avoid and most of the time panicking is perfectly valid +(hence why these lints are allow-by-default), +but users may want to forbid any use of panicky functions altogether in specific contexts. + +One use case could be to annotate `GlobalAlloc` impls in which unwinding is Undefined Behavior. + + +Lints: `arithmetic_side_effects`, `expect_used`, `unwrap_used`, `panic`, `unreachable`, `todo`, `unimplemented`, `string_slice`, `indexing_slicing` + +
+#![warn] attribute + +``` +#![warn( + clippy::arithmetic_side_effects, + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::unreachable, + clippy::todo, + clippy::unimplemented, + clippy::string_slice, + clippy::indexing_slicing +)] +``` +
+ +
+Lint table + +``` +[lints.clippy] +arithmetic_side_effects = "warn" +expect_used = "warn" +unwrap_used = "warn" +panic = "warn" +unreachable = "warn" +todo = "warn" +unimplemented = "warn" +string_slice = "warn" +indexing_slicing = "warn" +``` +
+ + +## Debugging + +These are lints that can be useful to disable in CI, as they might indicate that code needs more work +or has remaining debugging artifacts. + + +Lints: `dbg_macro`, `todo`, `unimplemented` + +
+#![warn] attribute + +``` +#![warn( + clippy::dbg_macro, + clippy::todo, + clippy::unimplemented +)] +``` +
+ +
+Lint table + +``` +[lints.clippy] +dbg_macro = "warn" +todo = "warn" +unimplemented = "warn" +``` +
+ diff --git a/tests/lint-groups.rs b/tests/lint-groups.rs new file mode 100644 index 000000000000..69c9cb6605fc --- /dev/null +++ b/tests/lint-groups.rs @@ -0,0 +1,132 @@ +//! This test checks and updates `book/src/granular_lint_groups.md` +//! +//! Specifically the parts in between lint-group-start and lint-group-end are not meant to be edited +//! by hand and are instead generated based on the const `GROUPS` slice. +#![feature(rustc_private)] + +use std::collections::HashSet; +use std::{env, fs}; + +use clippy_lints::LintInfo; +use clippy_lints::declared_lints::LINTS; +use indoc::formatdoc; +use itertools::Itertools; +use regex::{Captures, Regex}; + +const GROUPS: &[(&str, &[&str])] = &[ + ("perf-pedantic", &[ + "assigning_clones", + "inefficient_to_string", + "naive_bytecount", + "needless_bitwise_bool", + "trivially_copy_pass_by_ref", + "unnecessary_join", + "unnecessary_box_returns", + ]), + ("perf-restriction", &[ + "format_push_string", + "missing_asserts_for_indexing", + ]), + ("perf-nursery", &[ + "redundant_clone", + "iter_with_drain", + "mutex_integer", + "or_fun_call", + "significant_drop_tightening", + "trivial_regex", + "needless_collect", + ]), + ("panicking", &[ + "arithmetic_side_effects", + "expect_used", + "unwrap_used", + "panic", + "unreachable", + "todo", + "unimplemented", + "string_slice", + "indexing_slicing", + ]), + ("debugging", &["dbg_macro", "todo", "unimplemented"]), +]; + +#[test] +fn check_lint_groups() { + let file = fs::read_to_string("book/src/granular_lint_groups.md").expect("failed to read granular_lint_groups.md"); + let all_lint_names: HashSet<_> = LINTS + .iter() + .map(|LintInfo { lint, .. }| lint.name.strip_prefix("clippy::").unwrap().to_ascii_lowercase()) + .collect(); + + let regex = Regex::new( + "(?s)\ + (?
)\ + (?.*?)\ + (?