Skip to content

Fix min_ident_chars: ignore on trait impl. #15275

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 2 commits 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
80 changes: 78 additions & 2 deletions clippy_lints/src/min_ident_chars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +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, Item, ItemKind, ItemLocalId, Node, Pat, PatKind, TraitItem, UsePath};
use rustc_hir::{
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! {
Expand Down Expand Up @@ -84,6 +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, ident)
{
emit_min_ident_chars(self, cx, str, ident.span);
}
Expand Down Expand Up @@ -201,3 +208,72 @@ 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 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;
}

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 {
items,
of_trait: Some(_),
..
}) = &parent_item.kind
&& let Some(name) = get_param_name(items, impl_item, cx, ident)
{
return name != ident.name;
}

return true;
}
}

true
}

fn get_param_name(
items: &[ImplItemRef],
impl_item: &ImplItem<'_>,
cx: &LateContext<'_>,
ident: Ident,
) -> Option<Symbol> {
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<DefId> {
items.iter().find_map(|item| {
if item.id.owner_id == target {
item.trait_item_def_id
} else {
None
}
})
}
15 changes: 15 additions & 0 deletions tests/ui/min_ident_chars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,18 @@ fn wrong_pythagoras(a: f32, b: f32) -> f32 {
mod issue_11163 {
struct Array<T, const N: usize>([T; N]);
}

struct Issue13396;

impl core::fmt::Display for Issue13396 {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
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")
}
}
8 changes: 7 additions & 1 deletion tests/ui/min_ident_chars.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Loading