From 30832ae16140b553cc1966f0a2c177588c29c8bd Mon Sep 17 00:00:00 2001 From: Tom Webber Date: Mon, 14 Jul 2025 20:24:54 +0200 Subject: [PATCH 1/2] Fix min_ident_chars: ignore impl on trait impl --- clippy_lints/src/min_ident_chars.rs | 29 ++++++++++++++++++++++++++++- tests/ui/min_ident_chars.rs | 8 ++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs index 99f01c8001a5..a0fe59e6104d 100644 --- a/clippy_lints/src/min_ident_chars.rs +++ b/clippy_lints/src/min_ident_chars.rs @@ -4,7 +4,9 @@ use clippy_utils::is_from_proc_macro; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{Visitor, walk_item, walk_trait_item}; -use rustc_hir::{GenericParamKind, HirId, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, TraitItem, UsePath}; +use rustc_hir::{ + GenericParamKind, HirId, Impl, ImplItemKind, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, TraitItem, UsePath, +}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; use rustc_span::Span; @@ -84,6 +86,7 @@ impl LateLintPass<'_> for MinIdentChars { if let PatKind::Binding(_, _, ident, ..) = pat.kind && let str = ident.as_str() && self.is_ident_too_short(cx, str, ident.span) + && is_not_in_trait_impl(cx, pat) { emit_min_ident_chars(self, cx, str, ident.span); } @@ -201,3 +204,27 @@ fn opt_as_use_node(node: Node<'_>) -> Option<&'_ UsePath<'_>> { None } } + +/// Check if a pattern is a function param in an impl block for a trait. +fn is_not_in_trait_impl(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { + let parent_node = cx.tcx.parent_hir_node(pat.hir_id); + if !matches!(parent_node, Node::Param(_)) { + return true; + } + + for (_, parent_node) in cx.tcx.hir_parent_iter(pat.hir_id) { + if let Node::ImplItem(impl_item) = parent_node + && matches!(impl_item.kind, ImplItemKind::Fn(_, _)) + { + let impl_parent_node = cx.tcx.parent_hir_node(impl_item.hir_id()); + if let Node::Item(parent_item) = impl_parent_node + && let ItemKind::Impl(Impl { of_trait: Some(_), .. }) = &parent_item.kind + { + return false; + } + return true; + } + } + + true +} diff --git a/tests/ui/min_ident_chars.rs b/tests/ui/min_ident_chars.rs index f473ac848a8e..bb50d03df460 100644 --- a/tests/ui/min_ident_chars.rs +++ b/tests/ui/min_ident_chars.rs @@ -124,3 +124,11 @@ fn wrong_pythagoras(a: f32, b: f32) -> f32 { mod issue_11163 { struct Array([T; N]); } + +struct Issue13396; + +impl core::fmt::Display for Issue13396 { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "Issue13396") + } +} From baded8ca945f06ee72ddc6b6bd76aa6b11666129 Mon Sep 17 00:00:00 2001 From: Tom Webber Date: Mon, 14 Jul 2025 21:03:31 +0200 Subject: [PATCH 2/2] Fix min_ident_chars: only ignore on trait impl if right param name --- clippy_lints/src/min_ident_chars.rs | 63 +++++++++++++++++++++++++---- tests/ui/min_ident_chars.rs | 7 ++++ tests/ui/min_ident_chars.stderr | 8 +++- 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs index a0fe59e6104d..4099bce08414 100644 --- a/clippy_lints/src/min_ident_chars.rs +++ b/clippy_lints/src/min_ident_chars.rs @@ -3,13 +3,17 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::is_from_proc_macro; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_hir::hir_id::OwnerId; use rustc_hir::intravisit::{Visitor, walk_item, walk_trait_item}; use rustc_hir::{ - GenericParamKind, HirId, Impl, ImplItemKind, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, TraitItem, UsePath, + GenericParamKind, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item, ItemKind, ItemLocalId, Node, Pat, + PatKind, TraitItem, UsePath, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; -use rustc_span::Span; +use rustc_span::symbol::Ident; +use rustc_span::{Span, Symbol}; use std::borrow::Cow; declare_clippy_lint! { @@ -86,7 +90,7 @@ impl LateLintPass<'_> for MinIdentChars { if let PatKind::Binding(_, _, ident, ..) = pat.kind && let str = ident.as_str() && self.is_ident_too_short(cx, str, ident.span) - && is_not_in_trait_impl(cx, pat) + && is_not_in_trait_impl(cx, pat, ident) { emit_min_ident_chars(self, cx, str, ident.span); } @@ -205,8 +209,9 @@ fn opt_as_use_node(node: Node<'_>) -> Option<&'_ UsePath<'_>> { } } -/// Check if a pattern is a function param in an impl block for a trait. -fn is_not_in_trait_impl(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { +/// Check if a pattern is a function param in an impl block for a trait and that the param name is +/// the same than in the trait definition. +fn is_not_in_trait_impl(cx: &LateContext<'_>, pat: &Pat<'_>, ident: Ident) -> bool { let parent_node = cx.tcx.parent_hir_node(pat.hir_id); if !matches!(parent_node, Node::Param(_)) { return true; @@ -218,13 +223,57 @@ fn is_not_in_trait_impl(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { { let impl_parent_node = cx.tcx.parent_hir_node(impl_item.hir_id()); if let Node::Item(parent_item) = impl_parent_node - && let ItemKind::Impl(Impl { of_trait: Some(_), .. }) = &parent_item.kind + && let ItemKind::Impl(Impl { + items, + of_trait: Some(_), + .. + }) = &parent_item.kind + && let Some(name) = get_param_name(items, impl_item, cx, ident) { - return false; + return name != ident.name; } + return true; } } true } + +fn get_param_name( + items: &[ImplItemRef], + impl_item: &ImplItem<'_>, + cx: &LateContext<'_>, + ident: Ident, +) -> Option { + if let Some(trait_item_def_id) = find_trait_item_def_id(items, impl_item.owner_id) { + let trait_param_names = cx.tcx.fn_arg_idents(trait_item_def_id); + + let ImplItemKind::Fn(_, body_id) = impl_item.kind else { + return None; + }; + + if let Some(param_index) = cx + .tcx + .hir_body_param_idents(body_id) + .position(|param_ident| param_ident.is_some_and(|param_ident| param_ident.span == ident.span)) + && let Some(trait_param_name) = trait_param_names.get(param_index) + && let Some(trait_param_ident) = trait_param_name + { + return Some(trait_param_ident.name); + } + } + + None +} + +/// Get the trait item definition ID for an impl item +fn find_trait_item_def_id(items: &[ImplItemRef], target: OwnerId) -> Option { + items.iter().find_map(|item| { + if item.id.owner_id == target { + item.trait_item_def_id + } else { + None + } + }) +} diff --git a/tests/ui/min_ident_chars.rs b/tests/ui/min_ident_chars.rs index bb50d03df460..e656b04be261 100644 --- a/tests/ui/min_ident_chars.rs +++ b/tests/ui/min_ident_chars.rs @@ -132,3 +132,10 @@ impl core::fmt::Display for Issue13396 { write!(f, "Issue13396") } } + +impl core::fmt::Debug for Issue13396 { + fn fmt(&self, g: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + //~^ min_ident_chars + write!(g, "Issue13396") + } +} diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr index bd6c45cf648e..595feae354c6 100644 --- a/tests/ui/min_ident_chars.stderr +++ b/tests/ui/min_ident_chars.stderr @@ -193,5 +193,11 @@ error: this ident consists of a single char LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { | ^ -error: aborting due to 32 previous errors +error: this ident consists of a single char + --> tests/ui/min_ident_chars.rs:137:19 + | +LL | fn fmt(&self, g: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + | ^ + +error: aborting due to 33 previous errors