From 46659acdbde160324bae653e704203221df3b980 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Sun, 12 May 2024 22:13:17 +0800 Subject: [PATCH] add configuration to allow skipping on some certain traits & collect metadata --- CHANGELOG.md | 1 + book/src/lint_configuration.md | 22 ++++++++ clippy_config/src/conf.rs | 23 +++++++++ clippy_lints/src/functions/mod.rs | 35 +++++++++---- .../src/functions/renamed_function_params.rs | 50 +++++++++++-------- clippy_lints/src/lib.rs | 2 + .../default/clippy.toml | 2 + .../extend/clippy.toml | 2 + .../renamed_function_params.default.stderr} | 26 ++++------ .../renamed_function_params.extend.stderr | 34 +++++++++++++ .../renamed_function_params.rs | 11 ++-- .../toml_unknown_key/conf_unknown_key.stderr | 3 ++ 12 files changed, 159 insertions(+), 52 deletions(-) create mode 100644 tests/ui-toml/renamed_function_params/default/clippy.toml create mode 100644 tests/ui-toml/renamed_function_params/extend/clippy.toml rename tests/{ui/renamed_function_params.stderr => ui-toml/renamed_function_params/renamed_function_params.default.stderr} (69%) create mode 100644 tests/ui-toml/renamed_function_params/renamed_function_params.extend.stderr rename tests/{ui => ui-toml/renamed_function_params}/renamed_function_params.rs (85%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce2dda0cc..2abe48afc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5942,6 +5942,7 @@ Released 2018-09-13 [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings [`allow-print-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-print-in-tests [`allow-private-module-inception`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-private-module-inception +[`allow-renamed-params-for`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-renamed-params-for [`allow-unwrap-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-unwrap-in-tests [`allow-useless-vec-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-useless-vec-in-tests [`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index f6af9810c..bd8c6d14c 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -122,6 +122,28 @@ Whether to allow module inception if it's not public. * [`module_inception`](https://rust-lang.github.io/rust-clippy/master/index.html#module_inception) +## `allow-renamed-params-for` +List of trait paths to ignore when checking renamed function parameters. + +#### Example + +```toml +allow-renamed-params-for = [ "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. + +**Default Value:** `["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"]` + +--- +**Affected lints:** +* [`renamed_function_params`](https://rust-lang.github.io/rust-clippy/master/index.html#renamed_function_params) + + ## `allow-unwrap-in-tests` Whether `unwrap` should be allowed in test functions or `#[cfg(test)]` diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 5cfcbdb57..642abd4f3 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -40,6 +40,8 @@ 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_ALLOWED_TRAITS_WITH_RENAMED_PARAMS: &[&str] = + &["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"]; /// Conf with parse errors #[derive(Default)] @@ -613,6 +615,23 @@ 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 + /// allow-renamed-params-for = [ "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. + (allow_renamed_params_for: Vec = + DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS.iter().map(ToString::to_string).collect()), } /// Search for the configuration file. @@ -674,6 +693,10 @@ 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.allow_renamed_params_for, + DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS, + ); // 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 9a09dbaed..dfcaac9ab 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -7,11 +7,12 @@ mod result; mod too_many_arguments; mod too_many_lines; +use clippy_utils::def_path_def_ids; use rustc_hir as hir; use rustc_hir::intravisit; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::def_id::LocalDefId; +use rustc_span::def_id::{DefIdSet, LocalDefId}; use rustc_span::Span; declare_clippy_lint! { @@ -373,9 +374,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 +384,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 +396,16 @@ 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, + allow_renamed_params_for: Vec, + /// A set of resolved `def_id` of traits that are configured to allow + /// function params renaming. + trait_ids: DefIdSet, } impl Functions { @@ -410,12 +414,15 @@ impl Functions { too_many_lines_threshold: u64, large_error_threshold: u64, avoid_breaking_exported_api: bool, + allow_renamed_params_for: Vec, ) -> Self { Self { too_many_arguments_threshold, too_many_lines_threshold, large_error_threshold, avoid_breaking_exported_api, + allow_renamed_params_for, + trait_ids: DefIdSet::default(), } } } @@ -461,7 +468,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.trait_ids); } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { @@ -471,4 +478,12 @@ impl<'tcx> LateLintPass<'tcx> for Functions { result::check_trait_item(cx, item, self.large_error_threshold); impl_trait_in_params::check_trait_item(cx, item, self.avoid_breaking_exported_api); } + + fn check_crate(&mut self, cx: &LateContext<'tcx>) { + for path in &self.allow_renamed_params_for { + let path_segments: Vec<&str> = path.split("::").collect(); + let ids = def_path_def_ids(cx, &path_segments); + self.trait_ids.extend(ids); + } + } } diff --git a/clippy_lints/src/functions/renamed_function_params.rs b/clippy_lints/src/functions/renamed_function_params.rs index cea76996f..c7de0385c 100644 --- a/clippy_lints/src/functions/renamed_function_params.rs +++ b/clippy_lints/src/functions/renamed_function_params.rs @@ -1,18 +1,26 @@ 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 super::RENAMED_FUNCTION_PARAMS; -pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>) { +pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &ImplItem<'_>, ignored_traits: &DefIdSet) { 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(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 +33,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 +91,20 @@ 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(of_trait: &TraitRef<'_>, ignored_traits: &DefIdSet) -> bool { + let Some(trait_did) = of_trait.trait_def_id() else { + return false; + }; + ignored_traits.contains(&trait_did) } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 34dd47855..c9bd40a71 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 allow_renamed_params_for, blacklisted_names: _, cyclomatic_complexity_threshold: _, @@ -788,6 +789,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { too_many_lines_threshold, large_error_threshold, avoid_breaking_exported_api, + allow_renamed_params_for.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 000000000..5381e70a9 --- /dev/null +++ b/tests/ui-toml/renamed_function_params/default/clippy.toml @@ -0,0 +1,2 @@ +# Ignore `From`, `TryFrom`, `FromStr` by default +# allow-renamed-params-for = [] 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 000000000..9b3853e76 --- /dev/null +++ b/tests/ui-toml/renamed_function_params/extend/clippy.toml @@ -0,0 +1,2 @@ +# Ignore `From`, `TryFrom`, `FromStr` by default +allow-renamed-params-for = [ "..", "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 7193541ed..2d700f607 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 000000000..e57554fa6 --- /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 4f06ae706..f3eb910ab 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 722e9b3bc..e1ba60911 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -10,6 +10,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect allow-one-hash-in-raw-strings allow-print-in-tests allow-private-module-inception + allow-renamed-params-for allow-unwrap-in-tests allow-useless-vec-in-tests allowed-dotfiles @@ -91,6 +92,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect allow-one-hash-in-raw-strings allow-print-in-tests allow-private-module-inception + allow-renamed-params-for allow-unwrap-in-tests allow-useless-vec-in-tests allowed-dotfiles @@ -172,6 +174,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni allow-one-hash-in-raw-strings allow-print-in-tests allow-private-module-inception + allow-renamed-params-for allow-unwrap-in-tests allow-useless-vec-in-tests allowed-dotfiles