Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "clippy"
version = "0.0.46"
version = "0.0.47"
authors = [
"Manish Goregaokar <[email protected]>",
"Andre Bogus <[email protected]>",
Expand Down
44 changes: 23 additions & 21 deletions src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
if is_len_call(&r, &indexed) {
let take: Cow<_> = if let Some(ref end) = *end {
if is_len_call(&end, &indexed) {
"".into()
} else {
format!(".take({})", snippet(cx, r.span, "..")).into()
format!(".take({})", snippet(cx, end.span, "..")).into()
}
} else {
"".into()
Expand Down Expand Up @@ -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(stop_idx) = eval_const_expr_partial(&cx.tcx, stop_expr, ExprTypeChecked, None) {
// ...and the start index is greater than the stop index,
if let Ok(start_idx) = eval_const_expr_partial(&cx.tcx, start, ExprTypeChecked, None) {
if let Ok(end_idx) = eval_const_expr_partial(&cx.tcx, end, ExprTypeChecked, None) {
// ...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) {
(ConstVal::Int(start_idx), ConstVal::Int(stop_idx)) => {
(start_idx > stop_idx, start_idx == stop_idx)
let (sup, eq) = match (start_idx, end_idx) {
(ConstVal::Int(start_idx), ConstVal::Int(end_idx)) => {
(start_idx > end_idx, start_idx == end_idx)
}
(ConstVal::Uint(start_idx), ConstVal::Uint(stop_idx)) => {
(start_idx > stop_idx, start_idx == stop_idx)
(ConstVal::Uint(start_idx), ConstVal::Uint(end_idx)) => {
(start_idx > end_idx, start_idx == end_idx)
}
_ => (false, false),
};

if sup {
let start_snippet = snippet(cx, start_expr.span, "_");
let stop_snippet = snippet(cx, stop_expr.span, "_");
let start_snippet = snippet(cx, start.span, "_");
let end_snippet = snippet(cx, end.span, "_");

span_lint_and_then(cx,
REVERSE_RANGE_LOOP,
Expand All @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions src/no_effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) |
Expand All @@ -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,
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/ranges.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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,
is_integer_literal(from, 0),
let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg),
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,
Expand Down
22 changes: 9 additions & 13 deletions src/utils/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
60 changes: 59 additions & 1 deletion src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"];
Expand Down Expand Up @@ -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
}
}
32 changes: 31 additions & 1 deletion tests/compile-fail/copies.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(plugin)]
#![feature(plugin, inclusive_range_syntax)]
#![plugin(clippy)]

#![allow(dead_code, no_effect)]
Expand All @@ -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();
Expand Down
Loading