From 52cfc997afbaff3b0e98f82a06b90c8230c9225b Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Fri, 9 Jun 2023 13:33:30 -0500 Subject: [PATCH 1/8] Add lint `single_letter_idents` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 7 ++ clippy_lints/src/single_letter_idents.rs | 52 +++++++++ clippy_lints/src/utils/conf.rs | 1 + clippy_utils/src/check_proc_macro.rs | 29 +++-- .../toml_unknown_key/conf_unknown_key.stderr | 2 + tests/ui/single_letter_idents.rs | 59 +++++++++++ tests/ui/single_letter_idents.stderr | 100 ++++++++++++++++++ 9 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 clippy_lints/src/single_letter_idents.rs create mode 100644 tests/ui/single_letter_idents.rs create mode 100644 tests/ui/single_letter_idents.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a330b35d6938..5a03d768fee7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5160,6 +5160,7 @@ Released 2018-09-13 [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop +[`single_letter_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_letter_idents [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 874c173f70977..03c3e153a978e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -567,6 +567,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, + crate::single_letter_idents::SINGLE_LETTER_IDENTS_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, crate::size_of_ref::SIZE_OF_REF_INFO, crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fd10a9302c05c..9b8edc5d77e2b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -286,6 +286,7 @@ mod shadow; mod significant_drop_tightening; mod single_char_lifetime_names; mod single_component_path_imports; +mod single_letter_idents; mod size_of_in_element_count; mod size_of_ref; mod slow_vector_initialization; @@ -1033,6 +1034,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations)); store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync)); store.register_late_pass(|_| Box::new(needless_if::NeedlessIf)); + let allowed_idents = conf.allowed_idents.clone(); + store.register_early_pass(move || { + Box::new(single_letter_idents::SingleLetterIdents { + allowed_idents: allowed_idents.clone(), + }) + }); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/single_letter_idents.rs b/clippy_lints/src/single_letter_idents.rs new file mode 100644 index 0000000000000..e957b6fec6820 --- /dev/null +++ b/clippy_lints/src/single_letter_idents.rs @@ -0,0 +1,52 @@ +use clippy_utils::{diagnostics::span_lint, source::snippet}; +use itertools::Itertools; +use rustc_data_structures::fx::FxHashSet; +use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::symbol::Ident; + +declare_clippy_lint! { + /// ### What it does + /// Checks for idents which comprise of a single letter. + /// + /// Note: This lint can be very noisy when enabled; it even lints generics! it may be desirable + /// to only enable it temporarily. + /// + /// ### Why is this bad? + /// In many cases it's not, but at times it can severely hinder readability. Some codebases may + /// wish to disallow this to improve readability. + /// + /// ### Example + /// ```rust,ignore + /// for i in collection { + /// let x = i.x; + /// } + /// ``` + #[clippy::version = "1.72.0"] + pub SINGLE_LETTER_IDENTS, + restriction, + "disallows idents that can be represented as a char" +} +impl_lint_pass!(SingleLetterIdents => [SINGLE_LETTER_IDENTS]); + +#[derive(Clone)] +pub struct SingleLetterIdents { + pub allowed_idents: FxHashSet, +} + +impl EarlyLintPass for SingleLetterIdents { + fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { + let str = ident.name.as_str(); + let chars = str.chars(); + if let [char, rest @ ..] = chars.collect_vec().as_slice() + && rest.is_empty() + && self.allowed_idents.get(char).is_none() + && !in_external_macro(cx.sess(), ident.span) + // Ignore proc macros. Let's implement `WithSearchPat` for early lints someday :) + && snippet(cx, ident.span, str) == str + { + span_lint(cx, SINGLE_LETTER_IDENTS, ident.span, "this ident comprises of a single letter"); + } + } +} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 6962299e4681e..7b5413647c7e1 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -34,6 +34,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ "CamelCase", ]; const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"]; +const DEFAULT_ALLOWED_IDENTS: &[char] = &['i', 'j', 'x', 'y', 'z', 'n']; /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. #[derive(Clone, Debug, Deserialize)] diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index bcfac527c8a4e..c6d0b654f57b6 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -25,7 +25,7 @@ use rustc_hir::{ use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::TyCtxt; use rustc_session::Session; -use rustc_span::{Span, Symbol}; +use rustc_span::{symbol::Ident, Span, Symbol}; use rustc_target::spec::abi::Abi; /// The search pattern to look for. Used by `span_matches_pat` @@ -344,14 +344,18 @@ fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) { } } -pub trait WithSearchPat { +fn ident_search_pat(ident: Ident) -> (Pat, Pat) { + (Pat::OwnedStr(ident.name.as_str().to_owned()), Pat::Str("")) +} + +pub trait WithSearchPat<'cx> { type Context: LintContext; fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat); fn span(&self) -> Span; } macro_rules! impl_with_search_pat { ($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => { - impl<'cx> WithSearchPat for $ty<'cx> { + impl<'cx> WithSearchPat<'cx> for $ty<'cx> { type Context = $cx<'cx>; #[allow(unused_variables)] fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) { @@ -372,7 +376,7 @@ impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat); impl_with_search_pat!(LateContext: Variant with variant_search_pat); impl_with_search_pat!(LateContext: Ty with ty_search_pat); -impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { +impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { type Context = LateContext<'cx>; fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) { @@ -385,7 +389,7 @@ impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { } // `Attribute` does not have the `hir` associated lifetime, so we cannot use the macro -impl<'cx> WithSearchPat for &'cx Attribute { +impl<'cx> WithSearchPat<'cx> for &'cx Attribute { type Context = LateContext<'cx>; fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) { @@ -397,11 +401,24 @@ impl<'cx> WithSearchPat for &'cx Attribute { } } +// `Ident` does not have the `hir` associated lifetime, so we cannot use the macro +impl<'cx> WithSearchPat<'cx> for Ident { + type Context = LateContext<'cx>; + + fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) { + ident_search_pat(*self) + } + + fn span(&self) -> Span { + self.span + } +} + /// Checks if the item likely came from a proc-macro. /// /// This should be called after `in_external_macro` and the initial pattern matching of the ast as /// it is significantly slower than both of those. -pub fn is_from_proc_macro(cx: &T::Context, item: &T) -> bool { +pub fn is_from_proc_macro<'cx, T: WithSearchPat<'cx>>(cx: &T::Context, item: &T) -> bool { let (start_pat, end_pat) = item.search_pat(cx); !span_matches_pat(cx.sess(), item.span(), start_pat, end_pat) } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index c11835f56961d..2bc872e07e802 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -5,6 +5,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect allow-print-in-tests allow-private-module-inception allow-unwrap-in-tests + allowed-idents allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary @@ -68,6 +69,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect allow-print-in-tests allow-private-module-inception allow-unwrap-in-tests + allowed-idents allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary diff --git a/tests/ui/single_letter_idents.rs b/tests/ui/single_letter_idents.rs new file mode 100644 index 0000000000000..b33c569d31811 --- /dev/null +++ b/tests/ui/single_letter_idents.rs @@ -0,0 +1,59 @@ +//@aux-build:proc_macros.rs +#![allow(nonstandard_style, unused)] +#![warn(clippy::single_letter_idents)] + +extern crate proc_macros; +use proc_macros::external; +use proc_macros::with_span; + +struct A { + a: u32, + i: u32, + A: u32, + I: u32, +} + +struct B(u32); + +struct i; + +enum C { + D, + E, + F, + j, +} + +struct Vec4 { + x: u32, + y: u32, + z: u32, + w: u32, +} + +struct AA(T, E); + +fn main() { + // Allowed idents + let w = 1; + // Ok, not this one + // let i = 1; + let j = 1; + let n = 1; + let x = 1; + let y = 1; + let z = 1; + + for j in 0..1000 {} + + // Do not lint code from external macros + external! { for j in 0..1000 {} } + // Do not lint code from procedural macros + with_span! { + span + for j in 0..1000 {} + } +} + +fn b() {} +fn owo() {} diff --git a/tests/ui/single_letter_idents.stderr b/tests/ui/single_letter_idents.stderr new file mode 100644 index 0000000000000..712c166423503 --- /dev/null +++ b/tests/ui/single_letter_idents.stderr @@ -0,0 +1,100 @@ +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:9:8 + | +LL | struct A { + | ^ + | + = note: `-D clippy::single-letter-idents` implied by `-D warnings` + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:10:5 + | +LL | a: u32, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:12:5 + | +LL | A: u32, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:13:5 + | +LL | I: u32, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:16:8 + | +LL | struct B(u32); + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:20:6 + | +LL | enum C { + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:21:5 + | +LL | D, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:22:5 + | +LL | E, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:23:5 + | +LL | F, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:31:5 + | +LL | w: u32, + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:34:11 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:34:14 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:34:17 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:34:20 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:38:9 + | +LL | let w = 1; + | ^ + +error: this ident comprises of a single letter + --> $DIR/single_letter_idents.rs:58:4 + | +LL | fn b() {} + | ^ + +error: aborting due to 16 previous errors + From e2ecb132a5ccf238129acdbaff89bed6023a593e Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Fri, 9 Jun 2023 14:29:34 -0500 Subject: [PATCH 2/8] rename the lint --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/lib.rs | 4 ++-- ...single_letter_idents.rs => single_char_idents.rs} | 12 ++++++------ 4 files changed, 10 insertions(+), 10 deletions(-) rename clippy_lints/src/{single_letter_idents.rs => single_char_idents.rs} (81%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a03d768fee7e..27a5fc06b0702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5155,12 +5155,12 @@ Released 2018-09-13 [`significant_drop_tightening`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening [`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names [`single_char_add_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_add_str +[`single_char_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_idents [`single_char_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_lifetime_names [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop -[`single_letter_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_letter_idents [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 03c3e153a978e..1c494468226d2 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -565,9 +565,9 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::shadow::SHADOW_SAME_INFO, crate::shadow::SHADOW_UNRELATED_INFO, crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, + crate::single_char_idents::SINGLE_CHAR_IDENTS_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, - crate::single_letter_idents::SINGLE_LETTER_IDENTS_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, crate::size_of_ref::SIZE_OF_REF_INFO, crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9b8edc5d77e2b..463e4a429d13b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -284,9 +284,9 @@ mod semicolon_if_nothing_returned; mod serde_api; mod shadow; mod significant_drop_tightening; +mod single_char_idents; mod single_char_lifetime_names; mod single_component_path_imports; -mod single_letter_idents; mod size_of_in_element_count; mod size_of_ref; mod slow_vector_initialization; @@ -1036,7 +1036,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(needless_if::NeedlessIf)); let allowed_idents = conf.allowed_idents.clone(); store.register_early_pass(move || { - Box::new(single_letter_idents::SingleLetterIdents { + Box::new(single_char_idents::SingleCharIdents { allowed_idents: allowed_idents.clone(), }) }); diff --git a/clippy_lints/src/single_letter_idents.rs b/clippy_lints/src/single_char_idents.rs similarity index 81% rename from clippy_lints/src/single_letter_idents.rs rename to clippy_lints/src/single_char_idents.rs index e957b6fec6820..0125b576f43a3 100644 --- a/clippy_lints/src/single_letter_idents.rs +++ b/clippy_lints/src/single_char_idents.rs @@ -24,29 +24,29 @@ declare_clippy_lint! { /// } /// ``` #[clippy::version = "1.72.0"] - pub SINGLE_LETTER_IDENTS, + pub SINGLE_CHAR_IDENTS, restriction, "disallows idents that can be represented as a char" } -impl_lint_pass!(SingleLetterIdents => [SINGLE_LETTER_IDENTS]); +impl_lint_pass!(SingleCharIdents => [SINGLE_CHAR_IDENTS]); #[derive(Clone)] -pub struct SingleLetterIdents { +pub struct SingleCharIdents { pub allowed_idents: FxHashSet, } -impl EarlyLintPass for SingleLetterIdents { +impl EarlyLintPass for SingleCharIdents { fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { let str = ident.name.as_str(); let chars = str.chars(); - if let [char, rest @ ..] = chars.collect_vec().as_slice() + if let [char, rest @ ..] = &*chars.collect_vec() && rest.is_empty() && self.allowed_idents.get(char).is_none() && !in_external_macro(cx.sess(), ident.span) // Ignore proc macros. Let's implement `WithSearchPat` for early lints someday :) && snippet(cx, ident.span, str) == str { - span_lint(cx, SINGLE_LETTER_IDENTS, ident.span, "this ident comprises of a single letter"); + span_lint(cx, SINGLE_CHAR_IDENTS, ident.span, "this ident comprises of a single char"); } } } From 03c8db048e97835c3431eac5da90e207b0a458f6 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Fri, 9 Jun 2023 14:45:53 -0500 Subject: [PATCH 3/8] make cargo test pass + example --- clippy_lints/src/single_char_idents.rs | 11 +- ...letter_idents.rs => single_char_idents.rs} | 2 +- tests/ui/single_char_idents.stderr | 100 ++++++++++++++++++ tests/ui/single_letter_idents.stderr | 100 ------------------ 4 files changed, 110 insertions(+), 103 deletions(-) rename tests/ui/{single_letter_idents.rs => single_char_idents.rs} (95%) create mode 100644 tests/ui/single_char_idents.stderr delete mode 100644 tests/ui/single_letter_idents.stderr diff --git a/clippy_lints/src/single_char_idents.rs b/clippy_lints/src/single_char_idents.rs index 0125b576f43a3..17a2ebe93ed85 100644 --- a/clippy_lints/src/single_char_idents.rs +++ b/clippy_lints/src/single_char_idents.rs @@ -19,10 +19,17 @@ declare_clippy_lint! { /// /// ### Example /// ```rust,ignore - /// for i in collection { - /// let x = i.x; + /// for m in movies { + /// let title = m.t; /// } /// ``` + /// Use instead: + /// ```rust,ignore + /// for movie in movies { + /// let title = movie.title; + /// } + /// ``` + /// ``` #[clippy::version = "1.72.0"] pub SINGLE_CHAR_IDENTS, restriction, diff --git a/tests/ui/single_letter_idents.rs b/tests/ui/single_char_idents.rs similarity index 95% rename from tests/ui/single_letter_idents.rs rename to tests/ui/single_char_idents.rs index b33c569d31811..386633161feb2 100644 --- a/tests/ui/single_letter_idents.rs +++ b/tests/ui/single_char_idents.rs @@ -1,6 +1,6 @@ //@aux-build:proc_macros.rs #![allow(nonstandard_style, unused)] -#![warn(clippy::single_letter_idents)] +#![warn(clippy::single_char_idents)] extern crate proc_macros; use proc_macros::external; diff --git a/tests/ui/single_char_idents.stderr b/tests/ui/single_char_idents.stderr new file mode 100644 index 0000000000000..ceedab9c0da4a --- /dev/null +++ b/tests/ui/single_char_idents.stderr @@ -0,0 +1,100 @@ +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:9:8 + | +LL | struct A { + | ^ + | + = note: `-D clippy::single-char-idents` implied by `-D warnings` + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:10:5 + | +LL | a: u32, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:12:5 + | +LL | A: u32, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:13:5 + | +LL | I: u32, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:16:8 + | +LL | struct B(u32); + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:20:6 + | +LL | enum C { + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:21:5 + | +LL | D, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:22:5 + | +LL | E, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:23:5 + | +LL | F, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:31:5 + | +LL | w: u32, + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:34:11 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:34:14 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:34:17 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:34:20 + | +LL | struct AA(T, E); + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:38:9 + | +LL | let w = 1; + | ^ + +error: this ident comprises of a single char + --> $DIR/single_char_idents.rs:58:4 + | +LL | fn b() {} + | ^ + +error: aborting due to 16 previous errors + diff --git a/tests/ui/single_letter_idents.stderr b/tests/ui/single_letter_idents.stderr deleted file mode 100644 index 712c166423503..0000000000000 --- a/tests/ui/single_letter_idents.stderr +++ /dev/null @@ -1,100 +0,0 @@ -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:9:8 - | -LL | struct A { - | ^ - | - = note: `-D clippy::single-letter-idents` implied by `-D warnings` - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:10:5 - | -LL | a: u32, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:12:5 - | -LL | A: u32, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:13:5 - | -LL | I: u32, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:16:8 - | -LL | struct B(u32); - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:20:6 - | -LL | enum C { - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:21:5 - | -LL | D, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:22:5 - | -LL | E, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:23:5 - | -LL | F, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:31:5 - | -LL | w: u32, - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:34:11 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:34:14 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:34:17 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:34:20 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:38:9 - | -LL | let w = 1; - | ^ - -error: this ident comprises of a single letter - --> $DIR/single_letter_idents.rs:58:4 - | -LL | fn b() {} - | ^ - -error: aborting due to 16 previous errors - From 7cdd87ca4afe3ab7a1a8eb1544db132678335f87 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Fri, 9 Jun 2023 17:49:35 -0500 Subject: [PATCH 4/8] ignore generics and allow arbitrary threshold --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/lib.rs | 12 +- clippy_lints/src/min_ident_chars.rs | 133 ++++++++++++++++++ clippy_lints/src/single_char_idents.rs | 59 -------- clippy_lints/src/utils/conf.rs | 11 +- .../min_ident_chars/auxiliary/extern_types.rs | 3 + tests/ui-toml/min_ident_chars/clippy.toml | 2 + .../min_ident_chars/min_ident_chars.rs | 17 +++ .../min_ident_chars/min_ident_chars.stderr | 16 +++ .../toml_unknown_key/conf_unknown_key.stderr | 6 +- ...ngle_char_idents.rs => min_ident_chars.rs} | 12 +- tests/ui/min_ident_chars.stderr | 70 +++++++++ tests/ui/single_char_idents.stderr | 100 ------------- 14 files changed, 270 insertions(+), 175 deletions(-) create mode 100644 clippy_lints/src/min_ident_chars.rs delete mode 100644 clippy_lints/src/single_char_idents.rs create mode 100644 tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs create mode 100644 tests/ui-toml/min_ident_chars/clippy.toml create mode 100644 tests/ui-toml/min_ident_chars/min_ident_chars.rs create mode 100644 tests/ui-toml/min_ident_chars/min_ident_chars.stderr rename tests/ui/{single_char_idents.rs => min_ident_chars.rs} (86%) create mode 100644 tests/ui/min_ident_chars.stderr delete mode 100644 tests/ui/single_char_idents.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 27a5fc06b0702..2d62bfd4f99b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4958,6 +4958,7 @@ Released 2018-09-13 [`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none [`mem_replace_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default [`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit +[`min_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute [`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os @@ -5155,7 +5156,6 @@ Released 2018-09-13 [`significant_drop_tightening`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening [`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names [`single_char_add_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_add_str -[`single_char_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_idents [`single_char_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_lifetime_names [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1c494468226d2..769774b27c4cd 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -416,6 +416,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::VERBOSE_FILE_READS_INFO, crate::methods::WRONG_SELF_CONVENTION_INFO, crate::methods::ZST_OFFSET_INFO, + crate::min_ident_chars::MIN_IDENT_CHARS_INFO, crate::minmax::MIN_MAX_INFO, crate::misc::SHORT_CIRCUIT_STATEMENT_INFO, crate::misc::TOPLEVEL_REF_ARG_INFO, @@ -565,7 +566,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::shadow::SHADOW_SAME_INFO, crate::shadow::SHADOW_UNRELATED_INFO, crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, - crate::single_char_idents::SINGLE_CHAR_IDENTS_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 463e4a429d13b..dcf1c6f64a4bc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -197,6 +197,7 @@ mod matches; mod mem_forget; mod mem_replace; mod methods; +mod min_ident_chars; mod minmax; mod misc; mod misc_early; @@ -284,7 +285,6 @@ mod semicolon_if_nothing_returned; mod serde_api; mod shadow; mod significant_drop_tightening; -mod single_char_idents; mod single_char_lifetime_names; mod single_component_path_imports; mod size_of_in_element_count; @@ -1034,10 +1034,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations)); store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync)); store.register_late_pass(|_| Box::new(needless_if::NeedlessIf)); - let allowed_idents = conf.allowed_idents.clone(); - store.register_early_pass(move || { - Box::new(single_char_idents::SingleCharIdents { - allowed_idents: allowed_idents.clone(), + let allowed_idents_below_min_chars = conf.allowed_idents_below_min_chars.clone(); + let min_ident_chars_threshold = conf.min_ident_chars_threshold; + store.register_late_pass(move |_| { + Box::new(min_ident_chars::MinIdentChars { + allowed_idents_below_min_chars: allowed_idents_below_min_chars.clone(), + min_ident_chars_threshold, }) }); // add lints here, do not remove this comment, it's used in `new_lint` diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs new file mode 100644 index 0000000000000..bbe7fc540e927 --- /dev/null +++ b/clippy_lints/src/min_ident_chars.rs @@ -0,0 +1,133 @@ +use clippy_utils::{diagnostics::span_lint, is_from_proc_macro}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::{ + def::{DefKind, Res}, + intravisit::{walk_item, Visitor}, + GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, +}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use std::borrow::Cow; + +declare_clippy_lint! { + /// ### What it does + /// Checks for idents which comprise of a single letter. + /// + /// Note: This lint can be very noisy when enabled; it may be desirable to only enable it + /// temporarily. + /// + /// ### Why is this bad? + /// In many cases it's not, but at times it can severely hinder readability. Some codebases may + /// wish to disallow this to improve readability. + /// + /// ### Example + /// ```rust,ignore + /// for m in movies { + /// let title = m.t; + /// } + /// ``` + /// Use instead: + /// ```rust,ignore + /// for movie in movies { + /// let title = movie.title; + /// } + /// ``` + /// ``` + #[clippy::version = "1.72.0"] + pub MIN_IDENT_CHARS, + restriction, + "disallows idents that are too short" +} +impl_lint_pass!(MinIdentChars => [MIN_IDENT_CHARS]); + +#[derive(Clone)] +pub struct MinIdentChars { + pub allowed_idents_below_min_chars: FxHashSet, + pub min_ident_chars_threshold: u64, +} + +impl LateLintPass<'_> for MinIdentChars { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if self.min_ident_chars_threshold == 0 { + return; + } + + walk_item(&mut IdentVisitor { conf: self, cx }, item); + } +} + +struct IdentVisitor<'cx, 'tcx> { + conf: &'cx MinIdentChars, + cx: &'cx LateContext<'tcx>, +} + +#[expect(clippy::cast_possible_truncation)] +impl Visitor<'_> for IdentVisitor<'_, '_> { + fn visit_id(&mut self, hir_id: HirId) { + let Self { conf, cx } = *self; + // Reimplementation of `find`, as it uses indexing, which can (and will in async functions) panic. + // This should probably be fixed on the rustc side, this is just a temporary workaround. + // FIXME: Remove me if/when this is fixed in rustc + let node = if hir_id.local_id == ItemLocalId::from_u32(0) { + // In this case, we can just use `find`, `Owner`'s `node` field is private anyway so we can't + // reimplement it even if we wanted to + cx.tcx.hir().find(hir_id) + } else { + let Some(owner) = cx.tcx.hir_owner_nodes(hir_id.owner).as_owner() else { + return; + }; + owner.nodes.get(hir_id.local_id).copied().flatten().map(|p| p.node) + }; + let Some(node) = node else { + return; + }; + let Some(ident) = node.ident() else { + return; + }; + + let str = ident.as_str(); + if !in_external_macro(cx.sess(), ident.span) + && str.len() <= conf.min_ident_chars_threshold as usize + && !str.is_empty() + && conf.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() + { + if let Node::Item(item) = node && let ItemKind::Use(..) = item.kind { + return; + } + // `struct Awa(T)` + // ^ + if let Node::PathSegment(path) = node { + if let Res::Def(def_kind, ..) = path.res && let DefKind::TyParam = def_kind { + return; + } + if matches!(path.res, Res::PrimTy(..)) || path.res.opt_def_id().is_some_and(|def_id| !def_id.is_local()) + { + return; + } + } + // `struct Awa(T)` + // ^ + if let Node::GenericParam(generic_param) = node + && let GenericParamKind::Type { .. } = generic_param.kind + { + return; + } + + if is_from_proc_macro(cx, &ident) { + return; + } + + let help = if conf.min_ident_chars_threshold == 1 { + Cow::Borrowed("this ident consists of a single char") + } else { + Cow::Owned(format!( + "this ident is too short ({} <= {}) ", + str.len(), + conf.min_ident_chars_threshold + )) + }; + span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); + } + } +} diff --git a/clippy_lints/src/single_char_idents.rs b/clippy_lints/src/single_char_idents.rs deleted file mode 100644 index 17a2ebe93ed85..0000000000000 --- a/clippy_lints/src/single_char_idents.rs +++ /dev/null @@ -1,59 +0,0 @@ -use clippy_utils::{diagnostics::span_lint, source::snippet}; -use itertools::Itertools; -use rustc_data_structures::fx::FxHashSet; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::Ident; - -declare_clippy_lint! { - /// ### What it does - /// Checks for idents which comprise of a single letter. - /// - /// Note: This lint can be very noisy when enabled; it even lints generics! it may be desirable - /// to only enable it temporarily. - /// - /// ### Why is this bad? - /// In many cases it's not, but at times it can severely hinder readability. Some codebases may - /// wish to disallow this to improve readability. - /// - /// ### Example - /// ```rust,ignore - /// for m in movies { - /// let title = m.t; - /// } - /// ``` - /// Use instead: - /// ```rust,ignore - /// for movie in movies { - /// let title = movie.title; - /// } - /// ``` - /// ``` - #[clippy::version = "1.72.0"] - pub SINGLE_CHAR_IDENTS, - restriction, - "disallows idents that can be represented as a char" -} -impl_lint_pass!(SingleCharIdents => [SINGLE_CHAR_IDENTS]); - -#[derive(Clone)] -pub struct SingleCharIdents { - pub allowed_idents: FxHashSet, -} - -impl EarlyLintPass for SingleCharIdents { - fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { - let str = ident.name.as_str(); - let chars = str.chars(); - if let [char, rest @ ..] = &*chars.collect_vec() - && rest.is_empty() - && self.allowed_idents.get(char).is_none() - && !in_external_macro(cx.sess(), ident.span) - // Ignore proc macros. Let's implement `WithSearchPat` for early lints someday :) - && snippet(cx, ident.span, str) == str - { - span_lint(cx, SINGLE_CHAR_IDENTS, ident.span, "this ident comprises of a single char"); - } - } -} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 7b5413647c7e1..c84d8d8d1f0ff 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -34,7 +34,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ "CamelCase", ]; const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"]; -const DEFAULT_ALLOWED_IDENTS: &[char] = &['i', 'j', 'x', 'y', 'z', 'n']; +const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "n"]; /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. #[derive(Clone, Debug, Deserialize)] @@ -523,6 +523,15 @@ define_Conf! { /// /// Whether to allow module inception if it's not public. (allow_private_module_inception: bool = false), + /// Lint: MIN_IDENT_CHARS. + /// + /// Allowed names below the minimum allowed characters. + (allowed_idents_below_min_chars: rustc_data_structures::fx::FxHashSet = + super::DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string).collect()), + /// Lint: MIN_IDENT_CHARS. + /// + /// Minimum chars an ident can have, anything below or equal to this will be linted. + (min_ident_chars_threshold: u64 = 1), } /// Search for the configuration file. diff --git a/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs b/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs new file mode 100644 index 0000000000000..06a144f2218ce --- /dev/null +++ b/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs @@ -0,0 +1,3 @@ +#![allow(nonstandard_style, unused)] + +pub struct Aaa; diff --git a/tests/ui-toml/min_ident_chars/clippy.toml b/tests/ui-toml/min_ident_chars/clippy.toml new file mode 100644 index 0000000000000..0114ca7501432 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/clippy.toml @@ -0,0 +1,2 @@ +allowed-idents-below-min-chars = ["Owo", "Uwu", "wha", "t_e", "lse", "_do", "_i_", "put", "her", "_e"] +min-ident-chars-threshold = 3 diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.rs b/tests/ui-toml/min_ident_chars/min_ident_chars.rs new file mode 100644 index 0000000000000..33100119ca5b0 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.rs @@ -0,0 +1,17 @@ +//@aux-build:extern_types.rs +#![allow(nonstandard_style, unused)] +#![warn(clippy::min_ident_chars)] + +extern crate extern_types; +use extern_types::Aaa; + +struct Owo { + Uwu: u128, + aaa: Aaa, +} + +fn main() { + let wha = 1; + let vvv = 1; + let uuu = 1; +} diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.stderr b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr new file mode 100644 index 0000000000000..541f5b10051a6 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr @@ -0,0 +1,16 @@ +error: this ident is too short (3 <= 3) + --> $DIR/min_ident_chars.rs:6:19 + | +LL | use extern_types::Aaa; + | ^^^ + | + = note: `-D clippy::min-ident-chars` implied by `-D warnings` + +error: this ident is too short (3 <= 3) + --> $DIR/min_ident_chars.rs:10:5 + | +LL | aaa: Aaa, + | ^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 2bc872e07e802..c546d95eb464d 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -5,7 +5,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect allow-print-in-tests allow-private-module-inception allow-unwrap-in-tests - allowed-idents + allowed-idents-below-min-chars allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary @@ -37,6 +37,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect max-struct-bools max-suggested-slice-pattern-length max-trait-bounds + min-ident-chars-threshold missing-docs-in-crate-items msrv pass-by-value-size-limit @@ -69,7 +70,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect allow-print-in-tests allow-private-module-inception allow-unwrap-in-tests - allowed-idents + allowed-idents-below-min-chars allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary @@ -101,6 +102,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect max-struct-bools max-suggested-slice-pattern-length max-trait-bounds + min-ident-chars-threshold missing-docs-in-crate-items msrv pass-by-value-size-limit diff --git a/tests/ui/single_char_idents.rs b/tests/ui/min_ident_chars.rs similarity index 86% rename from tests/ui/single_char_idents.rs rename to tests/ui/min_ident_chars.rs index 386633161feb2..134c17c553f3e 100644 --- a/tests/ui/single_char_idents.rs +++ b/tests/ui/min_ident_chars.rs @@ -1,6 +1,6 @@ //@aux-build:proc_macros.rs #![allow(nonstandard_style, unused)] -#![warn(clippy::single_char_idents)] +#![warn(clippy::min_ident_chars)] extern crate proc_macros; use proc_macros::external; @@ -38,11 +38,11 @@ fn main() { let w = 1; // Ok, not this one // let i = 1; - let j = 1; - let n = 1; - let x = 1; - let y = 1; - let z = 1; + let jz = 1; + let nz = 1; + let zx = 1; + let yz = 1; + let zz = 1; for j in 0..1000 {} diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr new file mode 100644 index 0000000000000..4b887617058bb --- /dev/null +++ b/tests/ui/min_ident_chars.stderr @@ -0,0 +1,70 @@ +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:9:8 + | +LL | struct A { + | ^ + | + = note: `-D clippy::min-ident-chars` implied by `-D warnings` + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:10:5 + | +LL | a: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:12:5 + | +LL | A: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:13:5 + | +LL | I: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:16:8 + | +LL | struct B(u32); + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:20:6 + | +LL | enum C { + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:21:5 + | +LL | D, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:22:5 + | +LL | E, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:23:5 + | +LL | F, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:31:5 + | +LL | w: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:58:4 + | +LL | fn b() {} + | ^ + +error: aborting due to 11 previous errors + diff --git a/tests/ui/single_char_idents.stderr b/tests/ui/single_char_idents.stderr deleted file mode 100644 index ceedab9c0da4a..0000000000000 --- a/tests/ui/single_char_idents.stderr +++ /dev/null @@ -1,100 +0,0 @@ -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:9:8 - | -LL | struct A { - | ^ - | - = note: `-D clippy::single-char-idents` implied by `-D warnings` - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:10:5 - | -LL | a: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:12:5 - | -LL | A: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:13:5 - | -LL | I: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:16:8 - | -LL | struct B(u32); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:20:6 - | -LL | enum C { - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:21:5 - | -LL | D, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:22:5 - | -LL | E, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:23:5 - | -LL | F, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:31:5 - | -LL | w: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:11 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:14 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:17 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:20 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:38:9 - | -LL | let w = 1; - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:58:4 - | -LL | fn b() {} - | ^ - -error: aborting due to 16 previous errors - From 243943ff56c755822968a48d5949f79c1d3a7dfa Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sat, 10 Jun 2023 04:00:59 -0500 Subject: [PATCH 5/8] make it work for locals as well oopos --- book/src/lint_configuration.md | 19 --- clippy_lints/src/min_ident_chars.rs | 31 ++++- clippy_lints/src/utils/conf.rs | 2 +- .../min_ident_chars/min_ident_chars.rs | 2 + .../min_ident_chars/min_ident_chars.stderr | 36 +++++- tests/ui/min_ident_chars.rs | 34 +++++- tests/ui/min_ident_chars.stderr | 114 ++++++++++++++++-- 7 files changed, 196 insertions(+), 42 deletions(-) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 16dc5dcef42b5..1812d50ed1bc0 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -644,22 +644,3 @@ The maximum byte size a `Future` can have, before it triggers the `clippy::large ## `unnecessary-box-size` -The byte size a `T` in `Box` can have, below which it triggers the `clippy::unnecessary_box` lint - -**Default Value:** `128` (`u64`) - ---- -**Affected lints:** -* [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns) - - -## `allow-private-module-inception` -Whether to allow module inception if it's not public. - -**Default Value:** `false` (`bool`) - ---- -**Affected lints:** -* [`module_inception`](https://rust-lang.github.io/rust-clippy/master/index.html#module_inception) - - diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs index bbe7fc540e927..cc027b6f4988d 100644 --- a/clippy_lints/src/min_ident_chars.rs +++ b/clippy_lints/src/min_ident_chars.rs @@ -3,7 +3,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir::{ def::{DefKind, Res}, intravisit::{walk_item, Visitor}, - GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, + GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; @@ -55,6 +55,29 @@ impl LateLintPass<'_> for MinIdentChars { walk_item(&mut IdentVisitor { conf: self, cx }, item); } + + // This is necessary as bindings are not visited in `visit_id`. :/ + #[expect(clippy::cast_possible_truncation)] + fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { + if let PatKind::Binding(_, _, ident, ..) = pat.kind + && let str = ident.as_str() + && !in_external_macro(cx.sess(), ident.span) + && str.len() <= self.min_ident_chars_threshold as usize + && !str.is_empty() + && self.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() + { + let help = if self.min_ident_chars_threshold == 1 { + Cow::Borrowed("this ident consists of a single char") + } else { + Cow::Owned(format!( + "this ident is too short ({} <= {})", + str.len(), + self.min_ident_chars_threshold, + )) + }; + span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); + } + } } struct IdentVisitor<'cx, 'tcx> { @@ -62,8 +85,8 @@ struct IdentVisitor<'cx, 'tcx> { cx: &'cx LateContext<'tcx>, } -#[expect(clippy::cast_possible_truncation)] impl Visitor<'_> for IdentVisitor<'_, '_> { + #[expect(clippy::cast_possible_truncation)] fn visit_id(&mut self, hir_id: HirId) { let Self { conf, cx } = *self; // Reimplementation of `find`, as it uses indexing, which can (and will in async functions) panic. @@ -122,9 +145,9 @@ impl Visitor<'_> for IdentVisitor<'_, '_> { Cow::Borrowed("this ident consists of a single char") } else { Cow::Owned(format!( - "this ident is too short ({} <= {}) ", + "this ident is too short ({} <= {})", str.len(), - conf.min_ident_chars_threshold + conf.min_ident_chars_threshold, )) }; span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index c84d8d8d1f0ff..f050782a16dd5 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -34,7 +34,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ "CamelCase", ]; const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"]; -const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "n"]; +const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"]; /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. #[derive(Clone, Debug, Deserialize)] diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.rs b/tests/ui-toml/min_ident_chars/min_ident_chars.rs index 33100119ca5b0..4326c7159c832 100644 --- a/tests/ui-toml/min_ident_chars/min_ident_chars.rs +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.rs @@ -14,4 +14,6 @@ fn main() { let wha = 1; let vvv = 1; let uuu = 1; + let (mut a, mut b) = (1, 2); + for i in 0..1000 {} } diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.stderr b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr index 541f5b10051a6..d9a27628ddb22 100644 --- a/tests/ui-toml/min_ident_chars/min_ident_chars.stderr +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr @@ -1,4 +1,4 @@ -error: this ident is too short (3 <= 3) +error: this ident is too short (3 <= 3) --> $DIR/min_ident_chars.rs:6:19 | LL | use extern_types::Aaa; @@ -6,11 +6,41 @@ LL | use extern_types::Aaa; | = note: `-D clippy::min-ident-chars` implied by `-D warnings` -error: this ident is too short (3 <= 3) +error: this ident is too short (3 <= 3) --> $DIR/min_ident_chars.rs:10:5 | LL | aaa: Aaa, | ^^^ -error: aborting due to 2 previous errors +error: this ident is too short (3 <= 3) + --> $DIR/min_ident_chars.rs:15:9 + | +LL | let vvv = 1; + | ^^^ + +error: this ident is too short (3 <= 3) + --> $DIR/min_ident_chars.rs:16:9 + | +LL | let uuu = 1; + | ^^^ + +error: this ident is too short (1 <= 3) + --> $DIR/min_ident_chars.rs:17:14 + | +LL | let (mut a, mut b) = (1, 2); + | ^ + +error: this ident is too short (1 <= 3) + --> $DIR/min_ident_chars.rs:17:21 + | +LL | let (mut a, mut b) = (1, 2); + | ^ + +error: this ident is too short (1 <= 3) + --> $DIR/min_ident_chars.rs:18:9 + | +LL | for i in 0..1000 {} + | ^ + +error: aborting due to 7 previous errors diff --git a/tests/ui/min_ident_chars.rs b/tests/ui/min_ident_chars.rs index 134c17c553f3e..85608d030edae 100644 --- a/tests/ui/min_ident_chars.rs +++ b/tests/ui/min_ident_chars.rs @@ -1,5 +1,5 @@ //@aux-build:proc_macros.rs -#![allow(nonstandard_style, unused)] +#![allow(irrefutable_let_patterns, nonstandard_style, unused)] #![warn(clippy::min_ident_chars)] extern crate proc_macros; @@ -15,6 +15,10 @@ struct A { struct B(u32); +struct O { + o: u32, +} + struct i; enum C { @@ -38,11 +42,29 @@ fn main() { let w = 1; // Ok, not this one // let i = 1; - let jz = 1; - let nz = 1; - let zx = 1; - let yz = 1; - let zz = 1; + let j = 1; + let n = 1; + let z = 1; + let y = 1; + let z = 1; + // Implicitly disallowed idents + let h = 1; + let e = 2; + let l = 3; + let l = 4; + let o = 6; + // 2 len does not lint + let hi = 0; + // Lint + let (h, o, w) = (1, 2, 3); + for (a, (r, e)) in (0..1000).enumerate().enumerate() {} + let you = Vec4 { x: 1, y: 2, z: 3, w: 4 }; + while let (d, o, _i, n, g) = (true, true, false, false, true) {} + let today = true; + // Ideally this wouldn't lint, but this would (likely) require global analysis, outta scope + // of this lint regardless + let o = 1; + let o = O { o }; for j in 0..1000 {} diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr index 4b887617058bb..cd8825634c319 100644 --- a/tests/ui/min_ident_chars.stderr +++ b/tests/ui/min_ident_chars.stderr @@ -31,40 +31,136 @@ LL | struct B(u32); | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:20:6 + --> $DIR/min_ident_chars.rs:18:8 + | +LL | struct O { + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:19:5 + | +LL | o: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:24:6 | LL | enum C { | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:21:5 + --> $DIR/min_ident_chars.rs:25:5 | LL | D, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:22:5 + --> $DIR/min_ident_chars.rs:26:5 | LL | E, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:23:5 + --> $DIR/min_ident_chars.rs:27:5 | LL | F, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:31:5 + --> $DIR/min_ident_chars.rs:51:9 | -LL | w: u32, - | ^ +LL | let h = 1; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:52:9 + | +LL | let e = 2; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:53:9 + | +LL | let l = 3; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:54:9 + | +LL | let l = 4; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:55:9 + | +LL | let o = 6; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:59:10 + | +LL | let (h, o, w) = (1, 2, 3); + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:59:13 + | +LL | let (h, o, w) = (1, 2, 3); + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:60:10 + | +LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:60:14 + | +LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:60:17 + | +LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:62:16 + | +LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:62:19 + | +LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:62:29 + | +LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:66:9 + | +LL | let o = 1; + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:67:9 + | +LL | let o = O { o }; + | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:58:4 + --> $DIR/min_ident_chars.rs:80:4 | LL | fn b() {} | ^ -error: aborting due to 11 previous errors +error: aborting due to 27 previous errors From 95d1bff2251ff33e1e8ca2bc0c1941c4e91bc2e4 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sat, 10 Jun 2023 09:13:04 -0500 Subject: [PATCH 6/8] add to tests and configuration --- clippy_lints/src/min_ident_chars.rs | 40 ++++++++++++++--------------- clippy_lints/src/utils/conf.rs | 10 +++++++- tests/ui/min_ident_chars.rs | 5 +++- tests/ui/min_ident_chars.stderr | 16 ++++++++++-- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs index cc027b6f4988d..316c2bf122cb5 100644 --- a/clippy_lints/src/min_ident_chars.rs +++ b/clippy_lints/src/min_ident_chars.rs @@ -8,6 +8,7 @@ use rustc_hir::{ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; use std::borrow::Cow; declare_clippy_lint! { @@ -56,26 +57,18 @@ impl LateLintPass<'_> for MinIdentChars { walk_item(&mut IdentVisitor { conf: self, cx }, item); } - // This is necessary as bindings are not visited in `visit_id`. :/ + // This is necessary as `Node::Pat`s are not visited in `visit_id`. :/ #[expect(clippy::cast_possible_truncation)] fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { if let PatKind::Binding(_, _, ident, ..) = pat.kind && let str = ident.as_str() && !in_external_macro(cx.sess(), ident.span) && str.len() <= self.min_ident_chars_threshold as usize + && !str.starts_with('_') && !str.is_empty() && self.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() { - let help = if self.min_ident_chars_threshold == 1 { - Cow::Borrowed("this ident consists of a single char") - } else { - Cow::Owned(format!( - "this ident is too short ({} <= {})", - str.len(), - self.min_ident_chars_threshold, - )) - }; - span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); + emit_min_ident_chars(self, cx, str, ident.span); } } } @@ -112,6 +105,7 @@ impl Visitor<'_> for IdentVisitor<'_, '_> { let str = ident.as_str(); if !in_external_macro(cx.sess(), ident.span) && str.len() <= conf.min_ident_chars_threshold as usize + && !str.starts_with('_') && !str.is_empty() && conf.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() { @@ -141,16 +135,20 @@ impl Visitor<'_> for IdentVisitor<'_, '_> { return; } - let help = if conf.min_ident_chars_threshold == 1 { - Cow::Borrowed("this ident consists of a single char") - } else { - Cow::Owned(format!( - "this ident is too short ({} <= {})", - str.len(), - conf.min_ident_chars_threshold, - )) - }; - span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); + emit_min_ident_chars(conf, cx, str, ident.span); } } } + +fn emit_min_ident_chars(conf: &MinIdentChars, cx: &impl LintContext, ident: &str, span: Span) { + let help = if conf.min_ident_chars_threshold == 1 { + Cow::Borrowed("this ident consists of a single char") + } else { + Cow::Owned(format!( + "this ident is too short ({} <= {})", + ident.len(), + conf.min_ident_chars_threshold, + )) + }; + span_lint(cx, MIN_IDENT_CHARS, span, &help); +} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index f050782a16dd5..0a581e0ab9b19 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -525,7 +525,9 @@ define_Conf! { (allow_private_module_inception: bool = false), /// Lint: MIN_IDENT_CHARS. /// - /// Allowed names below the minimum allowed characters. + /// Allowed names below the minimum allowed characters. The value `".."` can be used as part of + /// the list to indicate, that the configured values should be appended to the default + /// configuration of Clippy. By default, any configuration will replace the default value. (allowed_idents_below_min_chars: rustc_data_structures::fx::FxHashSet = super::DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string).collect()), /// Lint: MIN_IDENT_CHARS. @@ -599,6 +601,12 @@ pub fn read(sess: &Session, path: &Path) -> TryConf { Ok(mut conf) => { extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS); extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES); + // TODO: THIS SHOULD BE TESTED, this comment will be gone soon + if conf.conf.allowed_idents_below_min_chars.contains(&"..".to_owned()) { + conf.conf + .allowed_idents_below_min_chars + .extend(DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string)); + } conf }, diff --git a/tests/ui/min_ident_chars.rs b/tests/ui/min_ident_chars.rs index 85608d030edae..c5e03468ef9df 100644 --- a/tests/ui/min_ident_chars.rs +++ b/tests/ui/min_ident_chars.rs @@ -67,6 +67,7 @@ fn main() { let o = O { o }; for j in 0..1000 {} + for _ in 0..10 {} // Do not lint code from external macros external! { for j in 0..1000 {} } @@ -78,4 +79,6 @@ fn main() { } fn b() {} -fn owo() {} +fn wrong_pythagoras(a: f32, b: f32) -> f32 { + a * a + a * b +} diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr index cd8825634c319..66a63f6575656 100644 --- a/tests/ui/min_ident_chars.stderr +++ b/tests/ui/min_ident_chars.stderr @@ -157,10 +157,22 @@ LL | let o = O { o }; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:80:4 + --> $DIR/min_ident_chars.rs:81:4 | LL | fn b() {} | ^ -error: aborting due to 27 previous errors +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:82:21 + | +LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:82:29 + | +LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { + | ^ + +error: aborting due to 29 previous errors From 203e875189647fa986e5d31998a7ecc4f3340c01 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sat, 10 Jun 2023 09:35:42 -0500 Subject: [PATCH 7/8] `cargo collect-metadata` --- book/src/lint_configuration.md | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 1812d50ed1bc0..b2e11d8de677e 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -644,3 +644,44 @@ The maximum byte size a `Future` can have, before it triggers the `clippy::large ## `unnecessary-box-size` +The byte size a `T` in `Box` can have, below which it triggers the `clippy::unnecessary_box` lint + +**Default Value:** `128` (`u64`) + +--- +**Affected lints:** +* [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns) + + +## `allow-private-module-inception` +Whether to allow module inception if it's not public. + +**Default Value:** `false` (`bool`) + +--- +**Affected lints:** +* [`module_inception`](https://rust-lang.github.io/rust-clippy/master/index.html#module_inception) + + +## `allowed-idents-below-min-chars` +Allowed names below the minimum allowed characters. The value `".."` can be used as part of +the list to indicate, that the configured values should be appended to the default +configuration of Clippy. By default, any configuration will replace the default value. + +**Default Value:** `{"j", "z", "i", "y", "n", "x", "w"}` (`rustc_data_structures::fx::FxHashSet`) + +--- +**Affected lints:** +* [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars) + + +## `min-ident-chars-threshold` +Minimum chars an ident can have, anything below or equal to this will be linted. + +**Default Value:** `1` (`u64`) + +--- +**Affected lints:** +* [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars) + + From 29c1c6e104340b04505572220b29e5e76d7053b9 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sun, 11 Jun 2023 14:51:50 -0500 Subject: [PATCH 8/8] refactor and add link to issue --- clippy_lints/src/min_ident_chars.rs | 33 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs index 316c2bf122cb5..d49bb0ca6e291 100644 --- a/clippy_lints/src/min_ident_chars.rs +++ b/clippy_lints/src/min_ident_chars.rs @@ -34,7 +34,6 @@ declare_clippy_lint! { /// let title = movie.title; /// } /// ``` - /// ``` #[clippy::version = "1.72.0"] pub MIN_IDENT_CHARS, restriction, @@ -48,6 +47,17 @@ pub struct MinIdentChars { pub min_ident_chars_threshold: u64, } +impl MinIdentChars { + #[expect(clippy::cast_possible_truncation)] + fn is_ident_too_short(&self, cx: &LateContext<'_>, str: &str, span: Span) -> bool { + !in_external_macro(cx.sess(), span) + && str.len() <= self.min_ident_chars_threshold as usize + && !str.starts_with('_') + && !str.is_empty() + && self.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() + } +} + impl LateLintPass<'_> for MinIdentChars { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if self.min_ident_chars_threshold == 0 { @@ -58,15 +68,10 @@ impl LateLintPass<'_> for MinIdentChars { } // This is necessary as `Node::Pat`s are not visited in `visit_id`. :/ - #[expect(clippy::cast_possible_truncation)] fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { if let PatKind::Binding(_, _, ident, ..) = pat.kind && let str = ident.as_str() - && !in_external_macro(cx.sess(), ident.span) - && str.len() <= self.min_ident_chars_threshold as usize - && !str.starts_with('_') - && !str.is_empty() - && self.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() + && self.is_ident_too_short(cx, str, ident.span) { emit_min_ident_chars(self, cx, str, ident.span); } @@ -79,12 +84,11 @@ struct IdentVisitor<'cx, 'tcx> { } impl Visitor<'_> for IdentVisitor<'_, '_> { - #[expect(clippy::cast_possible_truncation)] fn visit_id(&mut self, hir_id: HirId) { let Self { conf, cx } = *self; - // Reimplementation of `find`, as it uses indexing, which can (and will in async functions) panic. - // This should probably be fixed on the rustc side, this is just a temporary workaround. - // FIXME: Remove me if/when this is fixed in rustc + // FIXME(#112534) Reimplementation of `find`, as it uses indexing, which can (and will in + // async functions, or really anything async) panic. This should probably be fixed on the + // rustc side, this is just a temporary workaround. let node = if hir_id.local_id == ItemLocalId::from_u32(0) { // In this case, we can just use `find`, `Owner`'s `node` field is private anyway so we can't // reimplement it even if we wanted to @@ -103,12 +107,7 @@ impl Visitor<'_> for IdentVisitor<'_, '_> { }; let str = ident.as_str(); - if !in_external_macro(cx.sess(), ident.span) - && str.len() <= conf.min_ident_chars_threshold as usize - && !str.starts_with('_') - && !str.is_empty() - && conf.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() - { + if conf.is_ident_too_short(cx, str, ident.span) { if let Node::Item(item) = node && let ItemKind::Use(..) = item.kind { return; }