feat(multiple_inherent_impl): Add config option to target specific scope (#15843)

Add a config option `inherent-impl-lint-scope` to the lint
`multiple_inherent_impl` to target a different scope according to
people's needs. It can take three values: `module`, `file`, and `crate`
(default).

- `module` is the weakest option. It lints if there are two or more
impls in the same module.
- `file` is a bit stronger, since it lints if there are two or more
impls in the same file. So, this triggers the lint (where it did not
with module):
- `crate` is the strongest of them; it triggers as soon as there are two
or more impls anywhere in the crate. It is the current behaviour of the
lint, so it's the default option.

changelog: [`multiple_inherent_impl`] : Add config option (`module`,
`file` or `crate`) to target specific scope

fixes #14867

Author: samueltardieu

CHANGELOG.md

@@ -7035,6 +7035,7 @@ Released 2018-09-13
 [`excessive-nesting-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#excessive-nesting-threshold
 [`future-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#future-size-threshold
 [`ignore-interior-mutability`]: https://doc.rust-lang.org/clippy/lint_configuration.html#ignore-interior-mutability
+[`inherent-impl-lint-scope`]: https://doc.rust-lang.org/clippy/lint_configuration.html#inherent-impl-lint-scope
 [`large-error-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-error-threshold
 [`lint-commented-code`]: https://doc.rust-lang.org/clippy/lint_configuration.html#lint-commented-code
 [`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold

book/src/lint_configuration.md

@@ -671,6 +671,16 @@ A list of paths to types that should be treated as if they do not contain interi
 * [`mutable_key_type`](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)
 
 
+## `inherent-impl-lint-scope`
+Sets the scope ("crate", "file", or "module") in which duplicate inherent `impl` blocks for the same type are linted.
+
+**Default Value:** `"crate"`
+
+---
+**Affected lints:**
+* [`multiple_inherent_impl`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl)
+
+
 ## `large-error-threshold`
 The maximum size of the `Err`-variant in a `Result` returned from a function
 

clippy_config/src/conf.rs

@@ -1,9 +1,9 @@
 use crate::ClippyConfiguration;
 use crate::types::{
-    DisallowedPath, DisallowedPathWithoutReplacement, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour,
-    Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings,
-    SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
-    SourceItemOrderingWithinModuleItemGroupings,
+    DisallowedPath, DisallowedPathWithoutReplacement, InherentImplLintScope, MacroMatcher, MatchLintBehaviour,
+    PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering, SourceItemOrderingCategory,
+    SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind,
+    SourceItemOrderingTraitAssocItemKinds, SourceItemOrderingWithinModuleItemGroupings,
 };
 use clippy_utils::msrvs::Msrv;
 use itertools::Itertools;
@@ -663,6 +663,9 @@ define_Conf! {
     /// A list of paths to types that should be treated as if they do not contain interior mutability
     #[lints(borrow_interior_mutable_const, declare_interior_mutable_const, ifs_same_cond, mutable_key_type)]
     ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()]),
+    /// Sets the scope ("crate", "file", or "module") in which duplicate inherent `impl` blocks for the same type are linted.
+    #[lints(multiple_inherent_impl)]
+    inherent_impl_lint_scope: InherentImplLintScope = InherentImplLintScope::Crate,
     /// The maximum size of the `Err`-variant in a `Result` returned from a function
     #[lints(result_large_err)]
     large_error_threshold: u64 = 128,

clippy_config/src/types.rs

@@ -698,3 +698,11 @@ pub enum PubUnderscoreFieldsBehaviour {
     PubliclyExported,
     AllPubFields,
 }
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
+#[serde(rename_all = "lowercase")]
+pub enum InherentImplLintScope {
+    Crate,
+    File,
+    Module,
+}

clippy_lints/src/inherent_impl.rs

@@ -1,17 +1,23 @@
+use clippy_config::Conf;
+use clippy_config::types::InherentImplLintScope;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::is_lint_allowed;
+use clippy_utils::fulfill_or_allowed;
 use rustc_data_structures::fx::FxHashMap;
-use rustc_hir::def_id::LocalDefId;
+use rustc_hir::def_id::{LocalDefId, LocalModDefId};
 use rustc_hir::{Item, ItemKind, Node};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::declare_lint_pass;
-use rustc_span::Span;
+use rustc_session::impl_lint_pass;
+use rustc_span::{FileName, Span};
 use std::collections::hash_map::Entry;
 
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for multiple inherent implementations of a struct
     ///
+    /// The config option controls the scope in which multiple inherent `impl` blocks for the same
+    /// struct are linted, allowing values of `module` (only within the same module), `file`
+    /// (within the same file), or `crate` (anywhere in the crate, default).
+    ///
     /// ### Why restrict this?
     /// Splitting the implementation of a type makes the code harder to navigate.
     ///
@@ -41,7 +47,26 @@ declare_clippy_lint! {
     "Multiple inherent impl that could be grouped"
 }
 
-declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
+impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
+
+pub struct MultipleInherentImpl {
+    scope: InherentImplLintScope,
+}
+
+impl MultipleInherentImpl {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            scope: conf.inherent_impl_lint_scope,
+        }
+    }
+}
+
+#[derive(Hash, Eq, PartialEq, Clone)]
+enum Criterion {
+    Module(LocalModDefId),
+    File(FileName),
+    Crate,
+}
 
 impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
     fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
@@ -55,18 +80,27 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
 
         for (&id, impl_ids) in &impls.inherent_impls {
             if impl_ids.len() < 2
-            // Check for `#[allow]` on the type definition
-            || is_lint_allowed(
+            // Check for `#[expect]` or `#[allow]` on the type definition
+            || fulfill_or_allowed(
                 cx,
                 MULTIPLE_INHERENT_IMPL,
-                cx.tcx.local_def_id_to_hir_id(id),
+                [cx.tcx.local_def_id_to_hir_id(id)],
             ) {
                 continue;
             }
 
             for impl_id in impl_ids.iter().map(|id| id.expect_local()) {
                 let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity();
-                match type_map.entry(impl_ty) {
+                let hir_id = cx.tcx.local_def_id_to_hir_id(impl_id);
+                let criterion = match self.scope {
+                    InherentImplLintScope::Module => Criterion::Module(cx.tcx.parent_module(hir_id)),
+                    InherentImplLintScope::File => {
+                        let span = cx.tcx.hir_span(hir_id);
+                        Criterion::File(cx.tcx.sess.source_map().lookup_source_file(span.lo()).name.clone())
+                    },
+                    InherentImplLintScope::Crate => Criterion::Crate,
+                };
+                match type_map.entry((impl_ty, criterion)) {
                     Entry::Vacant(e) => {
                         // Store the id for the first impl block of this type. The span is retrieved lazily.
                         e.insert(IdOrSpan::Id(impl_id));
@@ -97,7 +131,6 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
             // Switching to the next type definition, no need to keep the current entries around.
             type_map.clear();
         }
-
         // `TyCtxt::crate_inherent_impls` doesn't have a defined order. Sort the lint output first.
         lint_spans.sort_by_key(|x| x.0.lo());
         for (span, first_span) in lint_spans {
@@ -125,7 +158,7 @@ fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option<Span> {
     {
         (!span.from_expansion()
             && impl_item.generics.params.is_empty()
-            && !is_lint_allowed(cx, MULTIPLE_INHERENT_IMPL, id))
+            && !fulfill_or_allowed(cx, MULTIPLE_INHERENT_IMPL, [id]))
         .then_some(span)
     } else {
         None

clippy_lints/src/lib.rs

@@ -588,7 +588,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings));
     store.register_late_pass(|_| Box::new(suspicious_trait_impl::SuspiciousImpl));
     store.register_late_pass(|_| Box::new(map_unit_fn::MapUnit));
-    store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl));
+    store.register_late_pass(move |_| Box::new(inherent_impl::MultipleInherentImpl::new(conf)));
     store.register_late_pass(|_| Box::new(neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd));
     store.register_late_pass(move |_| Box::new(unwrap::Unwrap::new(conf)));
     store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));

tests/ui-cargo/multiple_inherent_impl/config_fail/Cargo.stderr

@@ -0,0 +1,7 @@
+error: error reading Clippy's configuration file: unknown variant `FooBar`, expected one of `crate`, `file`, `module`
+ --> $DIR/tests/ui-cargo/multiple_inherent_impl/config_fail/clippy.toml:1:28
+  |
+1 | inherent-impl-lint-scope = "FooBar"
+  |                            ^^^^^^^^
+
+error: could not compile `config_fail` (bin "config_fail") due to 1 previous error

tests/ui-cargo/multiple_inherent_impl/config_fail/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "config_fail"
+version = "0.1.0"
+edition = "2024"
+publish = false
+
+[dependencies]

tests/ui-cargo/multiple_inherent_impl/config_fail/clippy.toml

@@ -0,0 +1 @@
+inherent-impl-lint-scope = "FooBar"

tests/ui-cargo/multiple_inherent_impl/config_fail/src/main.rs

@@ -0,0 +1,3 @@
+#![allow(dead_code)]
+#![deny(clippy::multiple_inherent_impl)]
+fn main() {}