diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 781282213cc4..8e4222c50bd7 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -40,6 +40,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"]; const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"]; const DEFAULT_ALLOWED_PREFIXES: &[&str] = &["to", "as", "into", "from", "try_into", "try_from"]; +const DEFAULT_IGNORED_TRAITS: &[&str] = &["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"]; /// Conf with parse errors #[derive(Default)] @@ -610,6 +611,22 @@ define_Conf! { /// - Use `".."` as part of the list to indicate that the configured values should be appended to the /// default configuration of Clippy. By default, any configuration will replace the default value (allowed_prefixes: Vec = DEFAULT_ALLOWED_PREFIXES.iter().map(ToString::to_string).collect()), + /// Lint: RENAMED_FUNCTION_PARAMS. + /// + /// List of trait paths to ignore when checking renamed function parameters. + /// + /// #### Example + /// + /// ```toml + /// ignored-traits = [ "std::convert::From" ] + /// ``` + /// + /// #### Noteworthy + /// + /// - By default, the following traits are ignored: `From`, `TryFrom`, `FromStr` + /// - `".."` can be used as part of the list to indicate that the configured values should be appended to the + /// default configuration of Clippy. By default, any configuration will replace the default value. + (ignored_traits: Vec = DEFAULT_IGNORED_TRAITS.iter().map(ToString::to_string).collect()), } /// Search for the configuration file. @@ -671,6 +688,7 @@ fn deserialize(file: &SourceFile) -> TryConf { extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS); extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES); extend_vec_if_indicator_present(&mut conf.conf.allowed_prefixes, DEFAULT_ALLOWED_PREFIXES); + extend_vec_if_indicator_present(&mut conf.conf.ignored_traits, DEFAULT_IGNORED_TRAITS); // TODO: THIS SHOULD BE TESTED, this comment will be gone soon if conf.conf.allowed_idents_below_min_chars.contains("..") { conf.conf diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index 9a09dbaed4ad..e16ae88da287 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -373,9 +373,9 @@ declare_clippy_lint! { /// ```rust /// struct A(u32); /// - /// impl From for String { - /// fn from(a: A) -> Self { - /// a.0.to_string() + /// impl PartialEq for A { + /// fn eq(&self, b: &Self) -> bool { + /// self.0 == b.0 /// } /// } /// ``` @@ -383,9 +383,9 @@ declare_clippy_lint! { /// ```rust /// struct A(u32); /// - /// impl From for String { - /// fn from(value: A) -> Self { - /// value.0.to_string() + /// impl PartialEq for A { + /// fn eq(&self, other: &Self) -> bool { + /// self.0 == other.0 /// } /// } /// ``` @@ -395,13 +395,13 @@ declare_clippy_lint! { "renamed function parameters in trait implementation" } -#[derive(Copy, Clone)] -#[allow(clippy::struct_field_names)] +#[derive(Clone)] pub struct Functions { too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64, avoid_breaking_exported_api: bool, + ignored_traits: Vec, } impl Functions { @@ -410,12 +410,14 @@ impl Functions { too_many_lines_threshold: u64, large_error_threshold: u64, avoid_breaking_exported_api: bool, + ignored_traits: Vec, ) -> Self { Self { too_many_arguments_threshold, too_many_lines_threshold, large_error_threshold, avoid_breaking_exported_api, + ignored_traits, } } } @@ -461,7 +463,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions { must_use::check_impl_item(cx, item); result::check_impl_item(cx, item, self.large_error_threshold); impl_trait_in_params::check_impl_item(cx, item); - renamed_function_params::check_impl_item(cx, item); + renamed_function_params::check_impl_item(cx, item, &self.ignored_traits); } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { diff --git a/clippy_lints/src/functions/renamed_function_params.rs b/clippy_lints/src/functions/renamed_function_params.rs index cea76996f052..61f8be8fe30c 100644 --- a/clippy_lints/src/functions/renamed_function_params.rs +++ b/clippy_lints/src/functions/renamed_function_params.rs @@ -1,18 +1,30 @@ +use clippy_utils::def_path_def_ids; use clippy_utils::diagnostics::span_lint_and_then; use rustc_errors::{Applicability, MultiSpan}; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_hir::hir_id::OwnerId; -use rustc_hir::{ImplItem, ImplItemKind, ItemKind, Node}; +use rustc_hir::{Impl, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Node, TraitRef}; use rustc_lint::LateContext; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; +use std::sync::OnceLock; use super::RENAMED_FUNCTION_PARAMS; -pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>) { +static IGNORED_TRAIT_IDS: OnceLock = OnceLock::new(); + +pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>, ignored_traits: &[String]) { 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 parent_node = cx.tcx.parent_hir_node(item.hir_id()) + && let Node::Item(parent_item) = parent_node + && let ItemKind::Impl(Impl { + items, + of_trait: Some(trait_ref), + .. + }) = &parent_item.kind + && let Some(did) = trait_item_def_id_of_impl(items, item.owner_id) + && !is_from_ignored_trait(cx, trait_ref, ignored_traits) { 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(); @@ -25,7 +37,7 @@ pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>) { cx, RENAMED_FUNCTION_PARAMS, multi_span, - &format!("renamed function parameter{plural} of trait impl"), + format!("renamed function parameter{plural} of trait impl"), |diag| { diag.multipart_suggestion( format!("consider using the default name{plural}"), @@ -83,20 +95,29 @@ fn is_unused_or_empty_symbol(symbol: Symbol) -> bool { 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 { - 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 { - item.trait_item_def_id - } else { - None - } - }) - } else { - None - } +/// Get the [`trait_item_def_id`](ImplItemRef::trait_item_def_id) of a relevant impl item. +fn trait_item_def_id_of_impl(items: &[ImplItemRef], target: OwnerId) -> Option { + items.iter().find_map(|item| { + if item.id.owner_id == target { + item.trait_item_def_id + } else { + None + } + }) +} + +fn is_from_ignored_trait(cx: &LateContext<'_>, of_trait: &TraitRef<'_>, ignored_traits: &[String]) -> bool { + let Some(trait_did) = of_trait.trait_def_id() else { + return false; + }; + let ignored_trait_ids = IGNORED_TRAIT_IDS.get_or_init(|| { + let mut id_set = DefIdSet::new(); + for path in ignored_traits { + let path_segments: Vec<&str> = path.split("::").collect(); + let ids = def_path_def_ids(cx, &path_segments); + id_set.extend(ids); + } + id_set + }); + ignored_trait_ids.contains(&trait_did) } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e2aac58bf979..d95e633b81fc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -595,6 +595,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { ref allowed_duplicate_crates, allow_comparison_to_zero, ref allowed_prefixes, + ref ignored_traits, blacklisted_names: _, cyclomatic_complexity_threshold: _, @@ -777,6 +778,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { too_many_lines_threshold, large_error_threshold, avoid_breaking_exported_api, + ignored_traits.clone(), )) }); store.register_late_pass(move |_| Box::new(doc::Documentation::new(doc_valid_idents, check_private_items))); diff --git a/tests/ui-toml/renamed_function_params/default/clippy.toml b/tests/ui-toml/renamed_function_params/default/clippy.toml new file mode 100644 index 000000000000..1a0d718de592 --- /dev/null +++ b/tests/ui-toml/renamed_function_params/default/clippy.toml @@ -0,0 +1,2 @@ +# Ignore `From`, `TryFrom`, `FromStr` by default +# ignored-traits = [] diff --git a/tests/ui-toml/renamed_function_params/extend/clippy.toml b/tests/ui-toml/renamed_function_params/extend/clippy.toml new file mode 100644 index 000000000000..11bc146983ff --- /dev/null +++ b/tests/ui-toml/renamed_function_params/extend/clippy.toml @@ -0,0 +1,2 @@ +# Ignore `From`, `TryFrom`, `FromStr` by default +ignored-traits = [ "..", "std::ops::Add", "renamed_function_params::MyTrait" ] diff --git a/tests/ui/renamed_function_params.stderr b/tests/ui-toml/renamed_function_params/renamed_function_params.default.stderr similarity index 69% rename from tests/ui/renamed_function_params.stderr rename to tests/ui-toml/renamed_function_params/renamed_function_params.default.stderr index 7193541edb62..2d700f607592 100644 --- a/tests/ui/renamed_function_params.stderr +++ b/tests/ui-toml/renamed_function_params/renamed_function_params.default.stderr @@ -1,38 +1,32 @@ error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:22:13 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:30:18 | -LL | fn from(b: B) -> Self { - | ^ help: consider using the default name: `value` +LL | fn eq(&self, rhs: &Self) -> bool { + | ^^^ help: consider using the default name: `other` | = note: `-D clippy::renamed-function-params` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::renamed_function_params)]` error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:28:18 - | -LL | fn eq(&self, rhs: &Self) -> bool { - | ^^^ help: consider using the default name: `other` - -error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:32:18 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:34:18 | LL | fn ne(&self, rhs: &Self) -> bool { | ^^^ help: consider using the default name: `other` error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:46:19 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:48:19 | -LL | fn foo(&self, i_dont_wanna_use_your_name: u8) {} +LL | fn foo(&self, i_dont_wanna_use_your_name: u8) {} // only lint in `extend` | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the default name: `val` error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:54:31 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:55:31 | LL | fn hash(&self, states: &mut H) { | ^^^^^^ help: consider using the default name: `state` error: renamed function parameters of trait impl - --> tests/ui/renamed_function_params.rs:58:30 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:59:30 | LL | fn hash_slice(date: &[Self], states: &mut H) { | ^^^^ ^^^^^^ @@ -43,10 +37,10 @@ LL | fn hash_slice(data: &[Self], state: &mut H) { | ~~~~ ~~~~~ error: renamed function parameter of trait impl - --> tests/ui/renamed_function_params.rs:79:18 + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:80:18 | LL | fn add(self, b: B) -> C { | ^ help: consider using the default name: `rhs` -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui-toml/renamed_function_params/renamed_function_params.extend.stderr b/tests/ui-toml/renamed_function_params/renamed_function_params.extend.stderr new file mode 100644 index 000000000000..e57554fa613a --- /dev/null +++ b/tests/ui-toml/renamed_function_params/renamed_function_params.extend.stderr @@ -0,0 +1,34 @@ +error: renamed function parameter of trait impl + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:30:18 + | +LL | fn eq(&self, rhs: &Self) -> bool { + | ^^^ help: consider using the default name: `other` + | + = note: `-D clippy::renamed-function-params` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::renamed_function_params)]` + +error: renamed function parameter of trait impl + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:34:18 + | +LL | fn ne(&self, rhs: &Self) -> bool { + | ^^^ help: consider using the default name: `other` + +error: renamed function parameter of trait impl + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:55:31 + | +LL | fn hash(&self, states: &mut H) { + | ^^^^^^ help: consider using the default name: `state` + +error: renamed function parameters of trait impl + --> tests/ui-toml/renamed_function_params/renamed_function_params.rs:59:30 + | +LL | fn hash_slice(date: &[Self], states: &mut H) { + | ^^^^ ^^^^^^ + | +help: consider using the default names + | +LL | fn hash_slice(data: &[Self], state: &mut H) { + | ~~~~ ~~~~~ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/renamed_function_params.rs b/tests/ui-toml/renamed_function_params/renamed_function_params.rs similarity index 85% rename from tests/ui/renamed_function_params.rs rename to tests/ui-toml/renamed_function_params/renamed_function_params.rs index 4f06ae706dde..f3eb910abbd6 100644 --- a/tests/ui/renamed_function_params.rs +++ b/tests/ui-toml/renamed_function_params/renamed_function_params.rs @@ -1,4 +1,7 @@ //@no-rustfix +//@revisions: default extend +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/renamed_function_params/default +//@[extend] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/renamed_function_params/extend #![warn(clippy::renamed_function_params)] #![allow(clippy::partialeq_ne_impl, clippy::to_string_trait_impl)] #![allow(unused)] @@ -18,9 +21,8 @@ impl ToString for A { } struct B(u32); -impl From for String { +impl std::convert::From for String { fn from(b: B) -> Self { - //~^ ERROR: renamed function parameter of trait impl b.0.to_string() } } @@ -43,8 +45,7 @@ trait MyTrait { } impl MyTrait for B { - fn foo(&self, i_dont_wanna_use_your_name: u8) {} - //~^ ERROR: renamed function parameter of trait impl + fn foo(&self, i_dont_wanna_use_your_name: u8) {} // only lint in `extend` fn bar(_a: u8, _: u8) {} fn baz(self, val: u8) {} fn quz(&self, val: u8) {} @@ -77,7 +78,7 @@ enum C { impl std::ops::Add for C { type Output = C; fn add(self, b: B) -> C { - //~^ ERROR: renamed function parameter of trait impl + // only lint in `extend` C::B(b.0) } } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 24645b61fdb0..53e946bb3bcd 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -41,6 +41,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect excessive-nesting-threshold future-size-threshold ignore-interior-mutability + ignored-traits large-error-threshold literal-representation-threshold matches-for-let-else @@ -121,6 +122,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect excessive-nesting-threshold future-size-threshold ignore-interior-mutability + ignored-traits large-error-threshold literal-representation-threshold matches-for-let-else @@ -201,6 +203,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni excessive-nesting-threshold future-size-threshold ignore-interior-mutability + ignored-traits large-error-threshold literal-representation-threshold matches-for-let-else