Auto merge of rust-lang#10916 - Centri3:single_letter_idents, r=blyxyas,xFrednet

New lint [`min_ident_chars`]

Closes rust-lang#10915

This also implements `WithSearchPat` for `Ident`, as I was going to rewrite this as a late lint to optionally ignore fields and/or generics but this was more complex than anticipated

changelog: New lint [`min_ident_chars`]
[rust-lang#10916](rust-lang/rust-clippy#10916)

Author: bors

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

book/src/lint_configuration.md

@@ -663,3 +663,25 @@ Whether to allow module inception if it's not public.
 * [`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<String>`)
+
+---
+**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)
+
+

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,

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;
@@ -1033,6 +1034,14 @@ 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_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`
 }
 

clippy_lints/src/min_ident_chars.rs

@@ -0,0 +1,153 @@
+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, Pat, PatKind,
+};
+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! {
+    /// ### 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<String>,
+    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 {
+            return;
+        }
+
+        walk_item(&mut IdentVisitor { conf: self, cx }, item);
+    }
+
+    // This is necessary as `Node::Pat`s are not visited in `visit_id`. :/
+    fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) {
+        if let PatKind::Binding(_, _, ident, ..) = pat.kind
+            && let str = ident.as_str()
+            && self.is_ident_too_short(cx, str, ident.span)
+        {
+            emit_min_ident_chars(self, cx, str, ident.span);
+        }
+    }
+}
+
+struct IdentVisitor<'cx, 'tcx> {
+    conf: &'cx MinIdentChars,
+    cx: &'cx LateContext<'tcx>,
+}
+
+impl Visitor<'_> for IdentVisitor<'_, '_> {
+    fn visit_id(&mut self, hir_id: HirId) {
+        let Self { conf, cx } = *self;
+        // 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
+            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 conf.is_ident_too_short(cx, str, ident.span) {
+            if let Node::Item(item) = node && let ItemKind::Use(..) = item.kind {
+                return;
+            }
+            // `struct Awa<T>(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>(T)`
+            //             ^
+            if let Node::GenericParam(generic_param) = node
+                && let GenericParamKind::Type { .. } = generic_param.kind
+            {
+                return;
+            }
+
+            if is_from_proc_macro(cx, &ident) {
+                return;
+            }
+
+            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);
+}

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_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"];
 
 /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint.
 #[derive(Clone, Debug, Deserialize)]
@@ -522,6 +523,17 @@ 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. 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<String> =
+        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.
@@ -589,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
         },

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/min_ident_chars/auxiliary/extern_types.rs

@@ -0,0 +1,3 @@
+#![allow(nonstandard_style, unused)]
+
+pub struct Aaa;

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

tests/ui-toml/min_ident_chars/min_ident_chars.rs

@@ -0,0 +1,19 @@
+//@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;
+    let (mut a, mut b) = (1, 2);
+    for i in 0..1000 {}
+}