fix doc test failure;
apply review suggestions by @Centri3: use multi suggestion; change output message format; add macro expansion check & tests;
This commit is contained in:
parent
28d5115067
commit
40ec760572
@ -371,6 +371,8 @@
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// struct A(u32);
|
||||
///
|
||||
/// impl From<A> for String {
|
||||
/// fn from(a: A) -> Self {
|
||||
/// a.0.to_string()
|
||||
@ -379,6 +381,8 @@
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// struct A(u32);
|
||||
///
|
||||
/// impl From<A> for String {
|
||||
/// fn from(value: A) -> Self {
|
||||
/// value.0.to_string()
|
||||
|
@ -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<I, T>(default_names: &mut I, current_names: &mut T) -> Vec<RenamedParam>
|
||||
impl RenamedFnArgs {
|
||||
/// Comparing between an iterator of default names and one with current names,
|
||||
/// then collect the ones that got renamed.
|
||||
fn new<I, T>(default_names: &mut I, current_names: &mut T) -> Self
|
||||
where
|
||||
I: Iterator<Item = Ident>,
|
||||
T: Iterator<Item = Ident>,
|
||||
{
|
||||
let mut renamed = vec![];
|
||||
// FIXME: Should we stop if they have different length?
|
||||
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_ignored_or_empty_symbol(current_name) || is_ignored_or_empty_symbol(default_name) {
|
||||
if is_unused_or_empty_symbol(current_name) || is_unused_or_empty_symbol(default_name) {
|
||||
continue;
|
||||
}
|
||||
if current_name != default_name {
|
||||
renamed.push(RenamedParam {
|
||||
renamed_span: cur_name.span,
|
||||
default_name,
|
||||
});
|
||||
renamed.push((cur_name.span, default_name.to_string()));
|
||||
}
|
||||
}
|
||||
renamed
|
||||
}
|
||||
|
||||
fn is_ignored_or_empty_symbol(symbol: Symbol) -> bool {
|
||||
let s = symbol.as_str();
|
||||
s.is_empty() || s.starts_with('_')
|
||||
Self(renamed)
|
||||
}
|
||||
|
||||
fn impled_item_def_id(cx: &LateContext<'_>, impl_item_id: OwnerId) -> Option<DefId> {
|
||||
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
|
||||
fn multi_span(&self) -> MultiSpan {
|
||||
self.0
|
||||
.iter()
|
||||
.map(|(span, _)| span)
|
||||
.copied()
|
||||
.collect::<Vec<Span>>()
|
||||
.into()
|
||||
}
|
||||
}
|
||||
|
||||
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('_')
|
||||
}
|
||||
|
||||
/// 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<DefId> {
|
||||
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 {
|
||||
|
@ -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 @@ fn to_string(&self) -> String {
|
||||
struct B(u32);
|
||||
impl From<B> 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<H: Hasher>(&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<H: Hasher>(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 @@ fn totally_irrelevant(&self, right: bool) {}
|
||||
fn some_fn(&self, other: impl MyTrait) {}
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum C {
|
||||
A,
|
||||
B(u32),
|
||||
}
|
||||
|
||||
impl std::ops::Add<B> for C {
|
||||
type Output = C;
|
||||
fn add(self, b: B) -> C {
|
||||
//~^ ERROR: renamed function parameter of trait impl
|
||||
C::B(b.0)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<A> 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() {}
|
||||
|
@ -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<H: Hasher>(&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<H: Hasher>(date: &[Self], states: &mut H) {
|
||||
| ^^^^
|
||||
| ^^^^ ^^^^^^
|
||||
|
|
||||
= help: consider changing the name to: 'data'
|
||||
help: consider using the default names
|
||||
|
|
||||
LL | fn hash_slice<H: Hasher>(data: &[Self], state: &mut H) {
|
||||
| ~~~~ ~~~~~
|
||||
|
||||
error: function parameter name was renamed from its trait default
|
||||
--> $DIR/renamed_function_params.rs:55:45
|
||||
error: renamed function parameter of trait impl
|
||||
--> tests/ui/renamed_function_params.rs:79:18
|
||||
|
|
||||
LL | fn hash_slice<H: Hasher>(date: &[Self], states: &mut H) {
|
||||
| ^^^^^^
|
||||
|
|
||||
= 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
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user