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](https://github.com/rust-lang/rust-clippy/pull/10916)
This commit is contained in:
bors 2023-06-12 08:40:17 +00:00
commit a3b185b60a
14 changed files with 563 additions and 6 deletions

View file

@ -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

View file

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

View file

@ -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,

View file

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

View file

@ -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);
}

View file

@ -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
},

View file

@ -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)
}

View file

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

View file

@ -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

View file

@ -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 {}
}

View file

@ -0,0 +1,46 @@
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: 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

View file

@ -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-below-min-chars
allowed-scripts
arithmetic-side-effects-allowed
arithmetic-side-effects-allowed-binary
@ -36,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
@ -68,6 +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-below-min-chars
allowed-scripts
arithmetic-side-effects-allowed
arithmetic-side-effects-allowed-binary
@ -99,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

View file

@ -0,0 +1,84 @@
//@aux-build:proc_macros.rs
#![allow(irrefutable_let_patterns, nonstandard_style, unused)]
#![warn(clippy::min_ident_chars)]
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 O {
o: 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 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 {}
for _ in 0..10 {}
// 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 wrong_pythagoras(a: f32, b: f32) -> f32 {
a * a + a * b
}

View file

@ -0,0 +1,178 @@
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: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:25:5
|
LL | D,
| ^
error: this ident consists of a single char
--> $DIR/min_ident_chars.rs:26:5
|
LL | E,
| ^
error: this ident consists of a single char
--> $DIR/min_ident_chars.rs:27:5
|
LL | F,
| ^
error: this ident consists of a single char
--> $DIR/min_ident_chars.rs:51:9
|
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:81:4
|
LL | fn b() {}
| ^
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