Skip to content

Add lint long_variable_names #14818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6109,6 +6109,7 @@ Released 2018-09-13
[`match_str_case_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_str_case_mismatch
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
[`max_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#max_ident_chars
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
[`maybe_misused_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_misused_cfg
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
Expand Down Expand Up @@ -6626,6 +6627,7 @@ Released 2018-09-13
[`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold
[`matches-for-let-else`]: https://doc.rust-lang.org/clippy/lint_configuration.html#matches-for-let-else
[`max-fn-params-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-fn-params-bools
[`max-ident-chars-length`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-ident-chars-length
[`max-include-file-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-include-file-size
[`max-struct-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-struct-bools
[`max-suggested-slice-pattern-length`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-suggested-slice-pattern-length
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,16 @@ The maximum number of bool parameters a function can have
* [`fn_params_excessive_bools`](https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools)


## `max-ident-chars-length`
The maximum length of an identifier

**Default Value:** `100`

---
**Affected lints:**
* [`max_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#max_ident_chars)


## `max-include-file-size`
The maximum size of a file included via `include_bytes!()` or `include_str!()`, in bytes

Expand Down
3 changes: 3 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,9 @@ define_Conf! {
/// The maximum number of bool parameters a function can have
#[lints(fn_params_excessive_bools)]
max_fn_params_bools: u64 = 3,
/// The maximum length of an identifier
#[lints(max_ident_chars)]
max_ident_chars_length: u32 = 100,
/// The maximum size of a file included via `include_bytes!()` or `include_str!()`, in bytes
#[lints(large_include_file)]
max_include_file_size: u64 = 1_000_000,
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::functions::TOO_MANY_ARGUMENTS_INFO,
crate::functions::TOO_MANY_LINES_INFO,
crate::future_not_send::FUTURE_NOT_SEND_INFO,
crate::ident_chars::MAX_IDENT_CHARS_INFO,
crate::ident_chars::MIN_IDENT_CHARS_INFO,
crate::if_let_mutex::IF_LET_MUTEX_INFO,
crate::if_not_else::IF_NOT_ELSE_INFO,
crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO,
Expand Down Expand Up @@ -497,7 +499,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::methods::WAKER_CLONE_WAKE_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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,54 @@ declare_clippy_lint! {
restriction,
"disallows idents that are too short"
}
impl_lint_pass!(MinIdentChars => [MIN_IDENT_CHARS]);

declare_clippy_lint! {
/// ### What it does
/// Checks for identifiers which are too long (higher than the configured threshold).
/// Note: This lint can be very noisy when enabled; it may be desirable to only enable it
/// temporarily.
///
/// ### Why restrict this?
/// To improve readability of code and make it possible to enforce this lint to align a code
/// style on this subject within a team.
///
/// ### Example of long identifiers
/// ```rust,ignore
/// for from_a_long_list_of_movies_which_might_contain_ferris_or_not_you_never_know in movies {
/// let title = from_a_long_list_of_movies_which_might_contain_ferris_or_not_you_never_know.title;
/// }
/// ```
/// Use instead:
/// ```rust,ignore
/// for movie in movies {
/// let title = movie.title;
/// }
/// ```
///
/// ### Limitations
/// Trait implementations which use the same function or parameter name as the trait declaration will
/// not be warned about, even if the name is longer than the configured limit.
#[clippy::version = "1.92.0"]
pub MAX_IDENT_CHARS,
restriction,
"disallows idents which exceeding the max ident length"
}

impl_lint_pass!(MinIdentChars => [MIN_IDENT_CHARS, MAX_IDENT_CHARS]);

#[allow(clippy::struct_field_names)]
pub struct MinIdentChars {
allowed_idents_below_min_chars: FxHashSet<String>,
min_ident_chars_threshold: u64,
max_variable_name_length: u32,
}

impl MinIdentChars {
pub fn new(conf: &'static Conf) -> Self {
Self {
allowed_idents_below_min_chars: conf.allowed_idents_below_min_chars.iter().cloned().collect(),
min_ident_chars_threshold: conf.min_ident_chars_threshold,
max_variable_name_length: conf.max_ident_chars_length,
}
}

Expand All @@ -68,6 +104,10 @@ impl MinIdentChars {
&& !str.is_empty()
&& !self.allowed_idents_below_min_chars.contains(str)
}

fn is_ident_too_long(&self, cx: &LateContext<'_>, str: &str, span: Span) -> bool {
!span.in_external_macro(cx.sess().source_map()) && str.len() > self.max_variable_name_length as usize
}
}

impl LateLintPass<'_> for MinIdentChars {
Expand Down Expand Up @@ -108,6 +148,13 @@ impl LateLintPass<'_> for MinIdentChars {
{
emit_min_ident_chars(self, cx, str, ident.span);
}

if let PatKind::Binding(_, _, ident, ..) = pat.kind
&& let str = ident.as_str()
&& self.is_ident_too_long(cx, str, ident.span)
{
emit_max_ident_chars(self, cx, str, ident.span);
}
}
}

Expand Down Expand Up @@ -199,6 +246,34 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {

emit_min_ident_chars(conf, cx, str, ident.span);
}

if conf.is_ident_too_long(cx, str, ident.span) {
// Check whether the node is part of a `use` statement. We don't want to emit a warning if the user
// has no control over the type.
let usenode = opt_as_use_node(node).or_else(|| {
cx.tcx
.hir_parent_iter(hir_id)
.find_map(|(_, node)| opt_as_use_node(node))
});

// If the name of the identifier is the same as the one of the imported item, this means that we
// found a `use foo::bar`. We can early-return to not emit the warning.
// If however the identifier is different, this means it is an alias (`use foo::bar as baz`). In
// this case, we need to emit the warning for `baz`.
if let Some(imported_item_path) = usenode
// use `present_items` because it could be in any of type_ns, value_ns, macro_ns
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.value_ns
&& cx.tcx.item_name(imported_item_defid).as_str() == str
{
return;
}

if is_from_proc_macro(cx, &ident) {
return;
}

emit_min_ident_chars(conf, cx, str, ident.span);
}
}
}

Expand All @@ -215,6 +290,15 @@ fn emit_min_ident_chars(conf: &MinIdentChars, cx: &impl LintContext, ident: &str
span_lint(cx, MIN_IDENT_CHARS, span, help);
}

fn emit_max_ident_chars(conf: &MinIdentChars, cx: &impl LintContext, ident: &str, span: Span) {
let help = Cow::Owned(format!(
"this ident is too long ({} > {})",
ident.len(),
conf.max_variable_name_length,
));
span_lint(cx, MAX_IDENT_CHARS, span, help);
}

/// Attempt to convert the node to an [`ItemKind::Use`] node.
///
/// If it is, return the [`UsePath`] contained within.
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ mod from_raw_with_void_ptr;
mod from_str_radix_10;
mod functions;
mod future_not_send;
mod ident_chars;
mod if_let_mutex;
mod if_not_else;
mod if_then_some_else_none;
Expand Down Expand Up @@ -232,7 +233,6 @@ mod match_result_ok;
mod matches;
mod mem_replace;
mod methods;
mod min_ident_chars;
mod minmax;
mod misc;
mod misc_early;
Expand Down Expand Up @@ -761,7 +761,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
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));
store.register_late_pass(move |_| Box::new(min_ident_chars::MinIdentChars::new(conf)));
store.register_late_pass(move |_| Box::new(ident_chars::MinIdentChars::new(conf)));
store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(conf)));
store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
store.register_late_pass(move |_| Box::new(needless_pass_by_ref_mut::NeedlessPassByRefMut::new(conf)));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
allowed-idents-below-min-chars = ["Owo", "Uwu", "wha", "t_e", "lse", "_do", "_i_", "put", "her", "_e"]
min-ident-chars-threshold = 3

# override the default length of variables to test the configuration
max-ident-chars-length = 50
15 changes: 15 additions & 0 deletions tests/ui-toml/ident_chars/max_ident_chars.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![warn(clippy::max_ident_chars)]

fn a_function(ferris_singlehandedly_refactored_the_monolith_while_juggling_crates_and_lifetimes: &str) {
//~^ max_ident_chars
}

fn another_function(just_a_short_name: &str) {
// should not cause a problem
}

fn main() {
// `ferris_singlehandedly_refactored_the_monolith_while_juggling_crates_and_lifetimes` is too long
let ferris_singlehandedly_refactored_the_monolith_while_juggling_crates_and_lifetimes = "very long indeed";
//~^ max_ident_chars
}
17 changes: 17 additions & 0 deletions tests/ui-toml/ident_chars/max_ident_chars.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: this ident is too long (81 > 50)
--> tests/ui-toml/ident_chars/max_ident_chars.rs:3:15
|
LL | fn a_function(ferris_singlehandedly_refactored_the_monolith_while_juggling_crates_and_lifetimes: &str) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::max-ident-chars` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::max_ident_chars)]`

error: this ident is too long (81 > 50)
--> tests/ui-toml/ident_chars/max_ident_chars.rs:13:9
|
LL | let ferris_singlehandedly_refactored_the_monolith_while_juggling_crates_and_lifetimes = "very long indeed";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this ident is too short (1 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:6:41
--> tests/ui-toml/ident_chars/min_ident_chars.rs:6:41
|
LL | use extern_types::{Aaa, LONGER, M, N as W};
| ^
Expand All @@ -8,43 +8,43 @@ LL | use extern_types::{Aaa, LONGER, M, N as W};
= help: to override `-D warnings` add `#[allow(clippy::min_ident_chars)]`

error: this ident is too short (1 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:9:11
--> tests/ui-toml/ident_chars/min_ident_chars.rs:9:11
|
LL | pub const N: u32 = 0;
| ^

error: this ident is too short (3 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:15:5
--> tests/ui-toml/ident_chars/min_ident_chars.rs:15:5
|
LL | aaa: Aaa,
| ^^^

error: this ident is too short (3 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:21:9
--> tests/ui-toml/ident_chars/min_ident_chars.rs:21:9
|
LL | let vvv = 1;
| ^^^

error: this ident is too short (3 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:23:9
--> tests/ui-toml/ident_chars/min_ident_chars.rs:23:9
|
LL | let uuu = 1;
| ^^^

error: this ident is too short (1 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:25:14
--> tests/ui-toml/ident_chars/min_ident_chars.rs:25:14
|
LL | let (mut a, mut b) = (1, 2);
| ^

error: this ident is too short (1 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:25:21
--> tests/ui-toml/ident_chars/min_ident_chars.rs:25:21
|
LL | let (mut a, mut b) = (1, 2);
| ^

error: this ident is too short (1 <= 3)
--> tests/ui-toml/min_ident_chars/min_ident_chars.rs:28:9
--> tests/ui-toml/ident_chars/min_ident_chars.rs:28:9
|
LL | for i in 0..1000 {}
| ^
Expand Down
3 changes: 3 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
literal-representation-threshold
matches-for-let-else
max-fn-params-bools
max-ident-chars-length
max-include-file-size
max-struct-bools
max-suggested-slice-pattern-length
Expand Down Expand Up @@ -147,6 +148,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
literal-representation-threshold
matches-for-let-else
max-fn-params-bools
max-ident-chars-length
max-include-file-size
max-struct-bools
max-suggested-slice-pattern-length
Expand Down Expand Up @@ -241,6 +243,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
literal-representation-threshold
matches-for-let-else
max-fn-params-bools
max-ident-chars-length
max-include-file-size
max-struct-bools
max-suggested-slice-pattern-length
Expand Down
File renamed without changes.
Loading