Add allow_private_error config option

CHANGELOG.md

@@ -5463,4 +5463,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 -->

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)
+
+

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 {
             }
             _ => {},
         }
-        {}
     }
 }

clippy_lints/src/lib.rs

@@ -1081,7 +1081,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     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(manual_float_methods::ManualFloatMethods));
-    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`
 }
 

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.

tests/ui-toml/error_impl_error/allow_private/clippy.toml

@@ -0,0 +1 @@
+allow-private-error = true

tests/ui-toml/error_impl_error/disallow_private/clippy.toml

@@ -0,0 +1 @@
+allow-private-error = false

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
+

tests/ui/error_impl_error.stderr → 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
 

tests/ui/error_impl_error.rs → 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 {

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