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/functions/must_use.rs

@@ -1,3 +1,4 @@
+use clippy_utils::res::MaybeDef as _;
 use hir::FnSig;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
@@ -222,6 +223,13 @@ fn check_must_use_candidate<'tcx>(
             format!("#[must_use] \n{indent}"),
             Applicability::MachineApplicable,
         );
+        if let Some(msg) = match return_ty(cx, item_id).opt_diag_name(cx) {
+            Some(sym::ControlFlow) => Some("`ControlFlow<B, C>` as `C` when `B` is uninhabited"),
+            Some(sym::Result) => Some("`Result<T, E>` as `T` when `E` is uninhabited"),
+            _ => None,
+        } {
+            diag.note(format!("a future version of Rust will treat {msg} wrt `#[must_use]`"));
+        }
     });
 }
 

clippy_utils/src/ty/mod.rs

@@ -305,10 +305,20 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
     }
 }
 
-// Returns whether the type has #[must_use] attribute
+// Returns whether the `ty` has `#[must_use]` attribute. If `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>) -> 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 args.type_at(1).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
+                is_must_use_ty(cx, args.type_at(0))
+            },
+            Some(sym::ControlFlow) if args.type_at(0).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
+                is_must_use_ty(cx, args.type_at(1))
+            },
+            _ => 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

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
 }

tests/ui/let_underscore_must_use.stderr

@@ -96,5 +96,13 @@ LL |     let _ = a;
    |
    = help: consider explicitly using expression value
 
-error: aborting due to 12 previous errors
+error: non-binding `let` on an expression with `#[must_use]` type
+  --> tests/ui/let_underscore_must_use.rs:120:5
+   |
+LL |     let _ = Result::<_, std::convert::Infallible>::Ok(T);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider explicitly using expression value
+
+error: aborting due to 13 previous errors
 

tests/ui/must_use_candidates.fixed

@@ -107,3 +107,15 @@ pub extern "C" fn unmangled(i: bool) -> bool {
 fn main() {
     assert_eq!(1, pure(1));
 }
+
+//~v must_use_candidate
+#[must_use] 
+pub fn result_uninhabited() -> Result<i32, std::convert::Infallible> {
+    todo!()
+}
+
+//~v must_use_candidate
+#[must_use] 
+pub fn controlflow_uninhabited() -> std::ops::ControlFlow<std::convert::Infallible, i32> {
+    todo!()
+}

tests/ui/must_use_candidates.rs

@@ -102,3 +102,13 @@ pub extern "C" fn unmangled(i: bool) -> bool {
 fn main() {
     assert_eq!(1, pure(1));
 }
+
+//~v must_use_candidate
+pub fn result_uninhabited() -> Result<i32, std::convert::Infallible> {
+    todo!()
+}
+
+//~v must_use_candidate
+pub fn controlflow_uninhabited() -> std::ops::ControlFlow<std::convert::Infallible, i32> {
+    todo!()
+}