diff --git a/CHANGELOG.md b/CHANGELOG.md index b3b6e3b865fc..65d9de76e2f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4819,6 +4819,7 @@ Released 2018-09-13 [`equatable_if_let`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_if_let [`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op [`err_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#err_expect +[`error_impl_error`]: https://rust-lang.github.io/rust-clippy/master/index.html#error_impl_error [`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence [`excessive_nesting`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_nesting [`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 3723de7299bc..a466df597167 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -155,6 +155,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::enum_variants::MODULE_INCEPTION_INFO, crate::enum_variants::MODULE_NAME_REPETITIONS_INFO, crate::equatable_if_let::EQUATABLE_IF_LET_INFO, + crate::error_impl_error::ERROR_IMPL_ERROR_INFO, crate::escape::BOXED_LOCAL_INFO, crate::eta_reduction::REDUNDANT_CLOSURE_INFO, crate::eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS_INFO, diff --git a/clippy_lints/src/error_impl_error.rs b/clippy_lints/src/error_impl_error.rs new file mode 100644 index 000000000000..7f361d9ba022 --- /dev/null +++ b/clippy_lints/src/error_impl_error.rs @@ -0,0 +1,76 @@ +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 rustc_hir_analysis::hir_ty_to_ty; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for types named `Error` that implement `Error`. + /// + /// ### 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. + /// + /// ### Example + /// ```rust,ignore + /// #[derive(Debug)] + /// pub enum Error { ... } + /// + /// impl std::fmt::Display for Error { ... } + /// + /// impl std::error::Error for Error { ... } + /// ``` + #[clippy::version = "1.72.0"] + pub ERROR_IMPL_ERROR, + restriction, + "types named `Error` that implement `Error`" +} +declare_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]); + +impl<'tcx> LateLintPass<'tcx> for ErrorImplError { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error) else { + 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 => + { + span_lint( + cx, + ERROR_IMPL_ERROR, + item.ident.span, + "type alias named `Error` that implements `Error`", + ); + }, + ItemKind::Impl(imp) if let Some(trait_def_id) = imp.of_trait.and_then(|t| t.trait_def_id()) + && error_def_id == trait_def_id + && let Some(def_id) = path_res(cx, imp.self_ty).opt_def_id().and_then(DefId::as_local) + && let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id) + && let Node::Item(ty_item) = cx.tcx.hir().get(hir_id) + && ty_item.ident.name == sym::Error => + { + span_lint_hir_and_then( + cx, + ERROR_IMPL_ERROR, + hir_id, + ty_item.ident.span, + "type named `Error` that implements `Error`", + |diag| { + diag.span_note(item.span, "`Error` was implemented here"); + } + ); + } + _ => {}, + } + {} + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a60da13d053c..9bbc05f0f486 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -120,6 +120,7 @@ mod entry; mod enum_clike; mod enum_variants; mod equatable_if_let; +mod error_impl_error; mod escape; mod eta_reduction; mod excessive_bools; @@ -1080,6 +1081,7 @@ 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)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/error_impl_error.rs b/tests/ui/error_impl_error.rs new file mode 100644 index 000000000000..d85f95220815 --- /dev/null +++ b/tests/ui/error_impl_error.rs @@ -0,0 +1,71 @@ +#![allow(unused)] +#![warn(clippy::error_impl_error)] +#![no_main] + +mod a { + #[derive(Debug)] + struct Error; + + impl std::fmt::Display for Error { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } + } + + impl std::error::Error for Error {} +} + +mod b { + #[derive(Debug)] + enum Error {} + + impl std::fmt::Display for Error { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } + } + + impl std::error::Error for Error {} +} + +mod c { + union Error { + a: u32, + b: u32, + } + + impl std::fmt::Debug for Error { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } + } + + impl std::fmt::Display for Error { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } + } + + impl std::error::Error for Error {} +} + +mod d { + type Error = std::fmt::Error; +} + +mod e { + #[derive(Debug)] + struct MyError; + + impl std::fmt::Display for MyError { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } + } + + impl std::error::Error for MyError {} +} + +mod f { + type MyError = std::fmt::Error; +} diff --git a/tests/ui/error_impl_error.stderr b/tests/ui/error_impl_error.stderr new file mode 100644 index 000000000000..64af0594e0a2 --- /dev/null +++ b/tests/ui/error_impl_error.stderr @@ -0,0 +1,45 @@ +error: type named `Error` that implements `Error` + --> $DIR/error_impl_error.rs:7:12 + | +LL | struct Error; + | ^^^^^ + | +note: `Error` was implemented here + --> $DIR/error_impl_error.rs:15: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 + | +LL | enum Error {} + | ^^^^^ + | +note: `Error` was implemented here + --> $DIR/error_impl_error.rs:28:5 + | +LL | impl std::error::Error for Error {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type named `Error` that implements `Error` + --> $DIR/error_impl_error.rs:32:11 + | +LL | union Error { + | ^^^^^ + | +note: `Error` was implemented here + --> $DIR/error_impl_error.rs:49:5 + | +LL | impl std::error::Error for Error {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type alias named `Error` that implements `Error` + --> $DIR/error_impl_error.rs:53:10 + | +LL | type Error = std::fmt::Error; + | ^^^^^ + +error: aborting due to 4 previous errors +