New lint: unit_as_impl_trait

Author: samueltardieu

CHANGELOG.md

@@ -6158,6 +6158,7 @@ Released 2018-09-13
 [`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec
 [`uninlined_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
 [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
+[`unit_as_impl_trait`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_as_impl_trait
 [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp
 [`unit_hash`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_hash
 [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord

clippy_lints/src/declared_lints.rs

@@ -757,6 +757,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::unicode::UNICODE_NOT_NFC_INFO,
     crate::uninhabited_references::UNINHABITED_REFERENCES_INFO,
     crate::uninit_vec::UNINIT_VEC_INFO,
+    crate::unit_as_impl_trait::UNIT_AS_IMPL_TRAIT_INFO,
     crate::unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD_INFO,
     crate::unit_types::LET_UNIT_VALUE_INFO,
     crate::unit_types::UNIT_ARG_INFO,

clippy_lints/src/lib.rs

@@ -368,6 +368,7 @@ mod undocumented_unsafe_blocks;
 mod unicode;
 mod uninhabited_references;
 mod uninit_vec;
+mod unit_as_impl_trait;
 mod unit_return_expecting_ord;
 mod unit_types;
 mod unnecessary_box_returns;
@@ -980,5 +981,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(unit_as_impl_trait::UnitAsImplTrait));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/unit_as_impl_trait.rs

@@ -0,0 +1,95 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+use rustc_span::{BytePos, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for function whose return type is an `impl` trait
+    /// implemented by `()`, and whose `()` return value is implicit.
+    ///
+    /// ### Why is this bad?
+    /// Since the required trait is implemented by `()`, adding an extra `;`
+    /// at the end of the function last expression will compile with no
+    /// indication that the expected value may have been silently swallowed.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn key() -> impl Eq {
+    ///     [1, 10, 2, 0].sort_unstable()
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn key() -> impl Eq {
+    ///     let mut arr = [1, 10, 2, 0];
+    ///     arr.sort_unstable();
+    ///     arr
+    /// }
+    /// ```
+    /// or
+    /// ```no_run
+    /// fn key() -> impl Eq {
+    ///     [1, 10, 2, 0].sort_unstable();
+    ///     ()
+    /// }
+    /// ```
+    /// if returning `()` is intentional.
+    #[clippy::version = "1.86.0"]
+    pub UNIT_AS_IMPL_TRAIT,
+    suspicious,
+    "`()` which implements the required trait might be returned unexpectedly"
+}
+
+declare_lint_pass!(UnitAsImplTrait => [UNIT_AS_IMPL_TRAIT]);
+
+impl<'tcx> LateLintPass<'tcx> for UnitAsImplTrait {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'tcx>,
+        decl: &'tcx FnDecl<'tcx>,
+        body: &'tcx Body<'tcx>,
+        span: Span,
+        _: LocalDefId,
+    ) {
+        if !span.from_expansion()
+            && let FnRetTy::Return(ty) = decl.output
+            && let TyKind::OpaqueDef(_) = ty.kind
+            && cx.typeck_results().expr_ty(body.value).is_unit()
+            && let ExprKind::Block(block, None) = body.value.kind
+            && block.expr.is_none_or(|e| !matches!(e.kind, ExprKind::Tup([])))
+        {
+            let block_end_span = block.span.with_hi(block.span.hi() - BytePos(1)).shrink_to_hi();
+
+            span_lint_and_then(
+                cx,
+                UNIT_AS_IMPL_TRAIT,
+                ty.span,
+                "this function returns `()` which implements the required trait",
+                |diag| {
+                    if let Some(expr) = block.expr {
+                        diag.span_note(expr.span, "this expression evaluates to `()`")
+                            .span_help(
+                                expr.span.shrink_to_hi(),
+                                "consider being explicit, and terminate the body with `()`",
+                            );
+                    } else if let Some(last) = block.stmts.last() {
+                        diag.span_note(last.span, "this statement evaluates to `()` because it ends with `;`")
+                            .span_note(block.span, "therefore the function body evaluates to `()`")
+                            .span_help(
+                                block_end_span,
+                                "if this is intentional, consider being explicit, and terminate the body with `()`",
+                            );
+                    } else {
+                        diag.span_note(block.span, "the empty body evaluates to `()`")
+                            .span_help(block_end_span, "consider being explicit and use `()` in the body");
+                    }
+                },
+            );
+        }
+    }
+}

tests/ui/crashes/ice-11422.fixed

@@ -1,4 +1,5 @@
 #![warn(clippy::implied_bounds_in_impls)]
+#![allow(clippy::unit_as_impl_trait)]
 
 use std::fmt::Debug;
 use std::ops::*;

tests/ui/crashes/ice-11422.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::implied_bounds_in_impls)]
+#![allow(clippy::unit_as_impl_trait)]
 
 use std::fmt::Debug;
 use std::ops::*;

tests/ui/crashes/ice-11422.stderr

@@ -1,5 +1,5 @@
 error: this bound is already specified as the supertrait of `PartialOrd`
-  --> tests/ui/crashes/ice-11422.rs:6:33
+  --> tests/ui/crashes/ice-11422.rs:7:33
    |
 LL | fn r#gen() -> impl PartialOrd + PartialEq + Debug {}
    |                                 ^^^^^^^^^

tests/ui/implied_bounds_in_impls.fixed

@@ -1,5 +1,5 @@
 #![warn(clippy::implied_bounds_in_impls)]
-#![allow(dead_code)]
+#![allow(dead_code, clippy::unit_as_impl_trait)]
 #![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
 
 use std::ops::{Deref, DerefMut};

tests/ui/implied_bounds_in_impls.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::implied_bounds_in_impls)]
-#![allow(dead_code)]
+#![allow(dead_code, clippy::unit_as_impl_trait)]
 #![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
 
 use std::ops::{Deref, DerefMut};

tests/ui/new_ret_no_self.rs

@@ -1,6 +1,6 @@
 #![feature(type_alias_impl_trait)]
 #![warn(clippy::new_ret_no_self)]
-#![allow(dead_code)]
+#![allow(dead_code, clippy::unit_as_impl_trait)]
 
 fn main() {}