From 7cdd87ca4afe3ab7a1a8eb1544db132678335f87 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Fri, 9 Jun 2023 17:49:35 -0500 Subject: [PATCH] ignore generics and allow arbitrary threshold --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/lib.rs | 12 +- clippy_lints/src/min_ident_chars.rs | 133 ++++++++++++++++++ clippy_lints/src/single_char_idents.rs | 59 -------- clippy_lints/src/utils/conf.rs | 11 +- .../min_ident_chars/auxiliary/extern_types.rs | 3 + tests/ui-toml/min_ident_chars/clippy.toml | 2 + .../min_ident_chars/min_ident_chars.rs | 17 +++ .../min_ident_chars/min_ident_chars.stderr | 16 +++ .../toml_unknown_key/conf_unknown_key.stderr | 6 +- ...ngle_char_idents.rs => min_ident_chars.rs} | 12 +- tests/ui/min_ident_chars.stderr | 70 +++++++++ tests/ui/single_char_idents.stderr | 100 ------------- 14 files changed, 270 insertions(+), 175 deletions(-) create mode 100644 clippy_lints/src/min_ident_chars.rs delete mode 100644 clippy_lints/src/single_char_idents.rs create mode 100644 tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs create mode 100644 tests/ui-toml/min_ident_chars/clippy.toml create mode 100644 tests/ui-toml/min_ident_chars/min_ident_chars.rs create mode 100644 tests/ui-toml/min_ident_chars/min_ident_chars.stderr rename tests/ui/{single_char_idents.rs => min_ident_chars.rs} (86%) create mode 100644 tests/ui/min_ident_chars.stderr delete mode 100644 tests/ui/single_char_idents.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 27a5fc06b07..2d62bfd4f99 100644 --- a/CHANGELOG.md +++ b/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 @@ -5155,7 +5156,6 @@ Released 2018-09-13 [`significant_drop_tightening`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening [`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names [`single_char_add_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_add_str -[`single_char_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_idents [`single_char_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_lifetime_names [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1c494468226..769774b27c4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, @@ -565,7 +566,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::shadow::SHADOW_SAME_INFO, crate::shadow::SHADOW_UNRELATED_INFO, crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, - crate::single_char_idents::SINGLE_CHAR_IDENTS_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 463e4a429d1..dcf1c6f64a4 100644 --- a/clippy_lints/src/lib.rs +++ b/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; @@ -284,7 +285,6 @@ mod semicolon_if_nothing_returned; mod serde_api; mod shadow; mod significant_drop_tightening; -mod single_char_idents; mod single_char_lifetime_names; mod single_component_path_imports; mod size_of_in_element_count; @@ -1034,10 +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_char_idents::SingleCharIdents { - allowed_idents: allowed_idents.clone(), + 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` diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs new file mode 100644 index 00000000000..bbe7fc540e9 --- /dev/null +++ b/clippy_lints/src/min_ident_chars.rs @@ -0,0 +1,133 @@ +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, +}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +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, + pub min_ident_chars_threshold: u64, +} + +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); + } +} + +struct IdentVisitor<'cx, 'tcx> { + conf: &'cx MinIdentChars, + cx: &'cx LateContext<'tcx>, +} + +#[expect(clippy::cast_possible_truncation)] +impl Visitor<'_> for IdentVisitor<'_, '_> { + fn visit_id(&mut self, hir_id: HirId) { + let Self { conf, cx } = *self; + // Reimplementation of `find`, as it uses indexing, which can (and will in async functions) panic. + // This should probably be fixed on the rustc side, this is just a temporary workaround. + // FIXME: Remove me if/when this is fixed in rustc + 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 !in_external_macro(cx.sess(), ident.span) + && str.len() <= conf.min_ident_chars_threshold as usize + && !str.is_empty() + && conf.allowed_idents_below_min_chars.get(&str.to_owned()).is_none() + { + if let Node::Item(item) = node && let ItemKind::Use(..) = item.kind { + return; + } + // `struct Awa(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)` + // ^ + if let Node::GenericParam(generic_param) = node + && let GenericParamKind::Type { .. } = generic_param.kind + { + return; + } + + if is_from_proc_macro(cx, &ident) { + return; + } + + 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 ({} <= {}) ", + str.len(), + conf.min_ident_chars_threshold + )) + }; + span_lint(cx, MIN_IDENT_CHARS, ident.span, &help); + } + } +} diff --git a/clippy_lints/src/single_char_idents.rs b/clippy_lints/src/single_char_idents.rs deleted file mode 100644 index 17a2ebe93ed..00000000000 --- a/clippy_lints/src/single_char_idents.rs +++ /dev/null @@ -1,59 +0,0 @@ -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 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 SINGLE_CHAR_IDENTS, - restriction, - "disallows idents that can be represented as a char" -} -impl_lint_pass!(SingleCharIdents => [SINGLE_CHAR_IDENTS]); - -#[derive(Clone)] -pub struct SingleCharIdents { - pub allowed_idents: FxHashSet, -} - -impl EarlyLintPass for SingleCharIdents { - 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() - && 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_CHAR_IDENTS, ident.span, "this ident comprises of a single char"); - } - } -} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 7b5413647c7..c84d8d8d1f0 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -34,7 +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']; +const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "n"]; /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. #[derive(Clone, Debug, Deserialize)] @@ -523,6 +523,15 @@ 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. + (allowed_idents_below_min_chars: rustc_data_structures::fx::FxHashSet = + 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. diff --git a/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs b/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs new file mode 100644 index 00000000000..06a144f2218 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs @@ -0,0 +1,3 @@ +#![allow(nonstandard_style, unused)] + +pub struct Aaa; diff --git a/tests/ui-toml/min_ident_chars/clippy.toml b/tests/ui-toml/min_ident_chars/clippy.toml new file mode 100644 index 00000000000..0114ca75014 --- /dev/null +++ b/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 diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.rs b/tests/ui-toml/min_ident_chars/min_ident_chars.rs new file mode 100644 index 00000000000..33100119ca5 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.rs @@ -0,0 +1,17 @@ +//@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; +} diff --git a/tests/ui-toml/min_ident_chars/min_ident_chars.stderr b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr new file mode 100644 index 00000000000..541f5b10051 --- /dev/null +++ b/tests/ui-toml/min_ident_chars/min_ident_chars.stderr @@ -0,0 +1,16 @@ +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: aborting due to 2 previous errors + 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 2bc872e07e8..c546d95eb46 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -5,7 +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-idents-below-min-chars allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary @@ -37,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 @@ -69,7 +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 + allowed-idents-below-min-chars allowed-scripts arithmetic-side-effects-allowed arithmetic-side-effects-allowed-binary @@ -101,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 diff --git a/tests/ui/single_char_idents.rs b/tests/ui/min_ident_chars.rs similarity index 86% rename from tests/ui/single_char_idents.rs rename to tests/ui/min_ident_chars.rs index 386633161fe..134c17c553f 100644 --- a/tests/ui/single_char_idents.rs +++ b/tests/ui/min_ident_chars.rs @@ -1,6 +1,6 @@ //@aux-build:proc_macros.rs #![allow(nonstandard_style, unused)] -#![warn(clippy::single_char_idents)] +#![warn(clippy::min_ident_chars)] extern crate proc_macros; use proc_macros::external; @@ -38,11 +38,11 @@ fn main() { 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; + let jz = 1; + let nz = 1; + let zx = 1; + let yz = 1; + let zz = 1; for j in 0..1000 {} diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr new file mode 100644 index 00000000000..4b887617058 --- /dev/null +++ b/tests/ui/min_ident_chars.stderr @@ -0,0 +1,70 @@ +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:20:6 + | +LL | enum C { + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:21:5 + | +LL | D, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:22:5 + | +LL | E, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:23:5 + | +LL | F, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:31:5 + | +LL | w: u32, + | ^ + +error: this ident consists of a single char + --> $DIR/min_ident_chars.rs:58:4 + | +LL | fn b() {} + | ^ + +error: aborting due to 11 previous errors + diff --git a/tests/ui/single_char_idents.stderr b/tests/ui/single_char_idents.stderr deleted file mode 100644 index ceedab9c0da..00000000000 --- a/tests/ui/single_char_idents.stderr +++ /dev/null @@ -1,100 +0,0 @@ -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:9:8 - | -LL | struct A { - | ^ - | - = note: `-D clippy::single-char-idents` implied by `-D warnings` - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:10:5 - | -LL | a: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:12:5 - | -LL | A: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:13:5 - | -LL | I: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:16:8 - | -LL | struct B(u32); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:20:6 - | -LL | enum C { - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:21:5 - | -LL | D, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:22:5 - | -LL | E, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:23:5 - | -LL | F, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:31:5 - | -LL | w: u32, - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:11 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:14 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:17 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:34:20 - | -LL | struct AA(T, E); - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:38:9 - | -LL | let w = 1; - | ^ - -error: this ident comprises of a single char - --> $DIR/single_char_idents.rs:58:4 - | -LL | fn b() {} - | ^ - -error: aborting due to 16 previous errors -