Handle Result<T, !> and ControlFlow<!, T> as T wrt #[must_use]

There is a proposal to change the behaviour of rustc's `must_use` lint
to consider `Result<T, U>` and `ControlFlow<U, T>` as `T` when `U` is
uninhabited. See <rust-lang/rust#148214>.

This might make the user adding extra `#[must_use]` attributes to
functions returning `Result<T, !>` or `ControlFlow<!, T>`, which would
trigger the `double_must_use` lint in Clippy without the current change.

Author: samueltardieu

clippy_lints/src/drop_forget_ref.rs

@@ -102,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
                 sym::mem_drop
                     if !(arg_ty.needs_drop(cx.tcx, cx.typing_env())
                         || is_must_use_func_call(cx, arg)
-                        || is_must_use_ty(cx, arg_ty)
+                        || is_must_use_ty(cx, arg_ty, true)
                         || drop_is_single_call_in_arm) =>
                 {
                     (DROP_NON_DROP, DROP_NON_DROP_SUMMARY.into(), Some(arg.span))

clippy_lints/src/functions/must_use.rs

@@ -173,12 +173,12 @@ fn check_needless_must_use(
                 "remove `must_use`",
             );
         }
-    } else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
+    } else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id), true) {
         // Ignore async functions unless Future::Output type is a must_use type
         if sig.header.is_async() {
             let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode());
             if let Some(future_ty) = infcx.err_ctxt().get_impl_future_output_ty(return_ty(cx, item_id))
-                && !is_must_use_ty(cx, future_ty)
+                && !is_must_use_ty(cx, future_ty, true)
             {
                 return;
             }
@@ -209,7 +209,9 @@ fn check_must_use_candidate<'tcx>(
         || item_span.in_external_macro(cx.sess().source_map())
         || returns_unit(decl)
         || !cx.effective_visibilities.is_exported(item_id.def_id)
-        || is_must_use_ty(cx, return_ty(cx, item_id))
+        // The `simplify_uninhabited` argument can be later changed to `true` when
+        // rustc's `must_use` lint handles `Result<T, !>` and `ControlFlow<!, T>` as `T`.
+        || is_must_use_ty(cx, return_ty(cx, item_id), false)
         || item_span.from_expansion()
     {
         return;

clippy_lints/src/let_underscore.rs

@@ -170,7 +170,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
                         diag.help("consider awaiting the future or dropping explicitly with `std::mem::drop`");
                     },
                 );
-            } else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
+            } else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init), true) {
                 #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
                 span_lint_and_then(
                     cx,

clippy_lints/src/return_self_not_must_use.rs

@@ -88,7 +88,7 @@ fn check_method(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_def: LocalDefId, spa
         // there is one, we shouldn't emit a warning!
         && self_arg.peel_refs() == ret_ty
         // If `Self` is already marked as `#[must_use]`, no need for the attribute here.
-        && !is_must_use_ty(cx, ret_ty)
+        && !is_must_use_ty(cx, ret_ty, false)
     {
         span_lint_and_help(
             cx,

clippy_utils/src/ty/mod.rs

@@ -305,17 +305,31 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
     }
 }
 
-// Returns whether the type has #[must_use] attribute
-pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+// Returns whether the `ty` has `#[must_use]` attribute. If `simplify_uninhabited` is set and
+// `ty` is a `Result`/`ControlFlow` whose `Err`/`Break` payload is an uninhabited type,
+// the `Ok`/`Continue` payload type will be used instead. See <https://github.com/rust-lang/rust/pull/148214>.
+pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, simplify_uninhabited: bool) -> bool {
     match ty.kind() {
-        ty::Adt(adt, _) => find_attr!(cx.tcx.get_all_attrs(adt.did()), AttributeKind::MustUse { .. }),
+        ty::Adt(adt, args) => match cx.tcx.get_diagnostic_name(adt.did()) {
+            Some(sym::Result)
+                if simplify_uninhabited && args.type_at(1).is_privately_uninhabited(cx.tcx, cx.typing_env()) =>
+            {
+                is_must_use_ty(cx, args.type_at(0), simplify_uninhabited)
+            },
+            Some(sym::ControlFlow)
+                if simplify_uninhabited && args.type_at(0).is_privately_uninhabited(cx.tcx, cx.typing_env()) =>
+            {
+                is_must_use_ty(cx, args.type_at(1), simplify_uninhabited)
+            },
+            _ => find_attr!(cx.tcx.get_all_attrs(adt.did()), AttributeKind::MustUse { .. }),
+        },
         ty::Foreign(did) => find_attr!(cx.tcx.get_all_attrs(*did), AttributeKind::MustUse { .. }),
         ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => {
             // for the Array case we don't need to care for the len == 0 case
             // because we don't want to lint functions returning empty arrays
-            is_must_use_ty(cx, *ty)
+            is_must_use_ty(cx, *ty, simplify_uninhabited)
         },
-        ty::Tuple(args) => args.iter().any(|ty| is_must_use_ty(cx, ty)),
+        ty::Tuple(args) => args.iter().any(|ty| is_must_use_ty(cx, ty, simplify_uninhabited)),
         ty::Alias(ty::Opaque, AliasTy { def_id, .. }) => {
             for (predicate, _) in cx.tcx.explicit_item_self_bounds(def_id).skip_binder() {
                 if let ty::ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()

tests/ui/double_must_use.rs

@@ -1,5 +1,8 @@
 #![warn(clippy::double_must_use)]
 #![allow(clippy::result_unit_err)]
+#![feature(never_type)]
+
+use std::ops::ControlFlow;
 
 #[must_use]
 pub fn must_use_result() -> Result<(), ()> {
@@ -40,6 +43,31 @@ async fn async_must_use_result() -> Result<(), ()> {
     Ok(())
 }
 
+#[must_use]
+pub fn must_use_result_with_uninhabited() -> Result<(), !> {
+    unimplemented!();
+}
+
+#[must_use]
+pub struct T;
+
+#[must_use]
+pub fn must_use_result_with_uninhabited_2() -> Result<T, !> {
+    //~^ double_must_use
+    unimplemented!();
+}
+
+#[must_use]
+pub fn must_use_controlflow_with_uninhabited() -> ControlFlow<std::convert::Infallible> {
+    unimplemented!();
+}
+
+#[must_use]
+pub fn must_use_controlflow_with_uninhabited_2() -> ControlFlow<std::convert::Infallible, T> {
+    //~^ double_must_use
+    unimplemented!();
+}
+
 fn main() {
     must_use_result();
     must_use_tuple();

tests/ui/double_must_use.stderr

@@ -1,5 +1,5 @@
 error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
-  --> tests/ui/double_must_use.rs:5:1
+  --> tests/ui/double_must_use.rs:8:1
    |
 LL | pub fn must_use_result() -> Result<(), ()> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -9,28 +9,44 @@ LL | pub fn must_use_result() -> Result<(), ()> {
    = help: to override `-D warnings` add `#[allow(clippy::double_must_use)]`
 
 error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
-  --> tests/ui/double_must_use.rs:12:1
+  --> tests/ui/double_must_use.rs:15:1
    |
 LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: either add some descriptive message or remove the attribute
 
 error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
-  --> tests/ui/double_must_use.rs:19:1
+  --> tests/ui/double_must_use.rs:22:1
    |
 LL | pub fn must_use_array() -> [Result<(), ()>; 1] {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: either add some descriptive message or remove the attribute
 
 error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
-  --> tests/ui/double_must_use.rs:37:1
+  --> tests/ui/double_must_use.rs:40:1
    |
 LL | async fn async_must_use_result() -> Result<(), ()> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: either add some descriptive message or remove the attribute
 
-error: aborting due to 4 previous errors
+error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
+  --> tests/ui/double_must_use.rs:55:1
+   |
+LL | pub fn must_use_result_with_uninhabited_2() -> Result<T, !> {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: either add some descriptive message or remove the attribute
+
+error: this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`
+  --> tests/ui/double_must_use.rs:66:1
+   |
+LL | pub fn must_use_controlflow_with_uninhabited_2() -> ControlFlow<std::convert::Infallible, T> {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: either add some descriptive message or remove the attribute
+
+error: aborting due to 6 previous errors
 

tests/ui/drop_non_drop.rs

@@ -6,6 +6,11 @@ fn make_result<T>(t: T) -> Result<T, ()> {
     Ok(t)
 }
 
+// The return type should behave as `T` as the `Err` variant is uninhabited
+fn make_result_uninhabited_err<T>(t: T) -> Result<T, std::convert::Infallible> {
+    Ok(t)
+}
+
 #[must_use]
 fn must_use<T>(t: T) -> T {
     t
@@ -41,4 +46,8 @@ fn main() {
 
     // Don't lint
     drop(Baz(Bar));
+
+    // Lint
+    drop(make_result_uninhabited_err(Foo));
+    //~^ drop_non_drop
 }

tests/ui/drop_non_drop.stderr

@@ -1,28 +1,40 @@
 error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes
-  --> tests/ui/drop_non_drop.rs:22:5
+  --> tests/ui/drop_non_drop.rs:27:5
    |
 LL |     drop(Foo);
    |     ^^^^^^^^^
    |
 note: argument has type `main::Foo`
-  --> tests/ui/drop_non_drop.rs:22:10
+  --> tests/ui/drop_non_drop.rs:27:10
    |
 LL |     drop(Foo);
    |          ^^^
    = note: `-D clippy::drop-non-drop` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::drop_non_drop)]`
 
 error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes
-  --> tests/ui/drop_non_drop.rs:39:5
+  --> tests/ui/drop_non_drop.rs:44:5
    |
 LL |     drop(Baz(Foo));
    |     ^^^^^^^^^^^^^^
    |
 note: argument has type `main::Baz<main::Foo>`
-  --> tests/ui/drop_non_drop.rs:39:10
+  --> tests/ui/drop_non_drop.rs:44:10
    |
 LL |     drop(Baz(Foo));
    |          ^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes
+  --> tests/ui/drop_non_drop.rs:51:5
+   |
+LL |     drop(make_result_uninhabited_err(Foo));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: argument has type `std::result::Result<main::Foo, std::convert::Infallible>`
+  --> tests/ui/drop_non_drop.rs:51:10
+   |
+LL |     drop(make_result_uninhabited_err(Foo));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
 

tests/ui/let_underscore_must_use.rs

@@ -109,4 +109,14 @@ fn main() {
 
     #[allow(clippy::let_underscore_must_use)]
     let _ = a;
+
+    // No lint because this type should behave as `()`
+    let _ = Result::<_, std::convert::Infallible>::Ok(());
+
+    #[must_use]
+    struct T;
+
+    // Lint because this type should behave as `T`
+    let _ = Result::<_, std::convert::Infallible>::Ok(T);
+    //~^ let_underscore_must_use
 }