2015-08-12 14:56:27 -05:00
use rustc ::lint ::* ;
2015-09-03 09:42:17 -05:00
use rustc_front ::hir ::* ;
use reexport ::* ;
2015-11-19 08:51:30 -06:00
use rustc_front ::intravisit ::{ Visitor , walk_expr , walk_block , walk_decl } ;
2015-08-25 11:26:20 -05:00
use rustc ::middle ::ty ;
2016-01-22 06:24:44 -06:00
use rustc ::middle ::def ::Def ;
2015-09-15 00:20:56 -05:00
use consts ::{ constant_simple , Constant } ;
2016-01-03 22:26:12 -06:00
use rustc ::front ::map ::Node ::NodeBlock ;
2015-09-27 02:39:42 -05:00
use std ::borrow ::Cow ;
2016-01-03 22:26:12 -06:00
use std ::collections ::{ HashSet , HashMap } ;
2015-08-12 14:56:27 -05:00
2016-01-03 22:26:12 -06:00
use utils ::{ snippet , span_lint , get_parent_expr , match_trait_method , match_type , in_external_macro , expr_block ,
span_help_and_lint , is_integer_literal , get_enclosing_block } ;
2016-01-29 17:15:57 -06:00
use utils ::{ HASHMAP_PATH , VEC_PATH , LL_PATH , OPTION_PATH , RESULT_PATH } ;
2015-08-12 14:56:27 -05:00
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index. It is `Warn` by default.
///
/// **Why is this bad?** Just iterating the collection itself makes the intent more clear and is probably faster.
///
/// **Known problems:** None
///
/// **Example:**
/// ```
/// for i in 0..vec.len() {
/// println!("{}", vec[i]);
/// }
/// ```
2015-08-12 14:56:27 -05:00
declare_lint! { pub NEEDLESS_RANGE_LOOP , Warn ,
2015-08-13 03:32:35 -05:00
" for-looping over a range of indices where an iterator over items would do " }
2015-08-12 14:56:27 -05:00
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks for loops on `x.iter()` where `&x` will do, and suggest the latter. It is `Warn` by default.
///
/// **Why is this bad?** Readability.
///
/// **Known problems:** False negatives. We currently only warn on some known types.
///
/// **Example:** `for x in y.iter() { .. }` (where y is a `Vec` or slice)
2015-08-13 08:36:31 -05:00
declare_lint! { pub EXPLICIT_ITER_LOOP , Warn ,
" for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do " }
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks for loops on `x.next()`. It is `Warn` by default.
///
/// **Why is this bad?** `next()` returns either `Some(value)` if there was a value, or `None` otherwise. The insidious thing is that `Option<_>` implements `IntoIterator`, so that possibly one value will be iterated, leading to some hard to find bugs. No one will want to write such code [except to win an Underhanded Rust Contest](https://www.reddit.com/r/rust/comments/3hb0wm/underhanded_rust_contest/cu5yuhr).
///
/// **Known problems:** None
///
/// **Example:** `for x in y.next() { .. }`
2015-08-17 00:23:57 -05:00
declare_lint! { pub ITER_NEXT_LOOP , Warn ,
" for-looping over `_.next()` which is probably not intended " }
2016-01-29 17:15:57 -06:00
/// **What it does:** This lint checks for `for` loops over `Option` values. It is `Warn` by default.
2016-01-29 01:34:09 -06:00
///
/// **Why is this bad?** Readability. This is more clearly expressed as an `if let`.
///
/// **Known problems:** None
///
/// **Example:** `for x in option { .. }`. This should be `if let Some(x) = option { .. }`.
declare_lint! { pub FOR_LOOP_OVER_OPTION , Warn ,
2016-01-29 17:15:57 -06:00
" for-looping over an `Option`, which is more clearly expressed as an `if let` " }
/// **What it does:** This lint checks for `for` loops over `Result` values. It is `Warn` by default.
///
/// **Why is this bad?** Readability. This is more clearly expressed as an `if let`.
///
/// **Known problems:** None
///
/// **Example:** `for x in result { .. }`. This should be `if let Ok(x) = result { .. }`.
declare_lint! { pub FOR_LOOP_OVER_RESULT , Warn ,
" for-looping over a `Result`, which is more clearly expressed as an `if let` " }
2016-01-29 01:34:09 -06:00
2016-01-01 10:48:19 -06:00
/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop. It is `Warn` by default.
2015-12-10 18:22:27 -06:00
///
/// **Why is this bad?** The `while let` loop is usually shorter and more readable
///
/// **Known problems:** Sometimes the wrong binding is displayed (#383)
///
/// **Example:**
///
/// ```
/// loop {
/// let x = match y {
/// Some(x) => x,
/// None => break,
/// }
/// // .. do something with x
/// }
/// // is easier written as
/// while let Some(x) = y {
/// // .. do something with x
/// }
/// ```
2015-08-29 04:41:06 -05:00
declare_lint! { pub WHILE_LET_LOOP , Warn ,
" `loop { if let { ... } else break }` can be written as a `while let` loop " }
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks for using `collect()` on an iterator without using the result. It is `Warn` by default.
///
/// **Why is this bad?** It is more idiomatic to use a `for` loop over the iterator instead.
///
/// **Known problems:** None
///
/// **Example:** `vec.iter().map(|x| /* some operation returning () */).collect::<Vec<_>>();`
2015-08-30 06:10:59 -05:00
declare_lint! { pub UNUSED_COLLECT , Warn ,
" `collect()`ing an iterator without using the result; this is usually better \
written as a for loop " }
2016-01-01 10:48:19 -06:00
/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`. It is `Warn` by default.
2015-12-10 18:22:27 -06:00
///
/// **Why is it bad?** Such loops will either be skipped or loop until wrap-around (in debug code, this may `panic!()`). Both options are probably not intended.
///
/// **Known problems:** The lint cannot catch loops over dynamically defined ranges. Doing this would require simulating all possible inputs and code paths through the program, which would be complex and error-prone.
///
/// **Examples**: `for x in 5..10-5 { .. }` (oops, stray `-`)
2015-09-14 19:19:05 -05:00
declare_lint! { pub REVERSE_RANGE_LOOP , Warn ,
" Iterating over an empty range, such as `10..0` or `5..5` " }
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks `for` loops over slices with an explicit counter and suggests the use of `.enumerate()`. It is `Warn` by default.
///
/// **Why is it bad?** Not only is the version using `.enumerate()` more readable, the compiler is able to remove bounds checks which can lead to faster code in some instances.
///
/// **Known problems:** None.
///
/// **Example:** `for i in 0..v.len() { foo(v[i]); }` or `for i in 0..v.len() { bar(i, v[i]); }`
2015-08-23 12:25:45 -05:00
declare_lint! { pub EXPLICIT_COUNTER_LOOP , Warn ,
" for-looping with an explicit counter when `_.enumerate()` would do " }
2015-12-10 18:22:27 -06:00
/// **What it does:** This lint checks for empty `loop` expressions. It is `Warn` by default.
///
/// **Why is this bad?** Those busy loops burn CPU cycles without doing anything. Think of the environment and either block on something or at least make the thread sleep for some microseconds.
///
/// **Known problems:** None
///
/// **Example:** `loop {}`
2015-10-12 06:38:18 -05:00
declare_lint! { pub EMPTY_LOOP , Warn , " empty `loop {}` detected " }
2015-12-14 15:16:56 -06:00
/// **What it does:** This lint checks for `while let` expressions on iterators. It is `Warn` by default.
///
/// **Why is this bad?** Readability. A simple `for` loop is shorter and conveys the intent better.
///
/// **Known problems:** None
///
/// **Example:** `while let Some(val) = iter() { .. }`
2015-10-16 13:27:13 -05:00
declare_lint! { pub WHILE_LET_ON_ITERATOR , Warn , " using a while-let loop instead of a for loop on an iterator " }
2015-08-12 14:56:27 -05:00
#[ derive(Copy, Clone) ]
pub struct LoopsPass ;
impl LintPass for LoopsPass {
fn get_lints ( & self ) -> LintArray {
2016-01-03 22:26:12 -06:00
lint_array! ( NEEDLESS_RANGE_LOOP ,
EXPLICIT_ITER_LOOP ,
ITER_NEXT_LOOP ,
WHILE_LET_LOOP ,
UNUSED_COLLECT ,
REVERSE_RANGE_LOOP ,
EXPLICIT_COUNTER_LOOP ,
EMPTY_LOOP ,
2015-10-16 13:27:13 -05:00
WHILE_LET_ON_ITERATOR )
2015-08-12 14:56:27 -05:00
}
2015-09-18 21:53:04 -05:00
}
2015-08-12 14:56:27 -05:00
2015-09-18 21:53:04 -05:00
impl LateLintPass for LoopsPass {
fn check_expr ( & mut self , cx : & LateContext , expr : & Expr ) {
2015-08-12 14:56:27 -05:00
if let Some ( ( pat , arg , body ) ) = recover_for_loop ( expr ) {
2015-11-18 05:35:18 -06:00
check_for_loop ( cx , pat , arg , body , expr ) ;
2015-08-12 14:56:27 -05:00
}
2015-08-29 04:41:06 -05:00
// check for `loop { if let {} else break }` that could be `while let`
2015-09-27 02:39:42 -05:00
// (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration)
2015-08-29 04:41:06 -05:00
if let ExprLoop ( ref block , _ ) = expr . node {
2015-10-12 06:38:18 -05:00
// also check for empty `loop {}` statements
if block . stmts . is_empty ( ) & & block . expr . is_none ( ) {
2016-01-03 22:26:12 -06:00
span_lint ( cx ,
EMPTY_LOOP ,
expr . span ,
" empty `loop {}` detected. You may want to either use `panic!()` or add \
` std ::thread ::sleep ( .. ) ; ` to the loop body . " );
2015-10-12 06:38:18 -05:00
}
2015-10-14 04:44:09 -05:00
2015-10-11 11:49:01 -05:00
// extract the expression from the first statement (if any) in a block
let inner_stmt_expr = extract_expr_from_first_stmt ( block ) ;
2015-12-14 07:30:09 -06:00
// or extract the first expression (if any) from the block
if let Some ( inner ) = inner_stmt_expr . or_else ( | | extract_first_expr ( block ) ) {
2015-08-29 04:41:06 -05:00
if let ExprMatch ( ref matchexpr , ref arms , ref source ) = inner . node {
2015-10-11 11:49:01 -05:00
// collect the remaining statements below the match
let mut other_stuff = block . stmts
2016-01-03 22:26:12 -06:00
. iter ( )
. skip ( 1 )
. map ( | stmt | format! ( " {} " , snippet ( cx , stmt . span , " .. " ) ) )
. collect ::< Vec < String > > ( ) ;
2015-12-14 07:30:09 -06:00
if inner_stmt_expr . is_some ( ) {
// if we have a statement which has a match,
if let Some ( ref expr ) = block . expr {
// then collect the expression (without semicolon) below it
other_stuff . push ( format! ( " {} " , snippet ( cx , expr . span , " .. " ) ) ) ;
2015-10-11 11:49:01 -05:00
}
}
2015-09-27 02:39:42 -05:00
2015-08-29 04:41:06 -05:00
// ensure "if let" compatible match structure
match * source {
2016-01-03 22:26:12 -06:00
MatchSource ::Normal | MatchSource ::IfLetDesugar { .. } = > {
if arms . len ( ) = = 2 & & arms [ 0 ] . pats . len ( ) = = 1 & & arms [ 0 ] . guard . is_none ( ) & &
arms [ 1 ] . pats . len ( ) = = 1 & & arms [ 1 ] . guard . is_none ( ) & &
is_break_expr ( & arms [ 1 ] . body ) {
if in_external_macro ( cx , expr . span ) {
return ;
}
let loop_body = if inner_stmt_expr . is_some ( ) {
// FIXME: should probably be an ellipsis
// tabbing and newline is probably a bad idea, especially for large blocks
Cow ::Owned ( format! ( " {{ \n {} \n }} " , other_stuff . join ( " \n " ) ) )
} else {
expr_block ( cx , & arms [ 0 ] . body , Some ( other_stuff . join ( " \n " ) ) , " .. " )
} ;
span_help_and_lint ( cx ,
WHILE_LET_LOOP ,
expr . span ,
" this loop could be written as a `while let` loop " ,
& format! ( " try \n while let {} = {} {} " ,
snippet ( cx , arms [ 0 ] . pats [ 0 ] . span , " .. " ) ,
snippet ( cx , matchexpr . span , " .. " ) ,
loop_body ) ) ;
}
}
_ = > ( ) ,
2015-08-29 04:41:06 -05:00
}
}
}
}
2015-10-22 16:16:58 -05:00
if let ExprMatch ( ref match_expr , ref arms , MatchSource ::WhileLetDesugar ) = expr . node {
2015-10-16 13:27:13 -05:00
let pat = & arms [ 0 ] . pats [ 0 ] . node ;
2015-10-22 16:16:58 -05:00
if let ( & PatEnum ( ref path , Some ( ref pat_args ) ) ,
2016-01-03 22:26:12 -06:00
& ExprMethodCall ( method_name , _ , ref method_args ) ) = ( pat , & match_expr . node ) {
2015-10-26 17:49:37 -05:00
let iter_expr = & method_args [ 0 ] ;
2015-10-22 16:16:58 -05:00
if let Some ( lhs_constructor ) = path . segments . last ( ) {
if method_name . node . as_str ( ) = = " next " & &
2016-01-03 22:26:12 -06:00
match_trait_method ( cx , match_expr , & [ " core " , " iter " , " Iterator " ] ) & &
lhs_constructor . identifier . name . as_str ( ) = = " Some " & &
! is_iterator_used_after_while_let ( cx , iter_expr ) {
2015-10-22 16:16:58 -05:00
let iterator = snippet ( cx , method_args [ 0 ] . span , " _ " ) ;
let loop_var = snippet ( cx , pat_args [ 0 ] . span , " _ " ) ;
2016-01-03 22:26:12 -06:00
span_help_and_lint ( cx ,
WHILE_LET_ON_ITERATOR ,
expr . span ,
2015-10-22 16:16:58 -05:00
" this loop could be written as a `for` loop " ,
2016-01-03 22:26:12 -06:00
& format! ( " try \n for {} in {} {{ ... }} " , loop_var , iterator ) ) ;
2015-10-22 16:16:58 -05:00
}
2015-10-16 13:27:13 -05:00
}
}
}
2015-08-12 14:56:27 -05:00
}
2015-08-30 06:10:59 -05:00
2015-09-18 21:53:04 -05:00
fn check_stmt ( & mut self , cx : & LateContext , stmt : & Stmt ) {
2015-08-30 06:10:59 -05:00
if let StmtSemi ( ref expr , _ ) = stmt . node {
if let ExprMethodCall ( ref method , _ , ref args ) = expr . node {
2015-09-28 00:04:06 -05:00
if args . len ( ) = = 1 & & method . node . as_str ( ) = = " collect " & &
2016-01-03 22:26:12 -06:00
match_trait_method ( cx , expr , & [ " core " , " iter " , " Iterator " ] ) {
span_lint ( cx ,
UNUSED_COLLECT ,
expr . span ,
& format! ( " you are collect()ing an iterator and throwing away the result. Consider \
using an explicit for loop to exhaust the iterator " ));
2015-08-30 06:10:59 -05:00
}
}
}
}
2015-08-12 14:56:27 -05:00
}
2015-11-18 05:35:18 -06:00
fn check_for_loop ( cx : & LateContext , pat : & Pat , arg : & Expr , body : & Expr , expr : & Expr ) {
2016-01-14 13:58:32 -06:00
check_for_loop_range ( cx , pat , arg , body , expr ) ;
check_for_loop_reverse_range ( cx , arg , expr ) ;
2016-01-29 01:34:09 -06:00
check_for_loop_arg ( cx , pat , arg , expr ) ;
2016-01-14 13:58:32 -06:00
check_for_loop_explicit_counter ( cx , arg , body , expr ) ;
}
2015-11-18 05:35:18 -06:00
2016-01-14 13:58:32 -06:00
/// 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 {
// the var must be a single name
if let PatIdent ( _ , ref ident , _ ) = pat . node {
let mut visitor = VarVisitor {
cx : cx ,
var : ident . node . name ,
indexed : HashSet ::new ( ) ,
nonindex : false ,
} ;
walk_expr ( & mut visitor , body ) ;
// linting condition: we only indexed one variable
if visitor . indexed . len ( ) = = 1 {
let indexed = visitor . indexed
. into_iter ( )
. next ( )
. expect ( " Len was nonzero, but no contents found " ) ;
let starts_at_zero = is_integer_literal ( l , 0 ) ;
let skip : Cow < _ > = if starts_at_zero {
" " . into ( )
}
else {
format! ( " .skip( {} ) " , snippet ( cx , l . span , " .. " ) ) . into ( )
} ;
let take : Cow < _ > = if let Some ( ref r ) = * r {
if ! is_len_call ( & r , & indexed ) {
format! ( " .take( {} ) " , snippet ( cx , r . span , " .. " ) ) . into ( )
}
else {
" " . into ( )
}
} else {
" " . into ( )
} ;
if visitor . nonindex {
span_lint ( cx ,
NEEDLESS_RANGE_LOOP ,
expr . span ,
& format! ( " the loop variable ` {} ` is used to index ` {} `. \
Consider using ` for ( { } , item ) in { } . iter ( ) . enumerate ( ) { } { } ` or similar iterators " ,
ident . node . name ,
indexed ,
ident . node . name ,
indexed ,
take ,
skip ) ) ;
} else {
let repl = if starts_at_zero & & take . is_empty ( ) {
format! ( " & {} " , indexed )
2015-11-18 05:35:18 -06:00
}
2016-01-14 13:58:32 -06:00
else {
format! ( " {} .iter() {} {} " , indexed , take , skip )
} ;
span_lint ( cx ,
NEEDLESS_RANGE_LOOP ,
expr . span ,
& format! ( " the loop variable ` {} ` is only used to index ` {} `. \
Consider using ` for item in { } ` or similar iterators " ,
ident . node . name ,
indexed ,
repl ) ) ;
2015-11-18 05:35:18 -06:00
}
}
}
}
2016-01-14 13:58:32 -06:00
}
fn is_len_call ( expr : & Expr , var : & Name ) -> bool {
if_let_chain! { [
let ExprMethodCall ( method , _ , ref len_args ) = expr . node ,
len_args . len ( ) = = 1 ,
method . node . as_str ( ) = = " len " ,
let ExprPath ( _ , ref path ) = len_args [ 0 ] . node ,
path . segments . len ( ) = = 1 ,
& path . segments [ 0 ] . identifier . name = = var
] , {
return true ;
} }
2015-11-18 05:35:18 -06:00
2016-01-14 13:58:32 -06:00
false
}
fn check_for_loop_reverse_range ( cx : & LateContext , arg : & Expr , expr : & Expr ) {
2015-11-18 05:35:18 -06:00
// if this for loop is iterating over a two-sided range...
if let ExprRange ( Some ( ref start_expr ) , Some ( ref stop_expr ) ) = arg . node {
// ...and both sides are compile-time constant integers...
2016-02-01 05:51:33 -06:00
if let Some ( start_idx @ Constant ::Int ( .. ) ) = constant_simple ( start_expr ) {
if let Some ( stop_idx @ Constant ::Int ( .. ) ) = constant_simple ( stop_expr ) {
2015-11-18 05:35:18 -06:00
// ...and the start index is greater than the stop 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.
if start_idx > stop_idx {
2016-01-03 22:26:12 -06:00
span_help_and_lint ( cx ,
REVERSE_RANGE_LOOP ,
expr . span ,
" this range is empty so this for loop will never run " ,
& format! ( " Consider using `( {} .. {} ).rev()` if you are attempting to iterate \
over this range in reverse " ,
stop_idx ,
start_idx ) ) ;
2015-11-18 05:35:18 -06:00
} else if start_idx = = stop_idx {
// if they are equal, it's also problematic - this loop
// will never run.
2016-01-03 22:26:12 -06:00
span_lint ( cx ,
REVERSE_RANGE_LOOP ,
expr . span ,
" this range is empty so this for loop will never run " ) ;
2015-11-18 05:35:18 -06:00
}
}
}
}
2016-01-14 13:58:32 -06:00
}
2015-11-18 05:35:18 -06:00
2016-01-29 01:34:09 -06:00
fn check_for_loop_arg ( cx : & LateContext , pat : & Pat , arg : & Expr , expr : & Expr ) {
let mut next_loop_linted = false ; // whether or not ITER_NEXT_LOOP lint was used
2015-11-18 05:35:18 -06:00
if let ExprMethodCall ( ref method , _ , ref args ) = arg . node {
// just the receiver, no arguments
if args . len ( ) = = 1 {
let method_name = method . node ;
// check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
if method_name . as_str ( ) = = " iter " | | method_name . as_str ( ) = = " iter_mut " {
if is_ref_iterable_type ( cx , & args [ 0 ] ) {
let object = snippet ( cx , args [ 0 ] . span , " _ " ) ;
2016-01-03 22:26:12 -06:00
span_lint ( cx ,
EXPLICIT_ITER_LOOP ,
expr . span ,
& format! ( " it is more idiomatic to loop over `& {} {} ` instead of ` {} . {} ()` " ,
if method_name . as_str ( ) = = " iter_mut " {
" mut "
} else {
" "
} ,
object ,
object ,
method_name ) ) ;
2015-11-18 05:35:18 -06:00
}
2016-01-03 22:26:12 -06:00
} else if method_name . as_str ( ) = = " next " & & match_trait_method ( cx , arg , & [ " core " , " iter " , " Iterator " ] ) {
span_lint ( cx ,
ITER_NEXT_LOOP ,
expr . span ,
" you are iterating over `Iterator::next()` which is an Option; this will compile but is \
probably not what you want " );
2016-01-29 01:34:09 -06:00
next_loop_linted = true ;
2015-11-18 05:35:18 -06:00
}
}
}
2016-01-29 01:34:09 -06:00
if ! next_loop_linted {
2016-01-29 17:15:57 -06:00
check_arg_type ( cx , pat , arg ) ;
2016-01-29 01:34:09 -06:00
}
}
2015-11-18 05:35:18 -06:00
2016-01-29 17:15:57 -06:00
/// Check for `for` loops over `Option`s and `Results`
fn check_arg_type ( cx : & LateContext , pat : & Pat , arg : & Expr ) {
2016-01-29 01:34:09 -06:00
let ty = cx . tcx . expr_ty ( arg ) ;
if match_type ( cx , ty , & OPTION_PATH ) {
span_help_and_lint (
cx ,
FOR_LOOP_OVER_OPTION ,
arg . span ,
2016-01-29 17:15:57 -06:00
& format! ( " for loop over ` {0} `, which is an `Option`. This is more readably written as \
2016-01-29 01:34:09 -06:00
an ` if let ` statement . " , snippet(cx, arg.span, " _ " )),
& format! ( " consider replacing `for {0} in {1} ` with `if let Some( {0} ) = {1} ` " ,
snippet ( cx , pat . span , " _ " ) , snippet ( cx , arg . span , " _ " ) )
) ;
}
2016-01-29 17:15:57 -06:00
else if match_type ( cx , ty , & RESULT_PATH ) {
span_help_and_lint (
cx ,
FOR_LOOP_OVER_RESULT ,
arg . span ,
& format! ( " for loop over ` {0} `, which is a `Result`. This is more readably written as \
an ` if let ` statement . " , snippet(cx, arg.span, " _ " )),
& format! ( " consider replacing `for {0} in {1} ` with `if let Ok( {0} ) = {1} ` " ,
snippet ( cx , pat . span , " _ " ) , snippet ( cx , arg . span , " _ " ) )
) ;
}
2016-01-14 13:58:32 -06:00
}
fn check_for_loop_explicit_counter ( cx : & LateContext , arg : & Expr , body : & Expr , expr : & Expr ) {
2015-11-18 05:35:18 -06:00
// Look for variables that are incremented once per loop iteration.
2016-01-03 22:26:12 -06:00
let mut visitor = IncrementVisitor {
cx : cx ,
states : HashMap ::new ( ) ,
depth : 0 ,
done : false ,
} ;
2015-11-18 05:35:18 -06:00
walk_expr ( & mut visitor , body ) ;
// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
let map = & cx . tcx . map ;
2016-01-03 22:26:12 -06:00
let parent_scope = map . get_enclosing_scope ( expr . id ) . and_then ( | id | map . get_enclosing_scope ( id ) ) ;
2015-11-18 05:35:18 -06:00
if let Some ( parent_id ) = parent_scope {
if let NodeBlock ( block ) = map . get ( parent_id ) {
2016-01-03 22:26:12 -06:00
for ( id , _ ) in visitor . states . iter ( ) . filter ( | & ( _ , v ) | * v = = VarState ::IncrOnce ) {
let mut visitor2 = InitializeVisitor {
cx : cx ,
end_expr : expr ,
var_id : id . clone ( ) ,
state : VarState ::IncrOnce ,
name : None ,
depth : 0 ,
past_loop : false ,
} ;
2015-11-18 05:35:18 -06:00
walk_block ( & mut visitor2 , block ) ;
if visitor2 . state = = VarState ::Warn {
if let Some ( name ) = visitor2 . name {
2016-01-03 22:26:12 -06:00
span_lint ( cx ,
EXPLICIT_COUNTER_LOOP ,
expr . span ,
& format! ( " the variable ` {0} ` is used as a loop counter. Consider using `for ( {0} , \
item ) in { 1 } . enumerate ( ) ` or similar iterators " ,
name ,
snippet ( cx , arg . span , " _ " ) ) ) ;
2015-11-18 05:35:18 -06:00
}
}
}
}
}
}
2015-08-12 14:56:27 -05:00
/// Recover the essential nodes of a desugared for loop:
/// `for pat in arg { body }` becomes `(pat, arg, body)`.
2015-08-13 09:41:51 -05:00
fn recover_for_loop ( expr : & Expr ) -> Option < ( & Pat , & Expr , & Expr ) > {
2015-08-12 14:56:27 -05:00
if_let_chain! {
[
let ExprMatch ( ref iterexpr , ref arms , _ ) = expr . node ,
let ExprCall ( _ , ref iterargs ) = iterexpr . node ,
2015-08-13 09:41:51 -05:00
iterargs . len ( ) = = 1 & & arms . len ( ) = = 1 & & arms [ 0 ] . guard . is_none ( ) ,
2015-08-12 14:56:27 -05:00
let ExprLoop ( ref block , _ ) = arms [ 0 ] . body . node ,
block . stmts . is_empty ( ) ,
let Some ( ref loopexpr ) = block . expr ,
let ExprMatch ( _ , ref innerarms , MatchSource ::ForLoopDesugar ) = loopexpr . node ,
innerarms . len ( ) = = 2 & & innerarms [ 0 ] . pats . len ( ) = = 1 ,
let PatEnum ( _ , Some ( ref somepats ) ) = innerarms [ 0 ] . pats [ 0 ] . node ,
somepats . len ( ) = = 1
] , {
2015-08-25 07:41:35 -05:00
return Some ( ( & somepats [ 0 ] ,
& iterargs [ 0 ] ,
& innerarms [ 0 ] . body ) ) ;
2015-08-12 14:56:27 -05:00
}
}
None
}
struct VarVisitor < ' v , ' t : ' v > {
2015-09-18 21:53:04 -05:00
cx : & ' v LateContext < ' v , ' t > , // context reference
2016-01-03 22:26:12 -06:00
var : Name , // var name to look for as index
indexed : HashSet < Name > , // indexed variables
nonindex : bool , // has the var been used otherwise?
2015-08-12 14:56:27 -05:00
}
impl < ' v , ' t > Visitor < ' v > for VarVisitor < ' v , ' t > {
fn visit_expr ( & mut self , expr : & ' v Expr ) {
if let ExprPath ( None , ref path ) = expr . node {
if path . segments . len ( ) = = 1 & & path . segments [ 0 ] . identifier . name = = self . var {
// we are referencing our variable! now check if it's as an index
if_let_chain! {
[
let Some ( parexpr ) = get_parent_expr ( self . cx , expr ) ,
let ExprIndex ( ref seqexpr , _ ) = parexpr . node ,
let ExprPath ( None , ref seqvar ) = seqexpr . node ,
seqvar . segments . len ( ) = = 1
] , {
self . indexed . insert ( seqvar . segments [ 0 ] . identifier . name ) ;
return ; // no need to walk further
}
}
// we are not indexing anything, record that
self . nonindex = true ;
return ;
}
}
walk_expr ( self , expr ) ;
}
}
2015-08-25 11:26:20 -05:00
2015-10-26 17:49:37 -05:00
fn is_iterator_used_after_while_let ( cx : & LateContext , iter_expr : & Expr ) -> bool {
let def_id = match var_def_id ( cx , iter_expr ) {
Some ( id ) = > id ,
2016-01-03 22:26:12 -06:00
None = > return false ,
2015-10-26 17:49:37 -05:00
} ;
let mut visitor = VarUsedAfterLoopVisitor {
cx : cx ,
def_id : def_id ,
iter_expr_id : iter_expr . id ,
past_while_let : false ,
2016-01-03 22:26:12 -06:00
var_used_after_while_let : false ,
2015-10-26 17:49:37 -05:00
} ;
if let Some ( enclosing_block ) = get_enclosing_block ( cx , def_id ) {
walk_block ( & mut visitor , enclosing_block ) ;
2015-10-19 18:04:21 -05:00
}
2015-10-26 17:49:37 -05:00
visitor . var_used_after_while_let
2015-10-19 18:04:21 -05:00
}
2015-10-26 17:49:37 -05:00
struct VarUsedAfterLoopVisitor < ' v , ' t : ' v > {
2015-10-19 18:04:21 -05:00
cx : & ' v LateContext < ' v , ' t > ,
def_id : NodeId ,
2015-10-26 17:49:37 -05:00
iter_expr_id : NodeId ,
past_while_let : bool ,
2016-01-03 22:26:12 -06:00
var_used_after_while_let : bool ,
2015-10-19 18:04:21 -05:00
}
2016-01-03 22:26:12 -06:00
impl < ' v , ' t > Visitor < ' v > for VarUsedAfterLoopVisitor < ' v , ' t > {
2015-10-19 18:04:21 -05:00
fn visit_expr ( & mut self , expr : & ' v Expr ) {
2015-10-26 17:49:37 -05:00
if self . past_while_let {
if Some ( self . def_id ) = = var_def_id ( self . cx , expr ) {
self . var_used_after_while_let = true ;
}
} else if self . iter_expr_id = = expr . id {
self . past_while_let = true ;
2015-10-19 18:04:21 -05:00
}
walk_expr ( self , expr ) ;
}
}
2015-10-26 17:49:37 -05:00
2015-08-25 11:26:20 -05:00
/// Return true if the type of expr is one that provides IntoIterator impls
/// for &T and &mut T, such as Vec.
2015-09-18 21:53:04 -05:00
fn is_ref_iterable_type ( cx : & LateContext , e : & Expr ) -> bool {
2015-08-31 01:29:34 -05:00
// no walk_ptrs_ty: calling iter() on a reference can make sense because it
// will allow further borrows afterwards
let ty = cx . tcx . expr_ty ( e ) ;
2016-01-03 22:26:12 -06:00
is_iterable_array ( ty ) | | match_type ( cx , ty , & VEC_PATH ) | | match_type ( cx , ty , & LL_PATH ) | |
match_type ( cx , ty , & HASHMAP_PATH ) | | match_type ( cx , ty , & [ " std " , " collections " , " hash " , " set " , " HashSet " ] ) | |
match_type ( cx , ty , & [ " collections " , " vec_deque " , " VecDeque " ] ) | |
match_type ( cx , ty , & [ " collections " , " binary_heap " , " BinaryHeap " ] ) | |
match_type ( cx , ty , & [ " collections " , " btree " , " map " , " BTreeMap " ] ) | |
match_type ( cx , ty , & [ " collections " , " btree " , " set " , " BTreeSet " ] )
2015-08-25 11:26:20 -05:00
}
2015-09-06 06:36:21 -05:00
fn is_iterable_array ( ty : ty ::Ty ) -> bool {
2015-09-27 02:39:42 -05:00
// IntoIterator is currently only implemented for array sizes <= 32 in rustc
2015-08-25 11:26:20 -05:00
match ty . sty {
2015-09-06 06:36:21 -05:00
ty ::TyArray ( _ , 0 .. . 32 ) = > true ,
2016-01-03 22:26:12 -06:00
_ = > false ,
2015-08-25 11:26:20 -05:00
}
}
2015-08-29 04:41:06 -05:00
2015-09-27 02:39:42 -05:00
/// If a block begins with a statement (possibly a `let` binding) and has an expression, return it.
fn extract_expr_from_first_stmt ( block : & Block ) -> Option < & Expr > {
2016-01-03 22:26:12 -06:00
if block . stmts . is_empty ( ) {
return None ;
}
2015-10-14 04:44:09 -05:00
if let StmtDecl ( ref decl , _ ) = block . stmts [ 0 ] . node {
if let DeclLocal ( ref local ) = decl . node {
2016-01-03 22:26:12 -06:00
if let Some ( ref expr ) = local . init {
Some ( expr )
} else {
None
}
} else {
None
}
} else {
None
}
2015-09-27 02:39:42 -05:00
}
/// If a block begins with an expression (with or without semicolon), return it.
fn extract_first_expr ( block : & Block ) -> Option < & Expr > {
match block . expr {
Some ( ref expr ) = > Some ( expr ) ,
2016-01-03 22:26:12 -06:00
None if ! block . stmts . is_empty ( ) = > {
match block . stmts [ 0 ] . node {
StmtExpr ( ref expr , _ ) | StmtSemi ( ref expr , _ ) = > Some ( expr ) ,
_ = > None ,
}
}
2015-10-02 02:55:34 -05:00
_ = > None ,
2015-08-29 04:41:06 -05:00
}
}
/// Return true if expr contains a single break expr (maybe within a block).
fn is_break_expr ( expr : & Expr ) -> bool {
match expr . node {
ExprBreak ( None ) = > true ,
2015-09-27 02:39:42 -05:00
// there won't be a `let <pat> = break` and so we can safely ignore the StmtDecl case
2016-01-03 22:26:12 -06:00
ExprBlock ( ref b ) = > {
match extract_first_expr ( b ) {
Some ( ref subexpr ) = > is_break_expr ( subexpr ) ,
None = > false ,
}
}
2015-08-29 04:41:06 -05:00
_ = > false ,
}
}
2015-08-23 12:25:45 -05:00
// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
// incremented exactly once in the loop body, and initialized to zero
// at the start of the loop.
#[ derive(PartialEq) ]
enum VarState {
2016-01-03 22:26:12 -06:00
Initial , // Not examined yet
IncrOnce , // Incremented exactly once, may be a loop counter
Declared , // Declared but not (yet) initialized to zero
2015-08-23 12:25:45 -05:00
Warn ,
2016-01-03 22:26:12 -06:00
DontWarn ,
2015-08-23 12:25:45 -05:00
}
// Scan a for loop for variables that are incremented exactly once.
struct IncrementVisitor < ' v , ' t : ' v > {
2016-01-03 22:26:12 -06:00
cx : & ' v LateContext < ' v , ' t > , // context reference
states : HashMap < NodeId , VarState > , // incremented variables
depth : u32 , // depth of conditional expressions
done : bool ,
2015-08-23 12:25:45 -05:00
}
impl < ' v , ' t > Visitor < ' v > for IncrementVisitor < ' v , ' t > {
fn visit_expr ( & mut self , expr : & ' v Expr ) {
if self . done {
return ;
}
// If node is a variable
if let Some ( def_id ) = var_def_id ( self . cx , expr ) {
if let Some ( parent ) = get_parent_expr ( self . cx , expr ) {
let state = self . states . entry ( def_id ) . or_insert ( VarState ::Initial ) ;
match parent . node {
2016-01-03 22:26:12 -06:00
ExprAssignOp ( op , ref lhs , ref rhs ) = > {
2015-08-23 12:25:45 -05:00
if lhs . id = = expr . id {
2015-09-04 08:26:58 -05:00
if op . node = = BiAdd & & is_integer_literal ( rhs , 1 ) {
2015-08-23 12:25:45 -05:00
* state = match * state {
VarState ::Initial if self . depth = = 0 = > VarState ::IncrOnce ,
2016-01-03 22:26:12 -06:00
_ = > VarState ::DontWarn ,
2015-08-23 12:25:45 -05:00
} ;
2016-01-03 22:26:12 -06:00
} else {
2015-08-23 12:25:45 -05:00
// Assigned some other value
* state = VarState ::DontWarn ;
}
2016-01-03 22:26:12 -06:00
}
}
2015-08-23 12:25:45 -05:00
ExprAssign ( ref lhs , _ ) if lhs . id = = expr . id = > * state = VarState ::DontWarn ,
2016-01-03 22:26:12 -06:00
ExprAddrOf ( mutability , _ ) if mutability = = MutMutable = > * state = VarState ::DontWarn ,
_ = > ( ) ,
2015-08-23 12:25:45 -05:00
}
}
2016-01-03 22:26:12 -06:00
} else if is_loop ( expr ) {
2015-08-23 12:25:45 -05:00
self . states . clear ( ) ;
self . done = true ;
return ;
2016-01-03 22:26:12 -06:00
} else if is_conditional ( expr ) {
2015-08-23 12:25:45 -05:00
self . depth + = 1 ;
walk_expr ( self , expr ) ;
self . depth - = 1 ;
return ;
}
walk_expr ( self , expr ) ;
}
}
// Check whether a variable is initialized to zero at the start of a loop.
struct InitializeVisitor < ' v , ' t : ' v > {
2015-09-18 21:53:04 -05:00
cx : & ' v LateContext < ' v , ' t > , // context reference
2016-01-03 22:26:12 -06:00
end_expr : & ' v Expr , // the for loop. Stop scanning here.
2015-08-23 12:25:45 -05:00
var_id : NodeId ,
state : VarState ,
name : Option < Name > ,
2016-01-03 22:26:12 -06:00
depth : u32 , // depth of conditional expressions
past_loop : bool ,
2015-08-23 12:25:45 -05:00
}
impl < ' v , ' t > Visitor < ' v > for InitializeVisitor < ' v , ' t > {
fn visit_decl ( & mut self , decl : & ' v Decl ) {
// Look for declarations of the variable
if let DeclLocal ( ref local ) = decl . node {
if local . pat . id = = self . var_id {
if let PatIdent ( _ , ref ident , _ ) = local . pat . node {
self . name = Some ( ident . node . name ) ;
self . state = if let Some ( ref init ) = local . init {
2015-09-04 08:26:58 -05:00
if is_integer_literal ( init , 0 ) {
2015-08-23 12:25:45 -05:00
VarState ::Warn
} else {
VarState ::Declared
}
2016-01-03 22:26:12 -06:00
} else {
2015-08-23 12:25:45 -05:00
VarState ::Declared
}
}
}
}
walk_decl ( self , decl ) ;
}
fn visit_expr ( & mut self , expr : & ' v Expr ) {
2015-11-25 17:09:01 -06:00
if self . state = = VarState ::DontWarn {
return ;
}
if expr = = self . end_expr {
self . past_loop = true ;
return ;
2015-08-23 12:25:45 -05:00
}
// No need to visit expressions before the variable is
2015-11-25 17:09:01 -06:00
// declared
if self . state = = VarState ::IncrOnce {
2015-08-23 12:25:45 -05:00
return ;
}
// If node is the desired variable, see how it's used
if var_def_id ( self . cx , expr ) = = Some ( self . var_id ) {
if let Some ( parent ) = get_parent_expr ( self . cx , expr ) {
match parent . node {
ExprAssignOp ( _ , ref lhs , _ ) if lhs . id = = expr . id = > {
self . state = VarState ::DontWarn ;
2015-11-16 22:39:42 -06:00
}
2015-08-23 12:25:45 -05:00
ExprAssign ( ref lhs , ref rhs ) if lhs . id = = expr . id = > {
2015-09-04 08:26:58 -05:00
self . state = if is_integer_literal ( rhs , 0 ) & & self . depth = = 0 {
2015-08-23 12:25:45 -05:00
VarState ::Warn
} else {
VarState ::DontWarn
2016-01-03 22:26:12 -06:00
}
}
ExprAddrOf ( mutability , _ ) if mutability = = MutMutable = > self . state = VarState ::DontWarn ,
_ = > ( ) ,
2015-08-23 12:25:45 -05:00
}
}
2015-11-25 17:09:01 -06:00
if self . past_loop {
self . state = VarState ::DontWarn ;
return ;
}
2016-01-03 22:26:12 -06:00
} else if ! self . past_loop & & is_loop ( expr ) {
2015-08-23 12:25:45 -05:00
self . state = VarState ::DontWarn ;
return ;
2016-01-03 22:26:12 -06:00
} else if is_conditional ( expr ) {
2015-08-23 12:25:45 -05:00
self . depth + = 1 ;
walk_expr ( self , expr ) ;
self . depth - = 1 ;
return ;
}
walk_expr ( self , expr ) ;
}
}
2015-09-18 21:53:04 -05:00
fn var_def_id ( cx : & LateContext , expr : & Expr ) -> Option < NodeId > {
2015-08-23 12:25:45 -05:00
if let Some ( path_res ) = cx . tcx . def_map . borrow ( ) . get ( & expr . id ) {
2016-01-22 06:24:44 -06:00
if let Def ::Local ( _ , node_id ) = path_res . base_def {
2016-01-03 22:26:12 -06:00
return Some ( node_id ) ;
2015-08-23 12:25:45 -05:00
}
}
None
}
fn is_loop ( expr : & Expr ) -> bool {
match expr . node {
2016-01-03 22:26:12 -06:00
ExprLoop ( .. ) | ExprWhile ( .. ) = > true ,
_ = > false ,
2015-08-23 12:25:45 -05:00
}
}
fn is_conditional ( expr : & Expr ) -> bool {
match expr . node {
ExprIf ( .. ) | ExprMatch ( .. ) = > true ,
2016-01-03 22:26:12 -06:00
_ = > false ,
2015-08-23 12:25:45 -05:00
}
}