Add lint `single_letter_idents`

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

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,

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`
 }
 

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<char>,
+}
+
+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");
+        }
+    }
+}

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)]

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<T: WithSearchPat>(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)
 }

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

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>(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() {}

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>(T, E);
+   |           ^
+
+error: this ident comprises of a single letter
+  --> $DIR/single_letter_idents.rs:34:14
+   |
+LL | struct AA<T, E>(T, E);
+   |              ^
+
+error: this ident comprises of a single letter
+  --> $DIR/single_letter_idents.rs:34:17
+   |
+LL | struct AA<T, E>(T, E);
+   |                 ^
+
+error: this ident comprises of a single letter
+  --> $DIR/single_letter_idents.rs:34:20
+   |
+LL | struct AA<T, E>(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
+