Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6874,6 +6874,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
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 7 additions & 4 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions clippy_config/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
55 changes: 49 additions & 6 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
@@ -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 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.
///
Expand Down Expand Up @@ -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>) {
Expand All @@ -66,7 +91,26 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {

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 => {
if let Node::Item(&Item {
kind: ItemKind::Impl(_impl_id),
span,
..
}) = cx.tcx.hir_node(hir_id)
{
Criterion::File(cx.tcx.sess.source_map().lookup_source_file(span.lo()).name.clone())
} else {
// We know we are working on an impl, so the pattern matching can
// not fail
unreachable!()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if let Node::Item(&Item {
kind: ItemKind::Impl(_impl_id),
span,
..
}) = cx.tcx.hir_node(hir_id)
{
Criterion::File(cx.tcx.sess.source_map().lookup_source_file(span.lo()).name.clone())
} else {
// We know we are working on an impl, so the pattern matching can
// not fail
unreachable!()
}
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));
Expand Down Expand Up @@ -97,7 +141,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 {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,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)));
Expand Down
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/config_fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "config_fail"
version = "0.1.0"
edition = "2024"
publish = false

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
inherent-impl-lint-scope = "FooBar"
3 changes: 3 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/config_fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#![allow(dead_code)]
#![deny(clippy::multiple_inherent_impl)]
fn main() {}
57 changes: 57 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/crate_fail/Cargo.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
error: multiple implementations of this structure
--> src/main.rs:11:1
|
11 | / impl S {
12 | | //^ Must trigger
13 | | fn second() {}
14 | | }
| |_^
|
note: first implementation here
--> src/main.rs:7:1
|
7 | / impl S {
8 | | fn first() {}
9 | | }
| |_^
note: the lint level is defined here
--> src/main.rs:2:9
|
2 | #![deny(clippy::multiple_inherent_impl)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: multiple implementations of this structure
--> src/main.rs:22:5
|
22 | / impl T {
23 | | //^ Must trigger
24 | | fn second() {}
25 | | }
| |_____^
|
note: first implementation here
--> src/main.rs:16:1
|
16 | / impl T {
17 | | fn first() {}
18 | | }
| |_^

error: multiple implementations of this structure
--> src/main.rs:36:1
|
36 | / impl b::T {
37 | | //^ Must trigger
38 | | fn second() {}
39 | | }
| |_^
|
note: first implementation here
--> src/b.rs:4:1
|
4 | / impl T {
5 | | fn first() {}
6 | | }
| |_^

error: could not compile `crate_fail` (bin "crate_fail") due to 3 previous errors
7 changes: 7 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/crate_fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "crate_fail"
version = "0.1.0"
edition = "2024"
publish = false

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
inherent-impl-lint-scope = "crate"
6 changes: 6 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/crate_fail/src/b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pub struct S;
pub struct T;

impl T {
fn first() {}
}
41 changes: 41 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/crate_fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#![allow(dead_code)]
#![deny(clippy::multiple_inherent_impl)]

struct S;
struct T;

impl S {
fn first() {}
}

impl S {
//^ Must trigger
fn second() {}
}

impl T {
fn first() {}
}

mod a {
use super::T;
impl T {
//^ Must trigger
fn second() {}
}
}

mod b;

impl b::S {
//^ Must NOT trigger
fn first() {}
fn second() {}
}

impl b::T {
//^ Must trigger
fn second() {}
}

fn main() {}
58 changes: 58 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/file_fail/Cargo.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
error: multiple implementations of this structure
--> src/main.rs:13:5
|
13 | / impl S {
14 | | //^ Must trigger
15 | | fn second() {}
16 | | }
| |_____^
|
note: first implementation here
--> src/main.rs:6:1
|
6 | / impl S {
7 | | fn first() {}
8 | | }
| |_^
note: the lint level is defined here
--> src/main.rs:2:9
|
2 | #![deny(clippy::multiple_inherent_impl)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: multiple implementations of this structure
--> src/main.rs:26:5
|
26 | / impl S {
27 | | //^ Must trigger
28 | |
29 | | fn second() {}
30 | | }
| |_____^
|
note: first implementation here
--> src/main.rs:22:5
|
22 | / impl S {
23 | | fn first() {}
24 | | }
| |_____^

error: multiple implementations of this structure
--> src/c.rs:17:5
|
17 | / impl T {
18 | | //^ Must trigger
19 | | fn second() {}
20 | | }
| |_____^
|
note: first implementation here
--> src/c.rs:10:5
|
10 | / impl T {
11 | | fn first() {}
12 | | }
| |_____^

error: could not compile `file_fail` (bin "file_fail") due to 3 previous errors
7 changes: 7 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/file_fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "file_fail"
version = "0.1.0"
edition = "2024"
publish = false

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
inherent-impl-lint-scope = "file"
21 changes: 21 additions & 0 deletions tests/ui-cargo/multiple_inherent_impl/file_fail/src/c.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
pub struct S;
struct T;

impl S {
fn first() {}
}

mod d {
use super::T;
impl T {
fn first() {}
}
}

mod e {
use super::T;
impl T {
//^ Must trigger
fn second() {}
}
}
Loading