Auto merge of #61100 - varkor:must_use-tuple-expr, r=cramertj

Apply #[must_use] lint to components of tuples

Fixes https://github.com/rust-lang/rust/issues/61061.

src/librustc_lint/unused.rs

@@ -1,7 +1,7 @@
 use rustc::hir::def::{Res, DefKind};
 use rustc::hir::def_id::DefId;
 use rustc::lint;
-use rustc::ty;
+use rustc::ty::{self, Ty};
 use rustc::ty::adjustment;
 use rustc_data_structures::fx::FxHashMap;
 use lint::{LateContext, EarlyContext, LintContext, LintArray};
@@ -47,43 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
             return;
         }
 
-        let t = cx.tables.expr_ty(&expr);
-        let type_permits_lack_of_use = if t.is_unit()
-            || cx.tcx.is_ty_uninhabited_from(
-                cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), t)
-        {
-            true
-        } else {
-            match t.sty {
-                ty::Adt(def, _) => check_must_use(cx, def.did, s.span, "", ""),
-                ty::Opaque(def, _) => {
-                    let mut must_use = false;
-                    for (predicate, _) in &cx.tcx.predicates_of(def).predicates {
-                        if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
-                            let trait_ref = poly_trait_predicate.skip_binder().trait_ref;
-                            if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ", "") {
-                                must_use = true;
-                                break;
-                            }
-                        }
-                    }
-                    must_use
-                }
-                ty::Dynamic(binder, _) => {
-                    let mut must_use = false;
-                    for predicate in binder.skip_binder().iter() {
-                        if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
-                            if check_must_use(cx, trait_ref.def_id, s.span, "", " trait object") {
-                                must_use = true;
-                                break;
-                            }
-                        }
-                    }
-                    must_use
-                }
-                _ => false,
-            }
-        };
+        let ty = cx.tables.expr_ty(&expr);
+        let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "");
 
         let mut fn_warned = false;
         let mut op_warned = false;
@@ -108,7 +73,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
             _ => None
         };
         if let Some(def_id) = maybe_def_id {
-            fn_warned = check_must_use(cx, def_id, s.span, "return value of ", "");
+            fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "");
         } else if type_permits_lack_of_use {
             // We don't warn about unused unit or uninhabited types.
             // (See https://github.com/rust-lang/rust/issues/43806 for details.)
@@ -162,10 +127,75 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
             cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
         }
 
-        fn check_must_use(
+        // Returns whether an error has been emitted (and thus another does not need to be later).
+        fn check_must_use_ty<'tcx>(
+            cx: &LateContext<'_, 'tcx>,
+            ty: Ty<'tcx>,
+            expr: &hir::Expr,
+            span: Span,
+            descr_post_path: &str,
+        ) -> bool {
+            if ty.is_unit() || cx.tcx.is_ty_uninhabited_from(
+                cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), ty)
+            {
+                return true;
+            }
+
+            match ty.sty {
+                ty::Adt(def, _) => check_must_use_def(cx, def.did, span, "", descr_post_path),
+                ty::Opaque(def, _) => {
+                    let mut has_emitted = false;
+                    for (predicate, _) in &cx.tcx.predicates_of(def).predicates {
+                        if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
+                            let trait_ref = poly_trait_predicate.skip_binder().trait_ref;
+                            let def_id = trait_ref.def_id;
+                            if check_must_use_def(cx, def_id, span, "implementer of ", "") {
+                                has_emitted = true;
+                                break;
+                            }
+                        }
+                    }
+                    has_emitted
+                }
+                ty::Dynamic(binder, _) => {
+                    let mut has_emitted = false;
+                    for predicate in binder.skip_binder().iter() {
+                        if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
+                            let def_id = trait_ref.def_id;
+                            if check_must_use_def(cx, def_id, span, "", " trait object") {
+                                has_emitted = true;
+                                break;
+                            }
+                        }
+                    }
+                    has_emitted
+                }
+                ty::Tuple(ref tys) => {
+                    let mut has_emitted = false;
+                    let spans = if let hir::ExprKind::Tup(comps) = &expr.node {
+                        debug_assert_eq!(comps.len(), tys.len());
+                        comps.iter().map(|e| e.span).collect()
+                    } else {
+                        vec![]
+                    };
+                    for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
+                        let descr_post_path = &format!(" in tuple element {}", i);
+                        let span = *spans.get(i).unwrap_or(&span);
+                        if check_must_use_ty(cx, ty, expr, span, descr_post_path) {
+                            has_emitted = true;
+                        }
+                    }
+                    has_emitted
+                }
+                _ => false,
+            }
+        }
+
+        // Returns whether an error has been emitted (and thus another does not need to be later).
+        fn check_must_use_def(
             cx: &LateContext<'_, '_>,
             def_id: DefId,
-            sp: Span,
+            span: Span,
             descr_pre_path: &str,
             descr_post_path: &str,
         ) -> bool {
@@ -173,7 +203,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
                 if attr.check_name(sym::must_use) {
                     let msg = format!("unused {}`{}`{} that must be used",
                         descr_pre_path, cx.tcx.def_path_str(def_id), descr_post_path);
-                    let mut err = cx.struct_span_lint(UNUSED_MUST_USE, sp, &msg);
+                    let mut err = cx.struct_span_lint(UNUSED_MUST_USE, span, &msg);
                     // check for #[must_use = "..."]
                     if let Some(note) = attr.value_str() {
                         err.note(&note.as_str());

src/test/ui/lint/must_use-tuple.rs

@@ -0,0 +1,17 @@
+#![deny(unused_must_use)]
+
+fn foo() -> (Result<(), ()>, ()) {
+    (Ok::<(), ()>(()), ())
+}
+
+fn main() {
+    (Ok::<(), ()>(()),); //~ ERROR unused `std::result::Result`
+
+    (Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
+    //~^ ERROR unused `std::result::Result`
+    //~^^ ERROR unused `std::result::Result`
+
+    foo(); //~ ERROR unused `std::result::Result`
+
+    ((Err::<(), ()>(()), ()), ()); //~ ERROR unused `std::result::Result`
+}

src/test/ui/lint/must_use-tuple.stderr

@@ -0,0 +1,47 @@
+error: unused `std::result::Result` in tuple element 0 that must be used
+  --> $DIR/must_use-tuple.rs:8:6
+   |
+LL |     (Ok::<(), ()>(()),);
+   |      ^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/must_use-tuple.rs:1:9
+   |
+LL | #![deny(unused_must_use)]
+   |         ^^^^^^^^^^^^^^^
+   = note: this `Result` may be an `Err` variant, which should be handled
+
+error: unused `std::result::Result` in tuple element 0 that must be used
+  --> $DIR/must_use-tuple.rs:10:6
+   |
+LL |     (Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
+   |      ^^^^^^^^^^^^^^^^
+   |
+   = note: this `Result` may be an `Err` variant, which should be handled
+
+error: unused `std::result::Result` in tuple element 2 that must be used
+  --> $DIR/must_use-tuple.rs:10:27
+   |
+LL |     (Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
+   |                           ^^^^^^^^^^^^^^^^
+   |
+   = note: this `Result` may be an `Err` variant, which should be handled
+
+error: unused `std::result::Result` in tuple element 0 that must be used
+  --> $DIR/must_use-tuple.rs:14:5
+   |
+LL |     foo();
+   |     ^^^^^^
+   |
+   = note: this `Result` may be an `Err` variant, which should be handled
+
+error: unused `std::result::Result` in tuple element 0 that must be used
+  --> $DIR/must_use-tuple.rs:16:6
+   |
+LL |     ((Err::<(), ()>(()), ()), ());
+   |      ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `Result` may be an `Err` variant, which should be handled
+
+error: aborting due to 5 previous errors
+