diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a330b35d..5a03d768f 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 874c173f7..03c3e153a 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 fd10a9302..9b8edc5d7 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 000000000..e957b6fec --- /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 6962299e4..7b5413647 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 bcfac527c..c6d0b654f 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 c11835f56..2bc872e07 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 000000000..b33c569d3 --- /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 000000000..712c16642 --- /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 +