Skip to content

Commit a3b185b

Browse files
committed
Auto merge of #10916 - Centri3:single_letter_idents, r=blyxyas,xFrednet
New lint [`min_ident_chars`] Closes #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`] [#10916](#10916)
2 parents 903fe3b + 29c1c6e commit a3b185b

File tree

14 files changed

+563
-6
lines changed

14 files changed

+563
-6
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4958,6 +4958,7 @@ Released 2018-09-13
49584958
[`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
49594959
[`mem_replace_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default
49604960
[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
4961+
[`min_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars
49614962
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
49624963
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
49634964
[`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os

book/src/lint_configuration.md

+22
Original file line numberDiff line numberDiff line change
@@ -663,3 +663,25 @@ Whether to allow module inception if it's not public.
663663
* [`module_inception`](https://rust-lang.github.io/rust-clippy/master/index.html#module_inception)
664664

665665

666+
## `allowed-idents-below-min-chars`
667+
Allowed names below the minimum allowed characters. The value `".."` can be used as part of
668+
the list to indicate, that the configured values should be appended to the default
669+
configuration of Clippy. By default, any configuration will replace the default value.
670+
671+
**Default Value:** `{"j", "z", "i", "y", "n", "x", "w"}` (`rustc_data_structures::fx::FxHashSet<String>`)
672+
673+
---
674+
**Affected lints:**
675+
* [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars)
676+
677+
678+
## `min-ident-chars-threshold`
679+
Minimum chars an ident can have, anything below or equal to this will be linted.
680+
681+
**Default Value:** `1` (`u64`)
682+
683+
---
684+
**Affected lints:**
685+
* [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars)
686+
687+

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
416416
crate::methods::VERBOSE_FILE_READS_INFO,
417417
crate::methods::WRONG_SELF_CONVENTION_INFO,
418418
crate::methods::ZST_OFFSET_INFO,
419+
crate::min_ident_chars::MIN_IDENT_CHARS_INFO,
419420
crate::minmax::MIN_MAX_INFO,
420421
crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
421422
crate::misc::TOPLEVEL_REF_ARG_INFO,

clippy_lints/src/lib.rs

+9
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ mod matches;
197197
mod mem_forget;
198198
mod mem_replace;
199199
mod methods;
200+
mod min_ident_chars;
200201
mod minmax;
201202
mod misc;
202203
mod misc_early;
@@ -1033,6 +1034,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10331034
store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations));
10341035
store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync));
10351036
store.register_late_pass(|_| Box::new(needless_if::NeedlessIf));
1037+
let allowed_idents_below_min_chars = conf.allowed_idents_below_min_chars.clone();
1038+
let min_ident_chars_threshold = conf.min_ident_chars_threshold;
1039+
store.register_late_pass(move |_| {
1040+
Box::new(min_ident_chars::MinIdentChars {
1041+
allowed_idents_below_min_chars: allowed_idents_below_min_chars.clone(),
1042+
min_ident_chars_threshold,
1043+
})
1044+
});
10361045
// add lints here, do not remove this comment, it's used in `new_lint`
10371046
}
10381047

clippy_lints/src/min_ident_chars.rs

+153
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
use clippy_utils::{diagnostics::span_lint, is_from_proc_macro};
2+
use rustc_data_structures::fx::FxHashSet;
3+
use rustc_hir::{
4+
def::{DefKind, Res},
5+
intravisit::{walk_item, Visitor},
6+
GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, Pat, PatKind,
7+
};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::lint::in_external_macro;
10+
use rustc_session::{declare_tool_lint, impl_lint_pass};
11+
use rustc_span::Span;
12+
use std::borrow::Cow;
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// Checks for idents which comprise of a single letter.
17+
///
18+
/// Note: This lint can be very noisy when enabled; it may be desirable to only enable it
19+
/// temporarily.
20+
///
21+
/// ### Why is this bad?
22+
/// In many cases it's not, but at times it can severely hinder readability. Some codebases may
23+
/// wish to disallow this to improve readability.
24+
///
25+
/// ### Example
26+
/// ```rust,ignore
27+
/// for m in movies {
28+
/// let title = m.t;
29+
/// }
30+
/// ```
31+
/// Use instead:
32+
/// ```rust,ignore
33+
/// for movie in movies {
34+
/// let title = movie.title;
35+
/// }
36+
/// ```
37+
#[clippy::version = "1.72.0"]
38+
pub MIN_IDENT_CHARS,
39+
restriction,
40+
"disallows idents that are too short"
41+
}
42+
impl_lint_pass!(MinIdentChars => [MIN_IDENT_CHARS]);
43+
44+
#[derive(Clone)]
45+
pub struct MinIdentChars {
46+
pub allowed_idents_below_min_chars: FxHashSet<String>,
47+
pub min_ident_chars_threshold: u64,
48+
}
49+
50+
impl MinIdentChars {
51+
#[expect(clippy::cast_possible_truncation)]
52+
fn is_ident_too_short(&self, cx: &LateContext<'_>, str: &str, span: Span) -> bool {
53+
!in_external_macro(cx.sess(), span)
54+
&& str.len() <= self.min_ident_chars_threshold as usize
55+
&& !str.starts_with('_')
56+
&& !str.is_empty()
57+
&& self.allowed_idents_below_min_chars.get(&str.to_owned()).is_none()
58+
}
59+
}
60+
61+
impl LateLintPass<'_> for MinIdentChars {
62+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
63+
if self.min_ident_chars_threshold == 0 {
64+
return;
65+
}
66+
67+
walk_item(&mut IdentVisitor { conf: self, cx }, item);
68+
}
69+
70+
// This is necessary as `Node::Pat`s are not visited in `visit_id`. :/
71+
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) {
72+
if let PatKind::Binding(_, _, ident, ..) = pat.kind
73+
&& let str = ident.as_str()
74+
&& self.is_ident_too_short(cx, str, ident.span)
75+
{
76+
emit_min_ident_chars(self, cx, str, ident.span);
77+
}
78+
}
79+
}
80+
81+
struct IdentVisitor<'cx, 'tcx> {
82+
conf: &'cx MinIdentChars,
83+
cx: &'cx LateContext<'tcx>,
84+
}
85+
86+
impl Visitor<'_> for IdentVisitor<'_, '_> {
87+
fn visit_id(&mut self, hir_id: HirId) {
88+
let Self { conf, cx } = *self;
89+
// FIXME(#112534) Reimplementation of `find`, as it uses indexing, which can (and will in
90+
// async functions, or really anything async) panic. This should probably be fixed on the
91+
// rustc side, this is just a temporary workaround.
92+
let node = if hir_id.local_id == ItemLocalId::from_u32(0) {
93+
// In this case, we can just use `find`, `Owner`'s `node` field is private anyway so we can't
94+
// reimplement it even if we wanted to
95+
cx.tcx.hir().find(hir_id)
96+
} else {
97+
let Some(owner) = cx.tcx.hir_owner_nodes(hir_id.owner).as_owner() else {
98+
return;
99+
};
100+
owner.nodes.get(hir_id.local_id).copied().flatten().map(|p| p.node)
101+
};
102+
let Some(node) = node else {
103+
return;
104+
};
105+
let Some(ident) = node.ident() else {
106+
return;
107+
};
108+
109+
let str = ident.as_str();
110+
if conf.is_ident_too_short(cx, str, ident.span) {
111+
if let Node::Item(item) = node && let ItemKind::Use(..) = item.kind {
112+
return;
113+
}
114+
// `struct Awa<T>(T)`
115+
// ^
116+
if let Node::PathSegment(path) = node {
117+
if let Res::Def(def_kind, ..) = path.res && let DefKind::TyParam = def_kind {
118+
return;
119+
}
120+
if matches!(path.res, Res::PrimTy(..)) || path.res.opt_def_id().is_some_and(|def_id| !def_id.is_local())
121+
{
122+
return;
123+
}
124+
}
125+
// `struct Awa<T>(T)`
126+
// ^
127+
if let Node::GenericParam(generic_param) = node
128+
&& let GenericParamKind::Type { .. } = generic_param.kind
129+
{
130+
return;
131+
}
132+
133+
if is_from_proc_macro(cx, &ident) {
134+
return;
135+
}
136+
137+
emit_min_ident_chars(conf, cx, str, ident.span);
138+
}
139+
}
140+
}
141+
142+
fn emit_min_ident_chars(conf: &MinIdentChars, cx: &impl LintContext, ident: &str, span: Span) {
143+
let help = if conf.min_ident_chars_threshold == 1 {
144+
Cow::Borrowed("this ident consists of a single char")
145+
} else {
146+
Cow::Owned(format!(
147+
"this ident is too short ({} <= {})",
148+
ident.len(),
149+
conf.min_ident_chars_threshold,
150+
))
151+
};
152+
span_lint(cx, MIN_IDENT_CHARS, span, &help);
153+
}

clippy_lints/src/utils/conf.rs

+18
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
3434
"CamelCase",
3535
];
3636
const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"];
37+
const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"];
3738

3839
/// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint.
3940
#[derive(Clone, Debug, Deserialize)]
@@ -522,6 +523,17 @@ define_Conf! {
522523
///
523524
/// Whether to allow module inception if it's not public.
524525
(allow_private_module_inception: bool = false),
526+
/// Lint: MIN_IDENT_CHARS.
527+
///
528+
/// Allowed names below the minimum allowed characters. The value `".."` can be used as part of
529+
/// the list to indicate, that the configured values should be appended to the default
530+
/// configuration of Clippy. By default, any configuration will replace the default value.
531+
(allowed_idents_below_min_chars: rustc_data_structures::fx::FxHashSet<String> =
532+
super::DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string).collect()),
533+
/// Lint: MIN_IDENT_CHARS.
534+
///
535+
/// Minimum chars an ident can have, anything below or equal to this will be linted.
536+
(min_ident_chars_threshold: u64 = 1),
525537
}
526538

527539
/// Search for the configuration file.
@@ -589,6 +601,12 @@ pub fn read(sess: &Session, path: &Path) -> TryConf {
589601
Ok(mut conf) => {
590602
extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS);
591603
extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
604+
// TODO: THIS SHOULD BE TESTED, this comment will be gone soon
605+
if conf.conf.allowed_idents_below_min_chars.contains(&"..".to_owned()) {
606+
conf.conf
607+
.allowed_idents_below_min_chars
608+
.extend(DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string));
609+
}
592610

593611
conf
594612
},

clippy_utils/src/check_proc_macro.rs

+23-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_hir::{
2525
use rustc_lint::{LateContext, LintContext};
2626
use rustc_middle::ty::TyCtxt;
2727
use rustc_session::Session;
28-
use rustc_span::{Span, Symbol};
28+
use rustc_span::{symbol::Ident, Span, Symbol};
2929
use rustc_target::spec::abi::Abi;
3030

3131
/// The search pattern to look for. Used by `span_matches_pat`
@@ -344,14 +344,18 @@ fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) {
344344
}
345345
}
346346

347-
pub trait WithSearchPat {
347+
fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
348+
(Pat::OwnedStr(ident.name.as_str().to_owned()), Pat::Str(""))
349+
}
350+
351+
pub trait WithSearchPat<'cx> {
348352
type Context: LintContext;
349353
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat);
350354
fn span(&self) -> Span;
351355
}
352356
macro_rules! impl_with_search_pat {
353357
($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => {
354-
impl<'cx> WithSearchPat for $ty<'cx> {
358+
impl<'cx> WithSearchPat<'cx> for $ty<'cx> {
355359
type Context = $cx<'cx>;
356360
#[allow(unused_variables)]
357361
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) {
@@ -372,7 +376,7 @@ impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat);
372376
impl_with_search_pat!(LateContext: Variant with variant_search_pat);
373377
impl_with_search_pat!(LateContext: Ty with ty_search_pat);
374378

375-
impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
379+
impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
376380
type Context = LateContext<'cx>;
377381

378382
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) {
@@ -385,7 +389,7 @@ impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
385389
}
386390

387391
// `Attribute` does not have the `hir` associated lifetime, so we cannot use the macro
388-
impl<'cx> WithSearchPat for &'cx Attribute {
392+
impl<'cx> WithSearchPat<'cx> for &'cx Attribute {
389393
type Context = LateContext<'cx>;
390394

391395
fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) {
@@ -397,11 +401,24 @@ impl<'cx> WithSearchPat for &'cx Attribute {
397401
}
398402
}
399403

404+
// `Ident` does not have the `hir` associated lifetime, so we cannot use the macro
405+
impl<'cx> WithSearchPat<'cx> for Ident {
406+
type Context = LateContext<'cx>;
407+
408+
fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) {
409+
ident_search_pat(*self)
410+
}
411+
412+
fn span(&self) -> Span {
413+
self.span
414+
}
415+
}
416+
400417
/// Checks if the item likely came from a proc-macro.
401418
///
402419
/// This should be called after `in_external_macro` and the initial pattern matching of the ast as
403420
/// it is significantly slower than both of those.
404-
pub fn is_from_proc_macro<T: WithSearchPat>(cx: &T::Context, item: &T) -> bool {
421+
pub fn is_from_proc_macro<'cx, T: WithSearchPat<'cx>>(cx: &T::Context, item: &T) -> bool {
405422
let (start_pat, end_pat) = item.search_pat(cx);
406423
!span_matches_pat(cx.sess(), item.span(), start_pat, end_pat)
407424
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#![allow(nonstandard_style, unused)]
2+
3+
pub struct Aaa;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
allowed-idents-below-min-chars = ["Owo", "Uwu", "wha", "t_e", "lse", "_do", "_i_", "put", "her", "_e"]
2+
min-ident-chars-threshold = 3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@aux-build:extern_types.rs
2+
#![allow(nonstandard_style, unused)]
3+
#![warn(clippy::min_ident_chars)]
4+
5+
extern crate extern_types;
6+
use extern_types::Aaa;
7+
8+
struct Owo {
9+
Uwu: u128,
10+
aaa: Aaa,
11+
}
12+
13+
fn main() {
14+
let wha = 1;
15+
let vvv = 1;
16+
let uuu = 1;
17+
let (mut a, mut b) = (1, 2);
18+
for i in 0..1000 {}
19+
}

0 commit comments

Comments
 (0)