diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs
index 387e0c964ccc..9a09dbaed4ad 100644
--- a/clippy_lints/src/functions/mod.rs
+++ b/clippy_lints/src/functions/mod.rs
@@ -371,6 +371,8 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
+ /// struct A(u32);
+ ///
/// impl From for String {
/// fn from(a: A) -> Self {
/// a.0.to_string()
@@ -379,6 +381,8 @@ declare_clippy_lint! {
/// ```
/// Use instead:
/// ```rust
+ /// struct A(u32);
+ ///
/// impl From for String {
/// fn from(value: A) -> Self {
/// value.0.to_string()
diff --git a/clippy_lints/src/functions/renamed_function_params.rs b/clippy_lints/src/functions/renamed_function_params.rs
index 4c0d5aba1122..cea76996f052 100644
--- a/clippy_lints/src/functions/renamed_function_params.rs
+++ b/clippy_lints/src/functions/renamed_function_params.rs
@@ -1,73 +1,93 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_then;
+use rustc_errors::{Applicability, MultiSpan};
use rustc_hir::def_id::DefId;
use rustc_hir::hir_id::OwnerId;
use rustc_hir::{ImplItem, ImplItemKind, ItemKind, Node};
use rustc_lint::LateContext;
-use rustc_span::symbol::{Ident, Symbol};
+use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::Span;
use super::RENAMED_FUNCTION_PARAMS;
pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>) {
- if let ImplItemKind::Fn(_, body_id) = item.kind &&
- let Some(did) = impled_item_def_id(cx, item.owner_id)
+ if !item.span.from_expansion()
+ && let ImplItemKind::Fn(_, body_id) = item.kind
+ && let Some(did) = trait_item_def_id_of_impl(cx, item.owner_id)
{
let mut param_idents_iter = cx.tcx.hir().body_param_names(body_id);
let mut default_param_idents_iter = cx.tcx.fn_arg_names(did).iter().copied();
- let renames = renamed_params(&mut default_param_idents_iter, &mut param_idents_iter);
- // FIXME: Should we use `MultiSpan` to combine output together?
- // But how should we display help message if so.
- for rename in renames {
- span_lint_and_help(
+ let renames = RenamedFnArgs::new(&mut default_param_idents_iter, &mut param_idents_iter);
+ if !renames.0.is_empty() {
+ let multi_span = renames.multi_span();
+ let plural = if renames.0.len() == 1 { "" } else { "s" };
+ span_lint_and_then(
cx,
RENAMED_FUNCTION_PARAMS,
- rename.renamed_span,
- "function parameter name was renamed from its trait default",
- None,
- &format!("consider changing the name to: '{}'", rename.default_name.as_str())
+ multi_span,
+ &format!("renamed function parameter{plural} of trait impl"),
+ |diag| {
+ diag.multipart_suggestion(
+ format!("consider using the default name{plural}"),
+ renames.0,
+ Applicability::Unspecified,
+ );
+ },
);
}
}
}
-struct RenamedParam {
- renamed_span: Span,
- default_name: Symbol,
-}
+struct RenamedFnArgs(Vec<(Span, String)>);
-fn renamed_params(default_names: &mut I, current_names: &mut T) -> Vec
-where
- I: Iterator- ,
- T: Iterator
- ,
-{
- let mut renamed = vec![];
- // FIXME: Should we stop if they have different length?
- while let (Some(def_name), Some(cur_name)) = (default_names.next(), current_names.next()) {
- let current_name = cur_name.name;
- let default_name = def_name.name;
- if is_ignored_or_empty_symbol(current_name) || is_ignored_or_empty_symbol(default_name) {
- continue;
- }
- if current_name != default_name {
- renamed.push(RenamedParam {
- renamed_span: cur_name.span,
- default_name,
- });
+impl RenamedFnArgs {
+ /// Comparing between an iterator of default names and one with current names,
+ /// then collect the ones that got renamed.
+ fn new(default_names: &mut I, current_names: &mut T) -> Self
+ where
+ I: Iterator
- ,
+ T: Iterator
- ,
+ {
+ let mut renamed: Vec<(Span, String)> = vec![];
+
+ debug_assert!(default_names.size_hint() == current_names.size_hint());
+ while let (Some(def_name), Some(cur_name)) = (default_names.next(), current_names.next()) {
+ let current_name = cur_name.name;
+ let default_name = def_name.name;
+ if is_unused_or_empty_symbol(current_name) || is_unused_or_empty_symbol(default_name) {
+ continue;
+ }
+ if current_name != default_name {
+ renamed.push((cur_name.span, default_name.to_string()));
+ }
}
+
+ Self(renamed)
+ }
+
+ fn multi_span(&self) -> MultiSpan {
+ self.0
+ .iter()
+ .map(|(span, _)| span)
+ .copied()
+ .collect::>()
+ .into()
}
- renamed
}
-fn is_ignored_or_empty_symbol(symbol: Symbol) -> bool {
- let s = symbol.as_str();
- s.is_empty() || s.starts_with('_')
+fn is_unused_or_empty_symbol(symbol: Symbol) -> bool {
+ // FIXME: `body_param_names` currently returning empty symbols for `wild` as well,
+ // so we need to check if the symbol is empty first.
+ // Therefore the check of whether it's equal to [`kw::Underscore`] has no use for now,
+ // but it would be nice to keep it here just to be future-proof.
+ symbol.is_empty() || symbol == kw::Underscore || symbol.as_str().starts_with('_')
}
-fn impled_item_def_id(cx: &LateContext<'_>, impl_item_id: OwnerId) -> Option {
- let trait_node = cx.tcx.hir().find_parent(impl_item_id.into())?;
- if let Node::Item(item) = trait_node &&
- let ItemKind::Impl(impl_) = &item.kind
+/// Get the [`trait_item_def_id`](rustc_hir::hir::ImplItemRef::trait_item_def_id) of an impl item.
+fn trait_item_def_id_of_impl(cx: &LateContext<'_>, impl_item_id: OwnerId) -> Option {
+ let trait_node = cx.tcx.parent_hir_node(impl_item_id.into());
+ if let Node::Item(item) = trait_node
+ && let ItemKind::Impl(impl_) = &item.kind
{
impl_.items.iter().find_map(|item| {
if item.id.owner_id == impl_item_id {
diff --git a/tests/ui/renamed_function_params.rs b/tests/ui/renamed_function_params.rs
index ccc11968bcc6..4f06ae706dde 100644
--- a/tests/ui/renamed_function_params.rs
+++ b/tests/ui/renamed_function_params.rs
@@ -1,5 +1,6 @@
+//@no-rustfix
#![warn(clippy::renamed_function_params)]
-#![allow(clippy::partialeq_ne_impl)]
+#![allow(clippy::partialeq_ne_impl, clippy::to_string_trait_impl)]
#![allow(unused)]
use std::hash::{Hash, Hasher};
@@ -19,17 +20,17 @@ impl ToString for A {
struct B(u32);
impl From for String {
fn from(b: B) -> Self {
- //~^ ERROR: function parameter name was renamed from its trait default
+ //~^ ERROR: renamed function parameter of trait impl
b.0.to_string()
}
}
impl PartialEq for B {
fn eq(&self, rhs: &Self) -> bool {
- //~^ ERROR: function parameter name was renamed from its trait default
+ //~^ ERROR: renamed function parameter of trait impl
self.0 == rhs.0
}
fn ne(&self, rhs: &Self) -> bool {
- //~^ ERROR: function parameter name was renamed from its trait default
+ //~^ ERROR: renamed function parameter of trait impl
self.0 != rhs.0
}
}
@@ -38,22 +39,24 @@ trait MyTrait {
fn foo(&self, val: u8);
fn bar(a: u8, b: u8);
fn baz(self, _val: u8);
+ fn quz(&self, _: u8);
}
impl MyTrait for B {
fn foo(&self, i_dont_wanna_use_your_name: u8) {}
- //~^ ERROR: function parameter name was renamed from its trait default
- fn bar(_a: u8, _b: u8) {}
+ //~^ ERROR: renamed function parameter of trait impl
+ fn bar(_a: u8, _: u8) {}
fn baz(self, val: u8) {}
+ fn quz(&self, val: u8) {}
}
impl Hash for B {
fn hash(&self, states: &mut H) {
- //~^ ERROR: function parameter name was renamed from its trait default
+ //~^ ERROR: renamed function parameter of trait impl
self.0.hash(states);
}
fn hash_slice(date: &[Self], states: &mut H) {
- //~^ ERROR: function parameter name was renamed from its trait default
+ //~^ ERROR: renamed function parameters of trait impl
for d in date {
d.hash(states);
}
@@ -65,4 +68,42 @@ impl B {
fn some_fn(&self, other: impl MyTrait) {}
}
+#[derive(Copy, Clone)]
+enum C {
+ A,
+ B(u32),
+}
+
+impl std::ops::Add for C {
+ type Output = C;
+ fn add(self, b: B) -> C {
+ //~^ ERROR: renamed function parameter of trait impl
+ C::B(b.0)
+ }
+}
+
+impl From for C {
+ fn from(_: A) -> C {
+ C::A
+ }
+}
+
+trait CustomTraitA {
+ fn foo(&self, other: u32);
+}
+trait CustomTraitB {
+ fn bar(&self, value: u8);
+}
+
+macro_rules! impl_trait {
+ ($impl_for:ident, $tr:ty, $fn_name:ident, $t:ty) => {
+ impl $tr for $impl_for {
+ fn $fn_name(&self, v: $t) {}
+ }
+ };
+}
+
+impl_trait!(C, CustomTraitA, foo, u32);
+impl_trait!(C, CustomTraitB, bar, u8);
+
fn main() {}
diff --git a/tests/ui/renamed_function_params.stderr b/tests/ui/renamed_function_params.stderr
index e42931a57b6c..7193541edb62 100644
--- a/tests/ui/renamed_function_params.stderr
+++ b/tests/ui/renamed_function_params.stderr
@@ -1,60 +1,52 @@
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:21:13
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:22:13
|
LL | fn from(b: B) -> Self {
- | ^
+ | ^ help: consider using the default name: `value`
|
- = help: consider changing the name to: 'value'
= note: `-D clippy::renamed-function-params` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::renamed_function_params)]`
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:27:18
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:28:18
|
LL | fn eq(&self, rhs: &Self) -> bool {
- | ^^^
- |
- = help: consider changing the name to: 'other'
+ | ^^^ help: consider using the default name: `other`
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:31:18
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:32:18
|
LL | fn ne(&self, rhs: &Self) -> bool {
- | ^^^
- |
- = help: consider changing the name to: 'other'
+ | ^^^ help: consider using the default name: `other`
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:44:19
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:46:19
|
LL | fn foo(&self, i_dont_wanna_use_your_name: u8) {}
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^
- |
- = help: consider changing the name to: 'val'
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the default name: `val`
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:51:31
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:54:31
|
LL | fn hash(&self, states: &mut H) {
- | ^^^^^^
- |
- = help: consider changing the name to: 'state'
+ | ^^^^^^ help: consider using the default name: `state`
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:55:30
+error: renamed function parameters of trait impl
+ --> tests/ui/renamed_function_params.rs:58:30
|
LL | fn hash_slice(date: &[Self], states: &mut H) {
- | ^^^^
+ | ^^^^ ^^^^^^
|
- = help: consider changing the name to: 'data'
-
-error: function parameter name was renamed from its trait default
- --> $DIR/renamed_function_params.rs:55:45
+help: consider using the default names
|
-LL | fn hash_slice(date: &[Self], states: &mut H) {
- | ^^^^^^
+LL | fn hash_slice(data: &[Self], state: &mut H) {
+ | ~~~~ ~~~~~
+
+error: renamed function parameter of trait impl
+ --> tests/ui/renamed_function_params.rs:79:18
|
- = help: consider changing the name to: 'state'
+LL | fn add(self, b: B) -> C {
+ | ^ help: consider using the default name: `rhs`
error: aborting due to 7 previous errors