Implement an unused_result lint

I attempted to implement the lint in two steps. My first attempt was a
default-warn lint about *all* unused results. While this attempt did indeed find
many possible bugs, I felt that the false-positive rate was too high to be
turned on by default for all of Rust.

My second attempt was to make unused-result a default-allow lint, but allow
certain types to opt-in to the notion of "you must use this". For example, the
Result type is now flagged with #[must_use]. This lint about "must use" types is
warn by default (it's different from unused-result).

The unused_must_use lint had a 100% hit rate in the compiler, but there's not
that many places that return Result right now. I believe that this lint is a
crucial step towards moving away from conditions for I/O (because all I/O will
return Result by default). I'm worried that this lint is a little too specific
to Result itself, but I believe that the false positive rate for the
unused_result lint is too high to make it useful when turned on by default.

Author: alexcrichton

src/librustc/middle/lint.rs

@@ -105,6 +105,9 @@ pub enum Lint {
     Experimental,
     Unstable,
 
+    UnusedMustUse,
+    UnusedResult,
+
     Warnings,
 }
 
@@ -356,12 +359,26 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
         desc: "unknown features found in crate-level #[feature] directives",
         default: deny,
     }),
-     ("unknown_crate_type",
-     LintSpec {
-         lint: UnknownCrateType,
-         desc: "unknown crate type found in #[crate_type] directive",
-         default: deny,
-     }),
+    ("unknown_crate_type",
+    LintSpec {
+        lint: UnknownCrateType,
+        desc: "unknown crate type found in #[crate_type] directive",
+        default: deny,
+    }),
+
+    ("unused_must_use",
+    LintSpec {
+        lint: UnusedMustUse,
+        desc: "unused result of an type flagged as #[must_use]",
+        default: warn,
+    }),
+
+    ("unused_result",
+    LintSpec {
+        lint: UnusedResult,
+        desc: "unused result of an expression in a statement",
+        default: allow,
+    }),
 ];
 
 /*
@@ -934,7 +951,7 @@ static other_attrs: &'static [&'static str] = &[
     "crate_map", "cfg", "doc", "export_name", "link_section", "no_freeze",
     "no_mangle", "no_send", "static_assert", "unsafe_no_drop_flag", "packed",
     "simd", "repr", "deriving", "unsafe_destructor", "link", "phase",
-    "macro_export",
+    "macro_export", "must_use",
 
     //mod-level
     "path", "link_name", "link_args", "nolink", "macro_escape", "no_implicit_prelude",
@@ -1016,6 +1033,54 @@ fn check_path_statement(cx: &Context, s: &ast::Stmt) {
     }
 }
 
+fn check_unused_result(cx: &Context, s: &ast::Stmt) {
+    let expr = match s.node {
+        ast::StmtSemi(expr, _) => expr,
+        _ => return
+    };
+    let t = ty::expr_ty(cx.tcx, expr);
+    match ty::get(t).sty {
+        ty::ty_nil | ty::ty_bot | ty::ty_bool => return,
+        _ => {}
+    }
+    match expr.node {
+        ast::ExprRet(..) => return,
+        _ => {}
+    }
+
+    let t = ty::expr_ty(cx.tcx, expr);
+    let mut warned = false;
+    match ty::get(t).sty {
+        ty::ty_struct(did, _) |
+        ty::ty_enum(did, _) => {
+            if ast_util::is_local(did) {
+                match cx.tcx.items.get(did.node) {
+                    ast_map::NodeItem(it, _) => {
+                        if attr::contains_name(it.attrs, "must_use") {
+                            cx.span_lint(UnusedMustUse, s.span,
+                                         "unused result which must be used");
+                            warned = true;
+                        }
+                    }
+                    _ => {}
+                }
+            } else {
+                csearch::get_item_attrs(cx.tcx.sess.cstore, did, |attrs| {
+                    if attr::contains_name(attrs, "must_use") {
+                        cx.span_lint(UnusedMustUse, s.span,
+                                     "unused result which must be used");
+                        warned = true;
+                    }
+                });
+            }
+        }
+        _ => {}
+    }
+    if !warned {
+        cx.span_lint(UnusedResult, s.span, "unused result");
+    }
+}
+
 fn check_item_non_camel_case_types(cx: &Context, it: &ast::Item) {
     fn is_camel_case(cx: ty::ctxt, ident: ast::Ident) -> bool {
         let ident = cx.sess.str_of(ident);
@@ -1478,6 +1543,7 @@ impl<'a> Visitor<()> for Context<'a> {
 
     fn visit_stmt(&mut self, s: &ast::Stmt, _: ()) {
         check_path_statement(self, s);
+        check_unused_result(self, s);
 
         visit::walk_stmt(self, s, ());
     }

src/test/compile-fail/unused-result.rs

@@ -0,0 +1,40 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[deny(unused_result, unused_must_use)];
+#[allow(dead_code)];
+
+#[must_use]
+enum MustUse { Test }
+
+fn foo<T>() -> T { fail!() }
+
+fn bar() -> int { return foo::<int>(); }
+fn baz() -> MustUse { return foo::<MustUse>(); }
+
+#[allow(unused_result)]
+fn test() {
+    foo::<int>();
+    foo::<MustUse>(); //~ ERROR: unused result which must be used
+}
+
+#[allow(unused_result, unused_must_use)]
+fn test2() {
+    foo::<int>();
+    foo::<MustUse>();
+}
+
+fn main() {
+    foo::<int>(); //~ ERROR: unused result
+    foo::<MustUse>(); //~ ERROR: unused result which must be used
+
+    let _ = foo::<int>();
+    let _ = foo::<MustUse>();
+}