missing_error_impl

Author: SpecificProtagonist

CHANGELOG.md

@@ -5873,6 +5873,7 @@ Released 2018-09-13
 [`missing_const_for_thread_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_thread_local
 [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
 [`missing_enforced_import_renames`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_enforced_import_renames
+[`missing_error_implementations`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_error_implementations
 [`missing_errors_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc
 [`missing_fields_in_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_fields_in_debug
 [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items

clippy_lints/src/declared_lints.rs

@@ -535,6 +535,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::missing_const_for_thread_local::MISSING_CONST_FOR_THREAD_LOCAL_INFO,
     crate::missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS_INFO,
     crate::missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES_INFO,
+    crate::missing_error_implementations::MISSING_ERROR_IMPLEMENTATIONS_INFO,
     crate::missing_fields_in_debug::MISSING_FIELDS_IN_DEBUG_INFO,
     crate::missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS_INFO,
     crate::missing_trait_methods::MISSING_TRAIT_METHODS_INFO,

clippy_lints/src/lib.rs

@@ -243,6 +243,7 @@ mod missing_const_for_fn;
 mod missing_const_for_thread_local;
 mod missing_doc;
 mod missing_enforced_import_rename;
+mod missing_error_implementations;
 mod missing_fields_in_debug;
 mod missing_inline;
 mod missing_trait_methods;
@@ -982,5 +983,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
     store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
+    store.register_late_pass(|_| Box::new(missing_error_implementations::MissingErrorImplementations));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/missing_error_impl.rs

@@ -0,0 +1,71 @@
+use clippy_utils::diagnostics::span_lint;
+use clippy_utils::ty::implements_trait;
+use rustc_hir::{Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Visibility;
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for potentially forgotten implementations of `Error` for public types.
+    ///
+    /// ### Why is this bad?
+    /// `Error` provides a common interface for errors.
+    /// Errors not implementing `Error` can not be used with functions that expect it.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// #[derive(Debug)]
+    /// pub struct ParseError;
+    ///
+    /// impl core::fmt::Display for ParseError { ... }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// #[derive(Debug)]
+    /// pub struct ParseError;
+    ///
+    /// impl core::fmt::Display for ParseError { ... }
+    ///
+    /// impl core::error::Error for ParseError { ... }
+    /// ```
+    #[clippy::version = "1.87.0"]
+    pub MISSING_ERROR_IMPL,
+    suspicious,
+    "exported types with potentially forgotten `Error` implementation"
+}
+
+declare_lint_pass!(MissingErrorImpl => [MISSING_ERROR_IMPL]);
+
+impl<'tcx> LateLintPass<'tcx> for MissingErrorImpl {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        match item.kind {
+            ItemKind::Enum(_, generics) | ItemKind::Struct(_, generics) => {
+                let is_error_candidate = {
+                    let name: &str = item.ident.name.as_str();
+                    name.ends_with("Error") && name != "Error"
+                };
+                if is_error_candidate
+                    // Only check public items, as missing impls for private items are easy to fix.
+                    && (cx.tcx.visibility(item.owner_id.def_id) == Visibility::Public)
+                    // Check whether Error is implemented,
+                    // skipping generic types as we'd have to ask whether there is an error impl
+                    // for any instantiation of it.
+                    && generics.params.is_empty()
+                    && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
+                    && let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error)
+                    && !implements_trait(cx, ty, error_def_id, &[])
+                {
+                    span_lint(
+                        cx,
+                        MISSING_ERROR_IMPL,
+                        item.ident.span,
+                        "error type doesn't implement `Error`",
+                    );
+                }
+            },
+            _ => {},
+        }
+    }
+}

tests/ui/missing_error_impl.rs

@@ -0,0 +1,21 @@
+#![warn(clippy::missing_error_impl)]
+
+struct InternalError;
+
+pub(crate) struct CrateInternalError;
+
+pub struct PublicError;
+//~^ missing_error_impl
+
+pub struct NotAnErrorType;
+
+#[derive(Debug)]
+pub struct GenericError<T>(T);
+
+impl<T> core::fmt::Display for GenericError<T> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        unimplemented!()
+    }
+}
+
+impl<T> core::error::Error for GenericError<T> where T: core::fmt::Display + core::fmt::Debug {}

tests/ui/missing_error_impl.stderr

@@ -0,0 +1,11 @@
+error: error type doesn't implement `Error`
+  --> tests/ui/missing_error_impl.rs:7:12
+   |
+LL | pub struct PublicError;
+   |            ^^^^^^^^^^^
+   |
+   = note: `-D clippy::missing-error-impl` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::missing_error_impl)]`
+
+error: aborting due to 1 previous error
+