Skip to content

Commit

Permalink
Add allow_private_error config option
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jul 7, 2023
1 parent 1adf54d commit 9ffe043
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5424,4 +5424,5 @@ Released 2018-09-13
[`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
[`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
[`allow-private-error`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-private-error
<!-- end autogenerated links to configuration documentation -->
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -730,3 +730,13 @@ Whether to allow `r#""#` when `r""` can be used
* [`unnecessary_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_raw_string_hashes)


## `allow-private-error`
Whether to allow private types named `Error` that implement `Error`

**Default Value:** `true` (`bool`)

---
**Affected lints:**
* [`error_impl_error`](https://rust-lang.github.io/rust-clippy/master/index.html#error_impl_error)


20 changes: 14 additions & 6 deletions clippy_lints/src/error_impl_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use clippy_utils::{
use rustc_hir::{def_id::DefId, Item, ItemKind, Node};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;

declare_clippy_lint! {
Expand All @@ -15,8 +15,9 @@ declare_clippy_lint! {
///
/// ### Why is this bad?
/// It can become confusing when a codebase has 20 types all named `Error`, requiring either
/// aliasing them in the `use` statement them or qualifying them like `my_module::Error`. This
/// severely hinders readability.
/// aliasing them in the `use` statement or qualifying them like `my_module::Error`. This
/// severely hinders comprehension, as it requires you to memorize every variation of importing
/// `Error` used across a codebase.
///
/// ### Example
/// ```rust,ignore
Expand All @@ -32,14 +33,22 @@ declare_clippy_lint! {
restriction,
"types named `Error` that implement `Error`"
}
declare_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);
impl_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);

#[derive(Clone, Copy)]
pub struct ErrorImplError {
pub allow_private_error: bool,
}

impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Self { allow_private_error } = *self;
let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error) else {
return;
};

if allow_private_error && !cx.effective_visibilities.is_exported(item.owner_id.def_id) {
return;
}
match item.kind {
ItemKind::TyAlias(ty, _) if implements_trait(cx, hir_ty_to_ty(cx.tcx, ty), error_def_id, &[])
&& item.ident.name == sym::Error =>
Expand Down Expand Up @@ -71,6 +80,5 @@ impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
}
_ => {},
}
{}
}
}
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
store.register_early_pass(|| Box::new(visibility::Visibility));
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));
store.register_late_pass(|_| Box::new(error_impl_error::ErrorImplError));
let allow_private_error = conf.allow_private_error;
store.register_late_pass(move |_| Box::new(error_impl_error::ErrorImplError { allow_private_error }));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,10 @@ define_Conf! {
///
/// Whether to allow `r#""#` when `r""` can be used
(allow_one_hash_in_raw_strings: bool = false),
/// Lint: ERROR_IMPL_ERROR.
///
/// Whether to allow private types named `Error` that implement `Error`
(allow_private_error: bool = true),
}

/// Search for the configuration file.
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/error_impl_error/allow_private/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-private-error = true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-private-error = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:11:16
|
LL | pub struct Error;
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:19:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `-D clippy::error-impl-error` implied by `-D warnings`

error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:36:15
|
LL | pub union Error {
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:53:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: type alias named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:57:14
|
LL | pub type Error = std::fmt::Error;
| ^^^^^

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -1,45 +1,45 @@
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:7:12
--> $DIR/error_impl_error.rs:11:16
|
LL | struct Error;
| ^^^^^
LL | pub struct Error;
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:15:5
--> $DIR/error_impl_error.rs:19:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `-D clippy::error-impl-error` implied by `-D warnings`

error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:20:10
--> $DIR/error_impl_error.rs:24:10
|
LL | enum Error {}
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:28:5
--> $DIR/error_impl_error.rs:32:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:32:11
--> $DIR/error_impl_error.rs:36:15
|
LL | union Error {
| ^^^^^
LL | pub union Error {
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:49:5
--> $DIR/error_impl_error.rs:53:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: type alias named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:53:10
--> $DIR/error_impl_error.rs:57:14
|
LL | type Error = std::fmt::Error;
| ^^^^^
LL | pub type Error = std::fmt::Error;
| ^^^^^

error: aborting due to 4 previous errors

Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
//@aux-build:../../ui/auxiliary/proc_macros.rs:proc-macro
//@revisions: allow_private disallow_private
//@[allow_private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/error_impl_error/allow_private
//@[disallow_private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/error_impl_error/disallow_private
#![allow(unused)]
#![warn(clippy::error_impl_error)]
#![no_main]

mod a {
pub mod a {
#[derive(Debug)]
struct Error;
pub struct Error;

impl std::fmt::Display for Error {
fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand All @@ -28,8 +32,8 @@ mod b {
impl std::error::Error for Error {}
}

mod c {
union Error {
pub mod c {
pub union Error {
a: u32,
b: u32,
}
Expand All @@ -49,8 +53,8 @@ mod c {
impl std::error::Error for Error {}
}

mod d {
type Error = std::fmt::Error;
pub mod d {
pub type Error = std::fmt::Error;
}

mod e {
Expand Down

0 comments on commit 9ffe043

Please sign in to comment.