Skip to content

Commit 97de6e0

Browse files
committed
feat(multiple_inherent_impl): Add config option to target specific scope
1 parent 6cc8f8d commit 97de6e0

File tree

28 files changed

+462
-18
lines changed

28 files changed

+462
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6874,6 +6874,7 @@ Released 2018-09-13
68746874
[`excessive-nesting-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#excessive-nesting-threshold
68756875
[`future-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#future-size-threshold
68766876
[`ignore-interior-mutability`]: https://doc.rust-lang.org/clippy/lint_configuration.html#ignore-interior-mutability
6877+
[`inherent-impl-lint-scope`]: https://doc.rust-lang.org/clippy/lint_configuration.html#inherent-impl-lint-scope
68776878
[`large-error-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-error-threshold
68786879
[`lint-commented-code`]: https://doc.rust-lang.org/clippy/lint_configuration.html#lint-commented-code
68796880
[`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold

book/src/lint_configuration.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,16 @@ A list of paths to types that should be treated as if they do not contain interi
671671
* [`mutable_key_type`](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)
672672

673673

674+
## `inherent-impl-lint-scope`
675+
Sets the scope ("crate", "file", or "module") in which duplicate inherent `impl` blocks for the same type are linted.
676+
677+
**Default Value:** `"crate"`
678+
679+
---
680+
**Affected lints:**
681+
* [`multiple_inherent_impl`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl)
682+
683+
674684
## `large-error-threshold`
675685
The maximum size of the `Err`-variant in a `Result` returned from a function
676686

clippy_config/src/conf.rs

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

clippy_config/src/types.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,3 +698,11 @@ pub enum PubUnderscoreFieldsBehaviour {
698698
PubliclyExported,
699699
AllPubFields,
700700
}
701+
702+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
703+
#[serde(rename_all = "lowercase")]
704+
pub enum InherentImplLintScope {
705+
Crate,
706+
File,
707+
Module,
708+
}

clippy_lints/src/inherent_impl.rs

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
1+
use clippy_config::Conf;
2+
use clippy_config::types::InherentImplLintScope;
13
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::is_lint_allowed;
4+
use clippy_utils::fulfill_or_allowed;
35
use rustc_data_structures::fx::FxHashMap;
4-
use rustc_hir::def_id::LocalDefId;
6+
use rustc_hir::def_id::{LocalDefId, LocalModDefId};
57
use rustc_hir::{Item, ItemKind, Node};
68
use rustc_lint::{LateContext, LateLintPass};
7-
use rustc_session::declare_lint_pass;
8-
use rustc_span::Span;
9+
use rustc_session::impl_lint_pass;
10+
use rustc_span::{FileName, Span};
911
use std::collections::hash_map::Entry;
1012

1113
declare_clippy_lint! {
1214
/// ### What it does
1315
/// Checks for multiple inherent implementations of a struct
1416
///
17+
/// The config option controls the scope in which multiple inherent `impl` blocks for the same
18+
/// struct are linted, allowing values of `module` (only within the same module), `file`
19+
/// (within the same file), or `crate` (anywhere in the crate, default).
20+
///
1521
/// ### Why restrict this?
1622
/// Splitting the implementation of a type makes the code harder to navigate.
1723
///
@@ -41,7 +47,26 @@ declare_clippy_lint! {
4147
"Multiple inherent impl that could be grouped"
4248
}
4349

44-
declare_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
50+
impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]);
51+
52+
pub struct MultipleInherentImpl {
53+
scope: InherentImplLintScope,
54+
}
55+
56+
impl MultipleInherentImpl {
57+
pub fn new(conf: &'static Conf) -> Self {
58+
Self {
59+
scope: conf.inherent_impl_lint_scope,
60+
}
61+
}
62+
}
63+
64+
#[derive(Hash, Eq, PartialEq, Clone)]
65+
enum Criterion {
66+
Module(LocalModDefId),
67+
File(FileName),
68+
Crate,
69+
}
4570

4671
impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
4772
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
@@ -56,17 +81,26 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
5681
for (&id, impl_ids) in &impls.inherent_impls {
5782
if impl_ids.len() < 2
5883
// Check for `#[allow]` on the type definition
59-
|| is_lint_allowed(
84+
|| fulfill_or_allowed(
6085
cx,
6186
MULTIPLE_INHERENT_IMPL,
62-
cx.tcx.local_def_id_to_hir_id(id),
87+
[cx.tcx.local_def_id_to_hir_id(id)],
6388
) {
6489
continue;
6590
}
6691

6792
for impl_id in impl_ids.iter().map(|id| id.expect_local()) {
6893
let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity();
69-
match type_map.entry(impl_ty) {
94+
let hir_id = cx.tcx.local_def_id_to_hir_id(impl_id);
95+
let criterion = match self.scope {
96+
InherentImplLintScope::Module => Criterion::Module(cx.tcx.parent_module(hir_id)),
97+
InherentImplLintScope::File => {
98+
let span = cx.tcx.hir_span(hir_id);
99+
Criterion::File(cx.tcx.sess.source_map().lookup_source_file(span.lo()).name.clone())
100+
},
101+
InherentImplLintScope::Crate => Criterion::Crate,
102+
};
103+
match type_map.entry((impl_ty, criterion)) {
70104
Entry::Vacant(e) => {
71105
// Store the id for the first impl block of this type. The span is retrieved lazily.
72106
e.insert(IdOrSpan::Id(impl_id));
@@ -97,7 +131,6 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
97131
// Switching to the next type definition, no need to keep the current entries around.
98132
type_map.clear();
99133
}
100-
101134
// `TyCtxt::crate_inherent_impls` doesn't have a defined order. Sort the lint output first.
102135
lint_spans.sort_by_key(|x| x.0.lo());
103136
for (span, first_span) in lint_spans {
@@ -125,7 +158,7 @@ fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option<Span> {
125158
{
126159
(!span.from_expansion()
127160
&& impl_item.generics.params.is_empty()
128-
&& !is_lint_allowed(cx, MULTIPLE_INHERENT_IMPL, id))
161+
&& !fulfill_or_allowed(cx, MULTIPLE_INHERENT_IMPL, [id]))
129162
.then_some(span)
130163
} else {
131164
None

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
587587
store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings));
588588
store.register_late_pass(|_| Box::new(suspicious_trait_impl::SuspiciousImpl));
589589
store.register_late_pass(|_| Box::new(map_unit_fn::MapUnit));
590-
store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl));
590+
store.register_late_pass(move |_| Box::new(inherent_impl::MultipleInherentImpl::new(conf)));
591591
store.register_late_pass(|_| Box::new(neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd));
592592
store.register_late_pass(move |_| Box::new(unwrap::Unwrap::new(conf)));
593593
store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
error: error reading Clippy's configuration file: unknown variant `FooBar`, expected one of `crate`, `file`, `module`
2+
--> $DIR/tests/ui-cargo/multiple_inherent_impl/config_fail/clippy.toml:1:28
3+
|
4+
1 | inherent-impl-lint-scope = "FooBar"
5+
| ^^^^^^^^
6+
7+
error: could not compile `config_fail` (bin "config_fail") due to 1 previous error
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[package]
2+
name = "config_fail"
3+
version = "0.1.0"
4+
edition = "2024"
5+
publish = false
6+
7+
[dependencies]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
inherent-impl-lint-scope = "FooBar"
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#![allow(dead_code)]
2+
#![deny(clippy::multiple_inherent_impl)]
3+
fn main() {}

0 commit comments

Comments
 (0)