add configuration to allow skipping on some certain traits & collect metadata

Author: J-ZhengLi

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

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)]`
 

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<String> = 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<String> =
+        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

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,19 +374,19 @@ declare_clippy_lint! {
     /// ```rust
     /// struct A(u32);
     ///
-    /// impl From<A> 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
     ///     }
     /// }
     /// ```
     /// Use instead:
     /// ```rust
     /// struct A(u32);
     ///
-    /// impl From<A> 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<String>,
+    /// 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<String>,
     ) -> 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);
+        }
+    }
 }

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<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 {
-                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<DefId> {
+    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)
 }

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)));

tests/ui-toml/renamed_function_params/default/clippy.toml

@@ -0,0 +1,2 @@
+# Ignore `From`, `TryFrom`, `FromStr` by default
+# allow-renamed-params-for = []

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" ]

tests/ui/renamed_function_params.stderr renamed to 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<H: Hasher>(&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<H: Hasher>(date: &[Self], states: &mut H) {
    |                              ^^^^           ^^^^^^
@@ -43,10 +37,10 @@ LL |     fn hash_slice<H: Hasher>(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
 

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<H: Hasher>(&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<H: Hasher>(date: &[Self], states: &mut H) {
+   |                              ^^^^           ^^^^^^
+   |
+help: consider using the default names
+   |
+LL |     fn hash_slice<H: Hasher>(data: &[Self], state: &mut H) {
+   |                              ~~~~           ~~~~~
+
+error: aborting due to 4 previous errors
+