From 652ac8f07a20b019502bcc43af52a86d6ee8f249 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
Date: Sat, 6 Oct 2018 21:39:32 -0700
Subject: [PATCH] in which we lint `#[must_use]` functions with discardable
 return type

Resolves #54828.
---
 src/librustc_lint/unused.rs                   | 40 ++++++++++++++++
 .../issue-43106-gating-of-builtin-attrs.rs    |  2 +-
 .../issue-54828-unused-must-use-attribute.rs  | 31 ++++++++++++
 ...sue-54828-unused-must-use-attribute.stderr | 48 +++++++++++++++++++
 4 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 src/test/ui/lint/issue-54828-unused-must-use-attribute.rs
 create mode 100644 src/test/ui/lint/issue-54828-unused-must-use-attribute.stderr

diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs
index ae178888b6a1c..dfff21900712e 100644
--- a/src/librustc_lint/unused.rs
+++ b/src/librustc_lint/unused.rs
@@ -252,6 +252,46 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes {
             debug!("Attr was used: {:?}", attr);
         }
     }
+
+    fn check_fn(&mut self,
+                cx: &LateContext<'_, 'tcx>,
+                fk: hir::intravisit::FnKind<'tcx>,
+                decl: &hir::FnDecl,
+                _: &hir::Body,
+                _: Span,
+                _: ast::NodeId) {
+        if let Some(attr) = fk.attrs().iter().find(|attr| attr.check_name("must_use")) {
+            // `#[must_use]` in particular is useless/unused on functions that
+            // return `()` or "return" `!`-likes (Issue #54828)
+            let (useless_must_use, ty_repr) = match &decl.output {
+                hir::FunctionRetTy::DefaultReturn(_) => (true, "()".to_owned()),
+                hir::FunctionRetTy::Return(ty) => {
+                    // The reason we're not sharing code from the UnusedResults
+                    // pass is because here these are `hir::Ty`s, not `ty::Ty`s
+                    let useless = match &ty.node {
+                        hir::TyKind::Tup(tys) => tys.is_empty(),
+                        hir::TyKind::Never => true,
+                        _ => false
+                    };
+                    let repr = hir::print::to_string(hir::print::NO_ANN, |s| s.print_type(&ty));
+                    (useless, repr)
+                }
+            };
+            if useless_must_use {
+                let mut err = cx.struct_span_lint(
+                    UNUSED_ATTRIBUTES, attr.span, "unused attribute"
+                );
+                err.span_label(
+                    decl.output.span(),
+                    format!("the return type `{}` can be discarded", ty_repr)
+                );
+                err.span_suggestion_short_with_applicability(
+                    attr.span, "remove it", String::new(), Applicability::MaybeIncorrect
+                );
+                err.emit();
+            }
+        }
+    }
 }
 
 declare_lint! {
diff --git a/src/test/ui/feature-gate/issue-43106-gating-of-builtin-attrs.rs b/src/test/ui/feature-gate/issue-43106-gating-of-builtin-attrs.rs
index dd235cb9c3aa9..1e904e1a84cf4 100644
--- a/src/test/ui/feature-gate/issue-43106-gating-of-builtin-attrs.rs
+++ b/src/test/ui/feature-gate/issue-43106-gating-of-builtin-attrs.rs
@@ -648,7 +648,7 @@ mod deprecated {
 mod must_use {
     mod inner { #![must_use="1400"] }
 
-    #[must_use = "1400"] fn f() { }
+    #[must_use = "1400"] fn f() -> usize { 1 }
 
     #[must_use = "1400"] struct S;
 
diff --git a/src/test/ui/lint/issue-54828-unused-must-use-attribute.rs b/src/test/ui/lint/issue-54828-unused-must-use-attribute.rs
new file mode 100644
index 0000000000000..2107017573bad
--- /dev/null
+++ b/src/test/ui/lint/issue-54828-unused-must-use-attribute.rs
@@ -0,0 +1,31 @@
+#![allow(dead_code)]
+#![deny(unused_attributes)]
+
+#[must_use]
+fn truth() {}
+
+#[must_use]
+fn pain() -> () {}
+
+#[must_use]
+fn dignity() -> ! { panic!("despair"); }
+
+struct Fear{}
+
+impl Fear {
+    #[must_use] fn bravery() {}
+}
+
+trait Suspicion {
+    #[must_use] fn inspect();
+}
+
+impl Suspicion for Fear {
+    // FIXME: it's actually rather problematic for this to get the unused-attributes
+    // lint on account of the return type—the unused-attributes lint should fire
+    // here, but it should be because `#[must_use]` needs to go on the trait
+    // definition, not the impl (Issue #48486)
+    #[must_use] fn inspect() {}
+}
+
+fn main() {}
diff --git a/src/test/ui/lint/issue-54828-unused-must-use-attribute.stderr b/src/test/ui/lint/issue-54828-unused-must-use-attribute.stderr
new file mode 100644
index 0000000000000..bc2230e05f399
--- /dev/null
+++ b/src/test/ui/lint/issue-54828-unused-must-use-attribute.stderr
@@ -0,0 +1,48 @@
+error: unused attribute
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:4:1
+   |
+LL | #[must_use]
+   | ^^^^^^^^^^^ help: remove it
+LL | fn truth() {}
+   |            - the return type `()` can be discarded
+   |
+note: lint level defined here
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:2:9
+   |
+LL | #![deny(unused_attributes)]
+   |         ^^^^^^^^^^^^^^^^^
+
+error: unused attribute
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:7:1
+   |
+LL | #[must_use]
+   | ^^^^^^^^^^^ help: remove it
+LL | fn pain() -> () {}
+   |              -- the return type `()` can be discarded
+
+error: unused attribute
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:10:1
+   |
+LL | #[must_use]
+   | ^^^^^^^^^^^ help: remove it
+LL | fn dignity() -> ! { panic!("despair"); }
+   |                 - the return type `!` can be discarded
+
+error: unused attribute
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:16:5
+   |
+LL |     #[must_use] fn bravery() {}
+   |     ^^^^^^^^^^^              - the return type `()` can be discarded
+   |     |
+   |     help: remove it
+
+error: unused attribute
+  --> $DIR/issue-54828-unused-must-use-attribute.rs:28:5
+   |
+LL |     #[must_use] fn inspect() {}
+   |     ^^^^^^^^^^^              - the return type `()` can be discarded
+   |     |
+   |     help: remove it
+
+error: aborting due to 5 previous errors
+