Skip to content

Commit

Permalink
Auto merge of #12285 - Ethiraric:fix-8573, r=dswij
Browse files Browse the repository at this point in the history
Ignore imported items in `min_ident_chars`

Suppress the `min_ident_chars` warning for items whose name we cannot control. Do not warn for `use a::b`, but warn for `use a::b as c`, since `c` is a local identifier.

Fixes #12232

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: [`min_ident_chars`]: Do not warn on non-local identifiers
  • Loading branch information
bors committed Feb 14, 2024
2 parents 03113a9 + c1c2c3e commit 2c3213f
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 15 deletions.
34 changes: 31 additions & 3 deletions clippy_lints/src/min_ident_chars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use clippy_utils::is_from_proc_macro;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_item, Visitor};
use rustc_hir::{GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, Pat, PatKind};
use rustc_hir::{GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, UsePath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::impl_lint_pass;
Expand Down Expand Up @@ -105,11 +105,26 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {

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
// 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
&& let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.first()
&& cx.tcx.item_name(*imported_item_defid).as_str() == str
{
return;
}

// `struct Awa<T>(T)`
// ^
if let Node::PathSegment(path) = node {
Expand Down Expand Up @@ -160,3 +175,16 @@ fn emit_min_ident_chars(conf: &MinIdentChars, cx: &impl LintContext, ident: &str
};
span_lint(cx, MIN_IDENT_CHARS, span, &help);
}

/// Attempt to convert the node to an [`ItemKind::Use`] node.
///
/// If it is, return the [`UsePath`] contained within.
fn opt_as_use_node(node: Node<'_>) -> Option<&'_ UsePath<'_>> {
if let Node::Item(item) = node
&& let ItemKind::Use(path, _) = item.kind
{
Some(path)
} else {
None
}
}
6 changes: 6 additions & 0 deletions tests/ui-toml/min_ident_chars/auxiliary/extern_types.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
#![allow(nonstandard_style, unused)]

pub struct Aaa;
pub struct Bbb;

pub const N: u32 = 3;

pub const M: u32 = 2;
pub const LONGER: u32 = 32;
5 changes: 4 additions & 1 deletion tests/ui-toml/min_ident_chars/min_ident_chars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
#![warn(clippy::min_ident_chars)]

extern crate extern_types;
use extern_types::Aaa;
use extern_types::{Aaa, LONGER, M, N as W};

pub const N: u32 = 0;
pub const LONG: u32 = 32;

struct Owo {
Uwu: u128,
Expand Down
28 changes: 17 additions & 11 deletions tests/ui-toml/min_ident_chars/min_ident_chars.stderr
Original file line number Diff line number Diff line change
@@ -1,47 +1,53 @@
error: this ident is too short (3 <= 3)
--> $DIR/min_ident_chars.rs:6:19
error: this ident is too short (1 <= 3)
--> $DIR/min_ident_chars.rs:6:41
|
LL | use extern_types::Aaa;
| ^^^
LL | use extern_types::{Aaa, LONGER, M, N as W};
| ^
|
= note: `-D clippy::min-ident-chars` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::min_ident_chars)]`

error: this ident is too short (1 <= 3)
--> $DIR/min_ident_chars.rs:8:11
|
LL | pub const N: u32 = 0;
| ^

error: this ident is too short (3 <= 3)
--> $DIR/min_ident_chars.rs:10:5
--> $DIR/min_ident_chars.rs:13:5
|
LL | aaa: Aaa,
| ^^^

error: this ident is too short (3 <= 3)
--> $DIR/min_ident_chars.rs:15:9
--> $DIR/min_ident_chars.rs:18:9
|
LL | let vvv = 1;
| ^^^

error: this ident is too short (3 <= 3)
--> $DIR/min_ident_chars.rs:16:9
--> $DIR/min_ident_chars.rs:19:9
|
LL | let uuu = 1;
| ^^^

error: this ident is too short (1 <= 3)
--> $DIR/min_ident_chars.rs:17:14
--> $DIR/min_ident_chars.rs:20:14
|
LL | let (mut a, mut b) = (1, 2);
| ^

error: this ident is too short (1 <= 3)
--> $DIR/min_ident_chars.rs:17:21
--> $DIR/min_ident_chars.rs:20:21
|
LL | let (mut a, mut b) = (1, 2);
| ^

error: this ident is too short (1 <= 3)
--> $DIR/min_ident_chars.rs:18:9
--> $DIR/min_ident_chars.rs:21:9
|
LL | for i in 0..1000 {}
| ^

error: aborting due to 7 previous errors
error: aborting due to 8 previous errors

0 comments on commit 2c3213f

Please sign in to comment.