Merge pull request #744 from mcarton/rustup

[WIP] Rustup to rustc 1.9.0-nightly (998a6720b 2016-03-07)

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "clippy"
-version = "0.0.46"
+version = "0.0.47"
 authors = [
 	"Manish Goregaokar <manishsmail@gmail.com>",
 	"Andre Bogus <bogusandre@gmail.com>",

src/loops.rs

@@ -10,10 +10,13 @@ use rustc_front::hir::*;
 use rustc_front::intravisit::{Visitor, walk_expr, walk_block, walk_decl};
 use std::borrow::Cow;
 use std::collections::HashMap;
+use syntax::ast;
 
 use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
-            span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, walk_ptrs_ty};
+            span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then,
+            unsugar_range, walk_ptrs_ty};
 use utils::{BTREEMAP_PATH, HASHMAP_PATH, LL_PATH, OPTION_PATH, RESULT_PATH, VEC_PATH};
+use utils::UnsugaredRange;
 
 /// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index.
 ///
@@ -323,10 +326,9 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E
 /// Check for looping over a range and then indexing a sequence with it.
 /// The iteratee must be a range literal.
 fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
-    if let ExprRange(Some(ref l), ref r) = arg.node {
+    if let Some(UnsugaredRange { start: Some(ref start), ref end, .. }) = unsugar_range(&arg) {
         // the var must be a single name
         if let PatKind::Ident(_, ref ident, _) = pat.node {
-
             let mut visitor = VarVisitor {
                 cx: cx,
                 var: ident.node.name,
@@ -348,19 +350,19 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                     return;
                 }
 
-                let starts_at_zero = is_integer_literal(l, 0);
+                let starts_at_zero = is_integer_literal(start, 0);
 
                 let skip: Cow<_> = if starts_at_zero {
                     "".into()
                 } else {
-                    format!(".skip({})", snippet(cx, l.span, "..")).into()
+                    format!(".skip({})", snippet(cx, start.span, "..")).into()
                 };
 
-                let take: Cow<_> = if let Some(ref r) = *r {
+                let take: Cow<_> = if let Some(ref end) = *end {
-                    if is_len_call(&r, &indexed) {
+                    if is_len_call(&end, &indexed) {
                         "".into()
                     } else {
-                        format!(".take({})", snippet(cx, r.span, "..")).into()
+                        format!(".take({})", snippet(cx, end.span, "..")).into()
                     }
                 } else {
                     "".into()
@@ -416,27 +418,27 @@ fn is_len_call(expr: &Expr, var: &Name) -> bool {
 
 fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) {
     // if this for loop is iterating over a two-sided range...
-    if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node {
+    if let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), limits }) = unsugar_range(&arg) {
         // ...and both sides are compile-time constant integers...
-        if let Ok(start_idx) = eval_const_expr_partial(&cx.tcx, start_expr, ExprTypeChecked, None) {
+        if let Ok(start_idx) = eval_const_expr_partial(&cx.tcx, start, ExprTypeChecked, None) {
-            if let Ok(stop_idx) = eval_const_expr_partial(&cx.tcx, stop_expr, ExprTypeChecked, None) {
+            if let Ok(end_idx) = eval_const_expr_partial(&cx.tcx, end, ExprTypeChecked, None) {
-                // ...and the start index is greater than the stop index,
+                // ...and the start index is greater than the end index,
                 // this loop will never run. This is often confusing for developers
                 // who think that this will iterate from the larger value to the
                 // smaller value.
-                let (sup, eq) = match (start_idx, stop_idx) {
+                let (sup, eq) = match (start_idx, end_idx) {
-                    (ConstVal::Int(start_idx), ConstVal::Int(stop_idx)) => {
+                    (ConstVal::Int(start_idx), ConstVal::Int(end_idx)) => {
-                        (start_idx > stop_idx, start_idx == stop_idx)
+                        (start_idx > end_idx, start_idx == end_idx)
                     }
-                    (ConstVal::Uint(start_idx), ConstVal::Uint(stop_idx)) => {
+                    (ConstVal::Uint(start_idx), ConstVal::Uint(end_idx)) => {
-                        (start_idx > stop_idx, start_idx == stop_idx)
+                        (start_idx > end_idx, start_idx == end_idx)
                     }
                     _ => (false, false),
                 };
 
                 if sup {
-                    let start_snippet = snippet(cx, start_expr.span, "_");
+                    let start_snippet = snippet(cx, start.span, "_");
-                    let stop_snippet = snippet(cx, stop_expr.span, "_");
+                    let end_snippet = snippet(cx, end.span, "_");
 
                     span_lint_and_then(cx,
                                        REVERSE_RANGE_LOOP,
@@ -447,9 +449,9 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) {
                                                               "consider using the following if \
                                                                you are attempting to iterate \
                                                                over this range in reverse",
-                                                              format!("({}..{}).rev()` ", stop_snippet, start_snippet));
+                                                              format!("({}..{}).rev()` ", end_snippet, start_snippet));
                                        });
-                } else if eq {
+                } else if eq && limits != ast::RangeLimits::Closed {
                     // if they are equal, it's also problematic - this loop
                     // will never run.
                     span_lint(cx,

src/no_effect.rs

@@ -23,15 +23,11 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
     match expr.node {
         Expr_::ExprLit(..) |
         Expr_::ExprClosure(..) |
-        Expr_::ExprRange(None, None) |
         Expr_::ExprPath(..) => true,
         Expr_::ExprIndex(ref a, ref b) |
-        Expr_::ExprRange(Some(ref a), Some(ref b)) |
         Expr_::ExprBinary(_, ref a, ref b) => has_no_effect(cx, a) && has_no_effect(cx, b),
         Expr_::ExprVec(ref v) |
         Expr_::ExprTup(ref v) => v.iter().all(|val| has_no_effect(cx, val)),
-        Expr_::ExprRange(Some(ref inner), None) |
-        Expr_::ExprRange(None, Some(ref inner)) |
         Expr_::ExprRepeat(ref inner, _) |
         Expr_::ExprCast(ref inner, _) |
         Expr_::ExprType(ref inner, _) |
@@ -55,6 +51,13 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
                 _ => false,
             }
         }
+        Expr_::ExprBlock(ref block) => {
+            block.stmts.is_empty() && if let Some(ref expr) = block.expr {
+                has_no_effect(cx, expr)
+            } else {
+                false
+            }
+        }
         _ => false,
     }
 }

src/ranges.rs

@@ -1,7 +1,7 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
 use syntax::codemap::Spanned;
-use utils::{is_integer_literal, match_type, snippet};
+use utils::{is_integer_literal, match_type, snippet, unsugar_range, UnsugaredRange};
 
 /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates.
 ///
@@ -47,17 +47,17 @@ impl LateLintPass for StepByZero {
                               instead")
             } else if name.as_str() == "zip" && args.len() == 2 {
                 let iter = &args[0].node;
-                let zip_arg = &args[1].node;
+                let zip_arg = &args[1];
                 if_let_chain! {
                     [
                         // .iter() call
                         let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter,
                         iter_name.as_str() == "iter",
                         // range expression in .zip() call: 0..x.len()
-                        let ExprRange(Some(ref from), Some(ref to)) = *zip_arg,
+                        let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg),
-                        is_integer_literal(from, 0),
+                        is_integer_literal(start, 0),
                         // .len() call
-                        let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = to.node,
+                        let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = end.node,
                         len_name.as_str() == "len" && len_args.len() == 1,
                         // .iter() and .len() called on same Path
                         let ExprPath(_, Path { segments: ref iter_path, .. }) = iter_args[0].node,

src/utils/hir.rs

@@ -109,14 +109,16 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
                 !self.ignore_fn && lname.node == rname.node && ltys.is_empty() && rtys.is_empty() &&
                 self.eq_exprs(largs, rargs)
             }
-            (&ExprRange(ref lb, ref le), &ExprRange(ref rb, ref re)) => {
-                both(lb, rb, |l, r| self.eq_expr(l, r)) && both(le, re, |l, r| self.eq_expr(l, r))
-            }
             (&ExprRepeat(ref le, ref ll), &ExprRepeat(ref re, ref rl)) => self.eq_expr(le, re) && self.eq_expr(ll, rl),
             (&ExprRet(ref l), &ExprRet(ref r)) => both(l, r, |l, r| self.eq_expr(l, r)),
             (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => {
                 both(lqself, rqself, |l, r| self.eq_qself(l, r)) && self.eq_path(lsubpath, rsubpath)
             }
+            (&ExprStruct(ref lpath, ref lf, ref lo), &ExprStruct(ref rpath, ref rf, ref ro)) => {
+                self.eq_path(lpath, rpath) &&
+                    both(lo, ro, |l, r| self.eq_expr(l, r)) &&
+                    over(lf, rf, |l, r| self.eq_field(l, r))
+            }
             (&ExprTup(ref ltup), &ExprTup(ref rtup)) => self.eq_exprs(ltup, rtup),
             (&ExprTupField(ref le, li), &ExprTupField(ref re, ri)) => li.node == ri.node && self.eq_expr(le, re),
             (&ExprUnary(lop, ref le), &ExprUnary(rop, ref re)) => lop == rop && self.eq_expr(le, re),
@@ -132,6 +134,10 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
         over(left, right, |l, r| self.eq_expr(l, r))
     }
 
+    fn eq_field(&self, left: &Field, right: &Field) -> bool {
+        left.name.node == right.name.node && self.eq_expr(&left.expr, &right.expr)
+    }
+
     /// Check whether two patterns are the same.
     pub fn eq_pat(&self, left: &Pat, right: &Pat) -> bool {
         match (&left.node, &right.node) {
@@ -375,16 +381,6 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
                 self.hash_name(&name.node);
                 self.hash_exprs(args);
             }
-            ExprRange(ref b, ref e) => {
-                let c: fn(_, _) -> _ = ExprRange;
-                c.hash(&mut self.s);
-                if let Some(ref b) = *b {
-                    self.hash_expr(b);
-                }
-                if let Some(ref e) = *e {
-                    self.hash_expr(e);
-                }
-            }
             ExprRepeat(ref e, ref l) => {
                 let c: fn(_, _) -> _ = ExprRepeat;
                 c.hash(&mut self.s);

src/utils/mod.rs

@@ -9,7 +9,7 @@ use std::borrow::Cow;
 use std::mem;
 use std::ops::{Deref, DerefMut};
 use std::str::FromStr;
-use syntax::ast::{self, LitKind};
+use syntax::ast::{self, LitKind, RangeLimits};
 use syntax::codemap::{ExpnInfo, Span, ExpnFormat};
 use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
@@ -40,6 +40,12 @@ pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedLis
 pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
 pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
 pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"];
+pub const RANGE_FROM_PATH: [&'static str; 3] = ["std", "ops", "RangeFrom"];
+pub const RANGE_FULL_PATH: [&'static str; 3] = ["std", "ops", "RangeFull"];
+pub const RANGE_INCLUSIVE_NON_EMPTY_PATH: [&'static str; 4] = ["std", "ops", "RangeInclusive", "NonEmpty"];
+pub const RANGE_PATH: [&'static str; 3] = ["std", "ops", "Range"];
+pub const RANGE_TO_INCLUSIVE_PATH: [&'static str; 3] = ["std", "ops", "RangeToInclusive"];
+pub const RANGE_TO_PATH: [&'static str; 3] = ["std", "ops", "RangeTo"];
 pub const REGEX_NEW_PATH: [&'static str; 3] = ["regex", "Regex", "new"];
 pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"];
 pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"];
@@ -673,3 +679,55 @@ pub fn camel_case_from(s: &str) -> usize {
     }
     last_i
 }
+
+/// Represents a range akin to `ast::ExprKind::Range`.
+pub struct UnsugaredRange<'a> {
+    pub start: Option<&'a Expr>,
+    pub end: Option<&'a Expr>,
+    pub limits: RangeLimits,
+}
+
+/// Unsugar a `hir` range.
+pub fn unsugar_range(expr: &Expr) -> Option<UnsugaredRange> {
+    // To be removed when ranges get stable.
+    fn unwrap_unstable(expr: &Expr) -> &Expr {
+        if let ExprBlock(ref block) = expr.node {
+            if block.rules == BlockCheckMode::PushUnstableBlock || block.rules == BlockCheckMode::PopUnstableBlock {
+                if let Some(ref expr) = block.expr {
+                    return expr;
+                }
+            }
+        }
+
+        expr
+    }
+
+    fn get_field<'a>(name: &str, fields: &'a [Field]) -> Option<&'a Expr> {
+        let expr = &fields.iter()
+                          .find(|field| field.name.node.as_str() == name)
+                          .unwrap_or_else(|| panic!("missing {} field for range", name))
+                          .expr;
+
+        Some(unwrap_unstable(expr))
+    }
+
+    if let ExprStruct(ref path, ref fields, None) = unwrap_unstable(&expr).node {
+        if match_path(path, &RANGE_FROM_PATH) {
+            Some(UnsugaredRange { start: get_field("start", fields), end: None, limits: RangeLimits::HalfOpen })
+        } else if match_path(path, &RANGE_FULL_PATH) {
+            Some(UnsugaredRange { start: None, end: None, limits: RangeLimits::HalfOpen })
+        } else if match_path(path, &RANGE_INCLUSIVE_NON_EMPTY_PATH) {
+            Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::Closed })
+        } else if match_path(path, &RANGE_PATH) {
+            Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::HalfOpen })
+        } else if match_path(path, &RANGE_TO_INCLUSIVE_PATH) {
+            Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::Closed })
+        } else if match_path(path, &RANGE_TO_PATH) {
+            Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::HalfOpen })
+        } else {
+            None
+        }
+    } else {
+        None
+    }
+}

tests/compile-fail/copies.rs

@@ -1,4 +1,4 @@
-#![feature(plugin)]
+#![feature(plugin, inclusive_range_syntax)]
 #![plugin(clippy)]
 
 #![allow(dead_code, no_effect)]
@@ -10,16 +10,46 @@
 fn bar<T>(_: T) {}
 fn foo() -> bool { unimplemented!() }
 
+struct Foo {
+    bar: u8,
+}
+
 #[deny(if_same_then_else)]
 #[deny(match_same_arms)]
 fn if_same_then_else() -> Result<&'static str, ()> {
     if true {
+        Foo { bar: 42 };
+        0..10;
+        ..;
+        0..;
+        ..10;
+        0...10;
         foo();
     }
     else { //~ERROR this `if` has identical blocks
+        Foo { bar: 42 };
+        0..10;
+        ..;
+        0..;
+        ..10;
+        0...10;
         foo();
     }
 
+    if true {
+        Foo { bar: 42 };
+    }
+    else {
+        Foo { bar: 43 };
+    }
+
+    if true {
+        0..10;
+    }
+    else {
+        0...10;
+    }
+
     if true {
         foo();
         foo();

tests/compile-fail/for_loop.rs

@@ -1,4 +1,4 @@
-#![feature(plugin, step_by)]
+#![feature(plugin, step_by, inclusive_range_syntax)]
 #![plugin(clippy)]
 
 use std::collections::*;
@@ -118,11 +118,21 @@ fn main() {
         println!("{}", vec[i]);
     }
 
+    for i in 0...MAX_LEN {
+        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
+        println!("{}", vec[i]);
+    }
+
     for i in 5..10 {
         //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
         println!("{}", vec[i]);
     }
 
+    for i in 5...10 {
+        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
+        println!("{}", vec[i]);
+    }
+
     for i in 5..vec.len() {
         //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().skip(5)`
         println!("{} {}", vec[i], i);
@@ -140,6 +150,13 @@ fn main() {
         println!("{}", i);
     }
 
+    for i in 10...0 {
+        //~^ERROR this range is empty so this for loop will never run
+        //~|HELP consider
+        //~|SUGGESTION (0..10).rev()
+        println!("{}", i);
+    }
+
     for i in MAX_LEN..0 { //~ERROR this range is empty so this for loop will never run
         //~|HELP consider
         //~|SUGGESTION (0..MAX_LEN).rev()
@@ -150,6 +167,10 @@ fn main() {
         println!("{}", i);
     }
 
+    for i in 5...5 { // not an error, this is the range with only one element “5”
+        println!("{}", i);
+    }
+
     for i in 0..10 { // not an error, the start index is less than the end index
         println!("{}", i);
     }
@@ -158,10 +179,6 @@ fn main() {
         println!("{}", i);
     }
 
-    for i in (10..0).rev() { // not an error, this is an established idiom for looping backwards on a range
-        println!("{}", i);
-    }
-
     for i in (10..0).map(|x| x * 2) { // not an error, it can't be known what arbitrary methods do to a range
         println!("{}", i);
     }

tests/compile-fail/no_effect.rs

@@ -1,4 +1,4 @@
-#![feature(plugin, box_syntax)]
+#![feature(plugin, box_syntax, inclusive_range_syntax)]
 #![plugin(clippy)]
 
 #![deny(no_effect)]
@@ -39,6 +39,7 @@ fn main() {
     5..; //~ERROR statement with no effect
     ..5; //~ERROR statement with no effect
     5..6; //~ERROR statement with no effect
+    5...6; //~ERROR statement with no effect
     [42, 55]; //~ERROR statement with no effect
     [42, 55][1]; //~ERROR statement with no effect
     (42, 55).1; //~ERROR statement with no effect