diff --git a/CHANGELOG.md b/CHANGELOG.md index 0445e59480c..0676752b766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5465,4 +5465,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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index f8073dac330..1035f3e7fb6 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -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) + + diff --git a/clippy_lints/src/error_impl_error.rs b/clippy_lints/src/error_impl_error.rs index 7f361d9ba02..585a0ad04c7 100644 --- a/clippy_lints/src/error_impl_error.rs +++ b/clippy_lints/src/error_impl_error.rs @@ -1,12 +1,11 @@ -use clippy_utils::{ - diagnostics::{span_lint, span_lint_hir_and_then}, - path_res, - ty::implements_trait, -}; -use rustc_hir::{def_id::DefId, Item, ItemKind, Node}; +use clippy_utils::diagnostics::{span_lint, span_lint_hir_and_then}; +use clippy_utils::path_res; +use clippy_utils::ty::implements_trait; +use rustc_hir::def_id::DefId; +use rustc_hir::{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! { @@ -15,8 +14,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 + /// hinders comprehension, as it requires you to memorize every variation of importing `Error` + /// used across a codebase. /// /// ### Example /// ```rust,ignore @@ -32,14 +32,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 => @@ -71,6 +79,5 @@ impl<'tcx> LateLintPass<'tcx> for ErrorImplError { } _ => {}, } - {} } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 76654bfe536..22aef713db6 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -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. diff --git a/tests/ui-toml/error_impl_error/allow_private/clippy.toml b/tests/ui-toml/error_impl_error/allow_private/clippy.toml new file mode 100644 index 00000000000..04202148031 --- /dev/null +++ b/tests/ui-toml/error_impl_error/allow_private/clippy.toml @@ -0,0 +1 @@ +allow-private-error = true diff --git a/tests/ui-toml/error_impl_error/disallow_private/clippy.toml b/tests/ui-toml/error_impl_error/disallow_private/clippy.toml new file mode 100644 index 00000000000..d42da3160a9 --- /dev/null +++ b/tests/ui-toml/error_impl_error/disallow_private/clippy.toml @@ -0,0 +1 @@ +allow-private-error = false diff --git a/tests/ui-toml/error_impl_error/error_impl_error.allow_private.stderr b/tests/ui-toml/error_impl_error/error_impl_error.allow_private.stderr new file mode 100644 index 00000000000..520c3e0a3e6 --- /dev/null +++ b/tests/ui-toml/error_impl_error/error_impl_error.allow_private.stderr @@ -0,0 +1,33 @@ +error: type named `Error` that implements `Error` + --> $DIR/error_impl_error.rs:10:16 + | +LL | pub struct Error; + | ^^^^^ + | +note: `Error` was implemented here + --> $DIR/error_impl_error.rs:18: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:35:15 + | +LL | pub union Error { + | ^^^^^ + | +note: `Error` was implemented here + --> $DIR/error_impl_error.rs:52:5 + | +LL | impl std::error::Error for Error {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type alias named `Error` that implements `Error` + --> $DIR/error_impl_error.rs:56:14 + | +LL | pub type Error = std::fmt::Error; + | ^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/error_impl_error.stderr b/tests/ui-toml/error_impl_error/error_impl_error.disallow_private.stderr similarity index 64% rename from tests/ui/error_impl_error.stderr rename to tests/ui-toml/error_impl_error/error_impl_error.disallow_private.stderr index 64af0594e0a..d29c5ef374f 100644 --- a/tests/ui/error_impl_error.stderr +++ b/tests/ui-toml/error_impl_error/error_impl_error.disallow_private.stderr @@ -1,45 +1,45 @@ error: type named `Error` that implements `Error` - --> $DIR/error_impl_error.rs:7:12 + --> $DIR/error_impl_error.rs:10: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:18: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:23:10 | LL | enum Error {} | ^^^^^ | note: `Error` was implemented here - --> $DIR/error_impl_error.rs:28:5 + --> $DIR/error_impl_error.rs:31: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:35: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:52: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:56:14 | -LL | type Error = std::fmt::Error; - | ^^^^^ +LL | pub type Error = std::fmt::Error; + | ^^^^^ error: aborting due to 4 previous errors diff --git a/tests/ui/error_impl_error.rs b/tests/ui-toml/error_impl_error/error_impl_error.rs similarity index 77% rename from tests/ui/error_impl_error.rs rename to tests/ui-toml/error_impl_error/error_impl_error.rs index d85f9522081..3f14b777ee8 100644 --- a/tests/ui/error_impl_error.rs +++ b/tests/ui-toml/error_impl_error/error_impl_error.rs @@ -1,10 +1,13 @@ +//@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 { @@ -28,8 +31,8 @@ mod b { impl std::error::Error for Error {} } -mod c { - union Error { +pub mod c { + pub union Error { a: u32, b: u32, } @@ -49,8 +52,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 { diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 6ba26e97730..227424f948c 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -6,6 +6,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect allow-mixed-uninlined-format-args allow-one-hash-in-raw-strings allow-print-in-tests + allow-private-error allow-private-module-inception allow-unwrap-in-tests allowed-idents-below-min-chars @@ -75,6 +76,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect allow-mixed-uninlined-format-args allow-one-hash-in-raw-strings allow-print-in-tests + allow-private-error allow-private-module-inception allow-unwrap-in-tests allowed-idents-below-min-chars