auto merge of #15561 : huonw/rust/must-use-iterators, r=alexcrichton

Similar to the stability attributes, a type annotated with `#[must_use =
"informative snippet"]` will print the normal warning message along with
"informative snippet". This allows the type author to provide some
guidance about why the type should be used.

---

It can be a little unintuitive that something like `v.iter().map(|x|
println!("{}", x));` does nothing: the majority of the iterator adaptors
are lazy and do not execute anything until something calls `next`, e.g.
a `for` loop, `collect`, `fold`, etc.

The majority of such errors can be seen by someone writing something
like the above, i.e. just calling an iterator adaptor and doing nothing
with it (and doing this is certainly useless), so we can co-opt the
`must_use` lint, using the message functionality to give a hint to the
reason why.

Fixes #14666.

Author: bors

src/libcore/iter.rs

@@ -751,6 +751,7 @@ impl<A, B, T: ExactSize<A>, U: ExactSize<B>> ExactSize<(A, B)> for Zip<T, U> {}
 
 /// An double-ended iterator with the direction inverted
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Rev<T> {
     iter: T
 }
@@ -779,6 +780,7 @@ impl<A, T: DoubleEndedIterator<A> + RandomAccessIterator<A>> RandomAccessIterato
 }
 
 /// A mutable reference to an iterator
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct ByRef<'a, T> {
     iter: &'a mut T
 }
@@ -1039,6 +1041,7 @@ impl<A, T: Clone + Iterator<A>> CloneableIterator for T {
 
 /// An iterator that repeats endlessly
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Cycle<T> {
     orig: T,
     iter: T,
@@ -1090,6 +1093,7 @@ impl<A, T: Clone + RandomAccessIterator<A>> RandomAccessIterator<A> for Cycle<T>
 
 /// An iterator which strings two iterators together
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Chain<T, U> {
     a: T,
     b: U,
@@ -1159,6 +1163,7 @@ for Chain<T, U> {
 
 /// An iterator which iterates two other iterators simultaneously
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Zip<T, U> {
     a: T,
     b: U
@@ -1237,6 +1242,7 @@ RandomAccessIterator<(A, B)> for Zip<T, U> {
 }
 
 /// An iterator which maps the values of `iter` with `f`
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Map<'a, A, B, T> {
     iter: T,
     f: |A|: 'a -> B
@@ -1287,6 +1293,7 @@ impl<'a, A, B, T: RandomAccessIterator<A>> RandomAccessIterator<B> for Map<'a, A
 }
 
 /// An iterator which filters the elements of `iter` with `predicate`
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Filter<'a, A, T> {
     iter: T,
     predicate: |&A|: 'a -> bool
@@ -1331,6 +1338,7 @@ impl<'a, A, T: DoubleEndedIterator<A>> DoubleEndedIterator<A> for Filter<'a, A,
 }
 
 /// An iterator which uses `f` to both filter and map elements from `iter`
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct FilterMap<'a, A, B, T> {
     iter: T,
     f: |A|: 'a -> Option<B>
@@ -1375,6 +1383,7 @@ for FilterMap<'a, A, B, T> {
 
 /// An iterator which yields the current count and the element during iteration
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Enumerate<T> {
     iter: T,
     count: uint
@@ -1429,6 +1438,7 @@ impl<A, T: RandomAccessIterator<A>> RandomAccessIterator<(uint, A)> for Enumerat
 }
 
 /// An iterator with a `peek()` that returns an optional reference to the next element.
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Peekable<A, T> {
     iter: T,
     peeked: Option<A>,
@@ -1479,6 +1489,7 @@ impl<'a, A, T: Iterator<A>> Peekable<A, T> {
 }
 
 /// An iterator which rejects elements while `predicate` is true
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct SkipWhile<'a, A, T> {
     iter: T,
     flag: bool,
@@ -1517,6 +1528,7 @@ impl<'a, A, T: Iterator<A>> Iterator<A> for SkipWhile<'a, A, T> {
 }
 
 /// An iterator which only accepts elements while `predicate` is true
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct TakeWhile<'a, A, T> {
     iter: T,
     flag: bool,
@@ -1552,6 +1564,7 @@ impl<'a, A, T: Iterator<A>> Iterator<A> for TakeWhile<'a, A, T> {
 
 /// An iterator which skips over `n` elements of `iter`.
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Skip<T> {
     iter: T,
     n: uint
@@ -1616,6 +1629,7 @@ impl<A, T: RandomAccessIterator<A>> RandomAccessIterator<A> for Skip<T> {
 
 /// An iterator which only iterates over the first `n` iterations of `iter`.
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Take<T> {
     iter: T,
     n: uint
@@ -1665,6 +1679,7 @@ impl<A, T: RandomAccessIterator<A>> RandomAccessIterator<A> for Take<T> {
 
 
 /// An iterator to maintain state while iterating another iterator
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Scan<'a, A, B, T, St> {
     iter: T,
     f: |&mut St, A|: 'a -> Option<B>,
@@ -1689,6 +1704,7 @@ impl<'a, A, B, T: Iterator<A>, St> Iterator<B> for Scan<'a, A, B, T, St> {
 /// An iterator that maps each element to an iterator,
 /// and yields the elements of the produced iterators
 ///
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct FlatMap<'a, A, T, U> {
     iter: T,
     f: |A|: 'a -> U,
@@ -1748,6 +1764,7 @@ impl<'a,
 /// An iterator that yields `None` forever after the underlying iterator
 /// yields `None` once.
 #[deriving(Clone)]
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Fuse<T> {
     iter: T,
     done: bool
@@ -1820,6 +1837,7 @@ impl<T> Fuse<T> {
 
 /// An iterator that calls a function with a reference to each
 /// element before yielding it.
+#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
 pub struct Inspect<'a, A, T> {
     iter: T,
     f: |&A|: 'a
@@ -2299,4 +2317,3 @@ pub mod order {
         }
     }
 }
-

src/librustc/lint/builtin.rs

@@ -669,22 +669,13 @@ impl LintPass for UnusedResult {
                 if ast_util::is_local(did) {
                     match cx.tcx.map.get(did.node) {
                         ast_map::NodeItem(it) => {
-                            if attr::contains_name(it.attrs.as_slice(),
-                                                   "must_use") {
-                                cx.span_lint(UNUSED_MUST_USE, s.span,
-                                             "unused result which must be used");
-                                warned = true;
-                            }
+                            warned |= check_must_use(cx, it.attrs.as_slice(), s.span);
                         }
                         _ => {}
                     }
                 } else {
                     csearch::get_item_attrs(&cx.sess().cstore, did, |attrs| {
-                        if attr::contains_name(attrs.as_slice(), "must_use") {
-                            cx.span_lint(UNUSED_MUST_USE, s.span,
-                                         "unused result which must be used");
-                            warned = true;
-                        }
+                        warned |= check_must_use(cx, attrs.as_slice(), s.span);
                     });
                 }
             }
@@ -693,6 +684,25 @@ impl LintPass for UnusedResult {
         if !warned {
             cx.span_lint(UNUSED_RESULT, s.span, "unused result");
         }
+
+        fn check_must_use(cx: &Context, attrs: &[ast::Attribute], sp: Span) -> bool {
+            for attr in attrs.iter() {
+                if attr.check_name("must_use") {
+                    let mut msg = "unused result which must be used".to_string();
+                    // check for #[must_use="..."]
+                    match attr.value_str() {
+                        None => {}
+                        Some(s) => {
+                            msg.push_str(": ");
+                            msg.push_str(s.get());
+                        }
+                    }
+                    cx.span_lint(UNUSED_MUST_USE, sp, msg.as_slice());
+                    return true;
+                }
+            }
+            false
+        }
     }
 }
 

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

@@ -14,27 +14,35 @@
 #[must_use]
 enum MustUse { Test }
 
+#[must_use = "some message"]
+enum MustUseMsg { Test2 }
+
 fn foo<T>() -> T { fail!() }
 
 fn bar() -> int { return foo::<int>(); }
 fn baz() -> MustUse { return foo::<MustUse>(); }
+fn qux() -> MustUseMsg { return foo::<MustUseMsg>(); }
 
 #[allow(unused_result)]
 fn test() {
     foo::<int>();
     foo::<MustUse>(); //~ ERROR: unused result which must be used
+    foo::<MustUseMsg>(); //~ ERROR: unused result which must be used: some message
 }
 
 #[allow(unused_result, unused_must_use)]
 fn test2() {
     foo::<int>();
     foo::<MustUse>();
+    foo::<MustUseMsg>();
 }
 
 fn main() {
     foo::<int>(); //~ ERROR: unused result
     foo::<MustUse>(); //~ ERROR: unused result which must be used
+    foo::<MustUseMsg>(); //~ ERROR: unused result which must be used: some message
 
     let _ = foo::<int>();
     let _ = foo::<MustUse>();
+    let _ = foo::<MustUseMsg>();
 }