[needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice
[needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `Foo { ..base }` when there are no missing fields
[neg_multiply](https://github.com/Manishearth/rust-clippy/wiki#neg_multiply) | warn | multiplying integers with -1
-[never_loop](https://github.com/Manishearth/rust-clippy/wiki#never_loop) | allow | any loop with an unconditional `break` or `return` statement
+[never_loop](https://github.com/Manishearth/rust-clippy/wiki#never_loop) | warn | any loop that will always `break` or `return`
[new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self) | warn | not returning `Self` in a `new` method
[new_without_default](https://github.com/Manishearth/rust-clippy/wiki#new_without_default) | warn | `fn new() -> Self` method without `Default` implementation
[new_without_default_derive](https://github.com/Manishearth/rust-clippy/wiki#new_without_default_derive) | warn | `fn new() -> Self` without `#[derive]`able `Default` implementation
}
fn is_relevant_block(tcx: TyCtxt, tables: &ty::TypeckTables, block: &Block) -> bool {
- for stmt in &block.stmts {
+ if let Some(stmt) = block.stmts.first() {
match stmt.node {
- StmtDecl(_, _) => return true,
+ StmtDecl(_, _) => true,
StmtExpr(ref expr, _) |
- StmtSemi(ref expr, _) => {
- return is_relevant_expr(tcx, tables, expr);
- },
+ StmtSemi(ref expr, _) => is_relevant_expr(tcx, tables, expr),
}
+ } else {
+ block.expr.as_ref().map_or(false, |e| is_relevant_expr(tcx, tables, e))
}
- block.expr.as_ref().map_or(false, |e| is_relevant_expr(tcx, tables, e))
}
fn is_relevant_expr(tcx: TyCtxt, tables: &ty::TypeckTables, expr: &Expr) -> bool {
type Item = (bool, char);
fn next(&mut self) -> Option<(bool, char)> {
- while self.line < self.docs.len() {
+ if self.line < self.docs.len() {
if self.reset {
self.line += 1;
self.reset = false;
self.pos += c.len_utf8();
let new_line = self.new_line;
self.new_line = c == '\n' || (self.new_line && c.is_whitespace());
- return Some((new_line, c));
+ Some((new_line, c))
} else if self.line == self.docs.len() - 1 {
- return None;
+ None
} else {
self.new_line = true;
self.reset = true;
self.pos += 1;
- return Some((true, '\n'));
+ Some((true, '\n'))
}
+ } else {
+ None
}
-
- None
}
}
enum_variants::STUTTER,
if_not_else::IF_NOT_ELSE,
items_after_statements::ITEMS_AFTER_STATEMENTS,
- loops::NEVER_LOOP,
matches::SINGLE_MATCH_ELSE,
mem_forget::MEM_FORGET,
methods::FILTER_MAP,
loops::FOR_LOOP_OVER_RESULT,
loops::ITER_NEXT_LOOP,
loops::NEEDLESS_RANGE_LOOP,
+ loops::NEVER_LOOP,
loops::REVERSE_RANGE_LOOP,
loops::UNUSED_COLLECT,
loops::WHILE_LET_LOOP,
"looping on a map using `iter` when `keys` or `values` would do"
}
-/// **What it does:** Checks for loops that contain an unconditional `break`
-/// or `return`.
+/// **What it does:** Checks for loops that will always `break`, `return` or
+/// `continue` an outer loop.
///
/// **Why is this bad?** This loop never loops, all it does is obfuscating the
/// code.
///
-/// **Known problems:** Ignores `continue` statements in the loop that create
-/// nontrivial control flow. Therefore set to `Allow` by default.
-/// See https://github.com/Manishearth/rust-clippy/issues/1586
+/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// ```
declare_lint! {
pub NEVER_LOOP,
- Allow,
- "any loop with an unconditional `break` or `return` statement"
+ Warn,
+ "any loop that will always `break` or `return`"
}
#[derive(Copy, Clone)]
if let Some((pat, arg, body)) = higher::for_loop(expr) {
check_for_loop(cx, pat, arg, body, expr);
}
+
+ // check for never_loop
+ match expr.node {
+ ExprWhile(_, ref block, _) |
+ ExprLoop(ref block, _, _) => {
+ if never_loop(block, &expr.id) {
+ span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops");
+ }
+ },
+ _ => (),
+ }
+
// check for `loop { if let {} else break }` that could be `while let`
// (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration)
"empty `loop {}` detected. You may want to either use `panic!()` or add \
`std::thread::sleep(..);` to the loop body.");
}
- if never_loop_block(block) {
- span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops");
- }
// extract the expression from the first statement (if any) in a block
let inner_stmt_expr = extract_expr_from_first_stmt(block);
}
}
-fn never_loop_block(block: &Block) -> bool {
- block.stmts.iter().any(never_loop_stmt) || block.expr.as_ref().map_or(false, |e| never_loop_expr(e))
+fn never_loop(block: &Block, id: &NodeId) -> bool {
+ !contains_continue_block(block, id) && loop_exit_block(block)
+}
+
+fn contains_continue_block(block: &Block, dest: &NodeId) -> bool {
+ block.stmts.iter().any(|e| contains_continue_stmt(e, dest))
+ || block.expr.as_ref().map_or(false, |e| contains_continue_expr(e, dest))
}
-fn never_loop_stmt(stmt: &Stmt) -> bool {
+fn contains_continue_stmt(stmt: &Stmt, dest: &NodeId) -> bool {
match stmt.node {
StmtSemi(ref e, _) |
- StmtExpr(ref e, _) => never_loop_expr(e),
- StmtDecl(ref d, _) => never_loop_decl(d),
+ StmtExpr(ref e, _) => contains_continue_expr(e, dest),
+ StmtDecl(ref d, _) => contains_continue_decl(d, dest),
}
}
-fn never_loop_decl(decl: &Decl) -> bool {
- if let DeclLocal(ref local) = decl.node {
- local.init.as_ref().map_or(false, |e| never_loop_expr(e))
- } else {
- false
+fn contains_continue_decl(decl: &Decl, dest: &NodeId) -> bool {
+ match decl.node {
+ DeclLocal(ref local) => local.init.as_ref().map_or(false, |e| contains_continue_expr(e, dest)),
+ _ => false
}
}
-fn never_loop_expr(expr: &Expr) -> bool {
+fn contains_continue_expr(expr: &Expr, dest: &NodeId) -> bool {
match expr.node {
- ExprBreak(..) | ExprRet(..) => true,
ExprBox(ref e) |
ExprUnary(_, ref e) |
- ExprBinary(_, ref e, _) | // because short-circuiting
ExprCast(ref e, _) |
ExprType(ref e, _) |
ExprField(ref e, _) |
ExprTupField(ref e, _) |
- ExprRepeat(ref e, _) |
- ExprAddrOf(_, ref e) => never_loop_expr(e),
+ ExprAddrOf(_, ref e) |
+ ExprRepeat(ref e, _) => contains_continue_expr(e, dest),
+ ExprArray(ref es) |
+ ExprMethodCall(_, _, ref es) |
+ ExprTup(ref es) => es.iter().any(|e| contains_continue_expr(e, dest)),
+ ExprCall(ref e, ref es) => contains_continue_expr(e, dest) || es.iter().any(|e| contains_continue_expr(e, dest)),
+ ExprBinary(_, ref e1, ref e2) |
ExprAssign(ref e1, ref e2) |
ExprAssignOp(_, ref e1, ref e2) |
- ExprIndex(ref e1, ref e2) => never_loop_expr(e1) || never_loop_expr(e2),
+ ExprIndex(ref e1, ref e2) => [e1, e2].iter().any(|e| contains_continue_expr(e, dest)),
+ ExprIf(ref e, ref e2, ref e3) => [e, e2].iter().chain(e3.as_ref().iter()).any(|e| contains_continue_expr(e, dest)),
+ ExprWhile(ref e, ref b, _) => contains_continue_expr(e, dest) || contains_continue_block(b, dest),
+ ExprMatch(ref e, ref arms, _) => contains_continue_expr(e, dest) || arms.iter().any(|a| contains_continue_expr(&a.body, dest)),
+ ExprBlock(ref block) => contains_continue_block(block, dest),
+ ExprStruct(_, _, ref base) => base.as_ref().map_or(false, |e| contains_continue_expr(e, dest)),
+ ExprAgain(d) => d.target_id.opt_id().map_or(false, |id| id == *dest),
+ _ => false,
+ }
+}
+
+fn loop_exit_block(block: &Block) -> bool {
+ block.stmts.iter().any(|e| loop_exit_stmt(e))
+ || block.expr.as_ref().map_or(false, |e| loop_exit_expr(e))
+}
+
+fn loop_exit_stmt(stmt: &Stmt) -> bool {
+ match stmt.node {
+ StmtSemi(ref e, _) |
+ StmtExpr(ref e, _) => loop_exit_expr(e),
+ StmtDecl(ref d, _) => loop_exit_decl(d),
+ }
+}
+
+fn loop_exit_decl(decl: &Decl) -> bool {
+ match decl.node {
+ DeclLocal(ref local) => local.init.as_ref().map_or(false, |e| loop_exit_expr(e)),
+ _ => false
+ }
+}
+
+fn loop_exit_expr(expr: &Expr) -> bool {
+ match expr.node {
+ ExprBox(ref e) |
+ ExprUnary(_, ref e) |
+ ExprCast(ref e, _) |
+ ExprType(ref e, _) |
+ ExprField(ref e, _) |
+ ExprTupField(ref e, _) |
+ ExprAddrOf(_, ref e) |
+ ExprRepeat(ref e, _) => loop_exit_expr(e),
ExprArray(ref es) |
- ExprTup(ref es) |
- ExprMethodCall(_, _, ref es) => es.iter().any(|e| never_loop_expr(e)),
- ExprCall(ref e, ref es) => never_loop_expr(e) || es.iter().any(|e| never_loop_expr(e)),
- ExprBlock(ref block) => never_loop_block(block),
- ExprStruct(_, _, ref base) => base.as_ref().map_or(false, |e| never_loop_expr(e)),
+ ExprMethodCall(_, _, ref es) |
+ ExprTup(ref es) => es.iter().any(|e| loop_exit_expr(e)),
+ ExprCall(ref e, ref es) => loop_exit_expr(e) || es.iter().any(|e| loop_exit_expr(e)),
+ ExprBinary(_, ref e1, ref e2) |
+ ExprAssign(ref e1, ref e2) |
+ ExprAssignOp(_, ref e1, ref e2) |
+ ExprIndex(ref e1, ref e2) => [e1, e2].iter().any(|e| loop_exit_expr(e)),
+ ExprIf(ref e, ref e2, ref e3) => loop_exit_expr(e) || e3.as_ref().map_or(false, |e| loop_exit_expr(e)) && loop_exit_expr(e2),
+ ExprWhile(ref e, ref b, _) => loop_exit_expr(e) || loop_exit_block(b),
+ ExprMatch(ref e, ref arms, _) => loop_exit_expr(e) || arms.iter().all(|a| loop_exit_expr(&a.body)),
+ ExprBlock(ref b) => loop_exit_block(b),
+ ExprBreak(_, _) |
+ ExprAgain(_) |
+ ExprRet(_) => true,
_ => false,
}
}
db.span_suggestion(
lit.span,
"if you mean to use a decimal constant, remove the `0` to remove confusion:",
- src[1..].to_string(),
+ src.trim_left_matches('0').to_string(),
);
db.span_suggestion(
lit.span,
"if you mean to use an octal constant, use `0o`:",
- format!("0o{}", &src[1..]),
+ format!("0o{}", src.trim_left_matches('0')),
);
});
}
use syntax::codemap::{Span, Spanned};
use syntax::visit::FnKind;
-use utils::{span_note_and_lint, span_lint_and_then, snippet_opt, match_path_ast, in_external_macro};
+use utils::{span_note_and_lint, span_lint_and_then, snippet_opt, match_path_ast, in_macro,
+ in_external_macro};
/// **What it does:** Checks for return statements at the end of a block.
///
}
fn emit_return_lint(&mut self, cx: &EarlyContext, ret_span: Span, inner_span: Span) {
- if in_external_macro(cx, inner_span) {
+ if in_external_macro(cx, inner_span) || in_macro(inner_span) {
return;
}
span_lint_and_then(cx,
= note: `-D for-loop-over-result` implied by `-D warnings`
= help: consider replacing `for x in v.iter().next().ok_or("x not found")` with `if let Ok(x) = v.iter().next().ok_or("x not found")`
+error: this loop never actually loops
+ --> for_loop.rs:52:5
+ |
+52 | / while let Some(x) = option {
+53 | | println!("{}", x);
+54 | | break;
+55 | | }
+ | |_____^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+ --> for_loop.rs:58:5
+ |
+58 | / while let Ok(x) = result {
+59 | | println!("{}", x);
+60 | | break;
+61 | | }
+ | |_____^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
error: the loop variable `i` is only used to index `vec`.
--> for_loop.rs:84:5
|
let fail1 = 0xabCD;
let fail2 = 0xabCD_u32;
let fail2 = 0xabCD_isize;
+ let fail_multi_zero = 000123usize;
let ok6 = 1234_i32;
let ok7 = 1234_f32;
= note: `-D mixed-case-hex-literals` implied by `-D warnings`
error: integer type suffix should be separated by an underscore
- --> literals.rs:21:17
+ --> literals.rs:17:27
|
-21 | let fail3 = 1234i32;
- | ^^^^^^^
+17 | let fail_multi_zero = 000123usize;
+ | ^^^^^^^^^^^
|
= note: `-D unseparated-literal-suffix` implied by `-D warnings`
+error: this is a decimal constant
+ --> literals.rs:17:27
+ |
+17 | let fail_multi_zero = 000123usize;
+ | ^^^^^^^^^^^
+ |
+ = note: `-D zero-prefixed-literal` implied by `-D warnings`
+help: if you mean to use a decimal constant, remove the `0` to remove confusion:
+ | let fail_multi_zero = 123usize;
+help: if you mean to use an octal constant, use `0o`:
+ | let fail_multi_zero = 0o123usize;
+
error: integer type suffix should be separated by an underscore
--> literals.rs:22:17
|
-22 | let fail4 = 1234u32;
+22 | let fail3 = 1234i32;
| ^^^^^^^
|
= note: `-D unseparated-literal-suffix` implied by `-D warnings`
error: integer type suffix should be separated by an underscore
--> literals.rs:23:17
|
-23 | let fail5 = 1234isize;
- | ^^^^^^^^^
+23 | let fail4 = 1234u32;
+ | ^^^^^^^
|
= note: `-D unseparated-literal-suffix` implied by `-D warnings`
error: integer type suffix should be separated by an underscore
--> literals.rs:24:17
|
-24 | let fail6 = 1234usize;
+24 | let fail5 = 1234isize;
| ^^^^^^^^^
|
= note: `-D unseparated-literal-suffix` implied by `-D warnings`
-error: float type suffix should be separated by an underscore
+error: integer type suffix should be separated by an underscore
--> literals.rs:25:17
|
-25 | let fail7 = 1.5f32;
+25 | let fail6 = 1234usize;
+ | ^^^^^^^^^
+ |
+ = note: `-D unseparated-literal-suffix` implied by `-D warnings`
+
+error: float type suffix should be separated by an underscore
+ --> literals.rs:26:17
+ |
+26 | let fail7 = 1.5f32;
| ^^^^^^
|
= note: `-D unseparated-literal-suffix` implied by `-D warnings`
error: this is a decimal constant
- --> literals.rs:29:17
+ --> literals.rs:30:17
|
-29 | let fail8 = 0123;
+30 | let fail8 = 0123;
| ^^^^
|
= note: `-D zero-prefixed-literal` implied by `-D warnings`
#![feature(plugin)]
#![plugin(clippy)]
+#![allow(single_match, unused_assignments, unused_variables)]
-#![warn(never_loop)]
-#![allow(dead_code, unused)]
-
-fn main() {
- loop {
- println!("This is only ever printed once");
+fn test1() {
+ let mut x = 0;
+ loop { // never_loop
+ x += 1;
+ if x == 1 {
+ return
+ }
break;
}
+}
- let x = 1;
+fn test2() {
+ let mut x = 0;
loop {
- println!("This, too"); // but that's OK
+ x += 1;
if x == 1 {
- break;
+ break
}
}
+}
+fn test3() {
+ let mut x = 0;
+ loop { // never loops
+ x += 1;
+ break
+ }
+}
+
+fn test4() {
+ let mut x = 1;
loop {
- loop {
- // another one
- break;
+ x += 1;
+ match x {
+ 5 => return,
+ _ => (),
}
- break;
}
+}
+
+fn test5() {
+ let i = 0;
+ loop { // never loops
+ while i == 0 { // never loops
+ break
+ }
+ return
+ }
+}
+
+fn test6() {
+ let mut x = 0;
+ 'outer: loop { // never loops
+ x += 1;
+ loop { // never loops
+ if x == 5 { break }
+ continue 'outer
+ }
+ return
+ }
+}
+fn test7() {
+ let mut x = 0;
loop {
- loop {
- if x == 1 { return; }
+ x += 1;
+ match x {
+ 1 => continue,
+ _ => (),
}
+ return
}
}
+
+fn test8() {
+ let mut x = 0;
+ loop {
+ x += 1;
+ match x {
+ 5 => return,
+ _ => continue,
+ }
+ }
+}
+
+fn test9() {
+ let x = Some(1);
+ while let Some(y) = x { // never loops
+ return
+ }
+}
+
+fn test10() {
+ for x in 0..10 { // never loops
+ match x {
+ 1 => break,
+ _ => return,
+ }
+ }
+}
+
+fn main() {
+ test1();
+ test2();
+ test3();
+ test4();
+ test5();
+ test6();
+ test7();
+ test8();
+ test9();
+ test10();
+}
+
error: this loop never actually loops
- --> never_loop.rs:8:5
+ --> never_loop.rs:7:5
|
-8 | / loop {
-9 | | println!("This is only ever printed once");
-10 | | break;
-11 | | }
+7 | / loop { // never_loop
+8 | | x += 1;
+9 | | if x == 1 {
+10 | | return
+11 | | }
+12 | | break;
+13 | | }
| |_____^
|
= note: `-D never-loop` implied by `-D warnings`
error: this loop never actually loops
- --> never_loop.rs:21:5
+ --> never_loop.rs:28:5
|
-21 | / loop {
-22 | | loop {
-23 | | // another one
-24 | | break;
-25 | | }
-26 | | break;
-27 | | }
+28 | / loop { // never loops
+29 | | x += 1;
+30 | | break
+31 | | }
| |_____^
|
= note: `-D never-loop` implied by `-D warnings`
error: this loop never actually loops
- --> never_loop.rs:22:9
+ --> never_loop.rs:47:2
|
-22 | / loop {
-23 | | // another one
-24 | | break;
-25 | | }
+47 | / loop { // never loops
+48 | | while i == 0 { // never loops
+49 | | break
+50 | | }
+51 | | return
+52 | | }
+ | |__^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+ --> never_loop.rs:48:9
+ |
+48 | / while i == 0 { // never loops
+49 | | break
+50 | | }
| |_________^
|
= note: `-D never-loop` implied by `-D warnings`
+error: this loop never actually loops
+ --> never_loop.rs:57:5
+ |
+57 | / 'outer: loop { // never loops
+58 | | x += 1;
+59 | | loop { // never loops
+60 | | if x == 5 { break }
+... |
+63 | | return
+64 | | }
+ | |__^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+ --> never_loop.rs:59:3
+ |
+59 | / loop { // never loops
+60 | | if x == 5 { break }
+61 | | continue 'outer
+62 | | }
+ | |___^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+ --> never_loop.rs:92:5
+ |
+92 | / while let Some(y) = x { // never loops
+93 | | return
+94 | | }
+ | |_____^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+ --> never_loop.rs:98:5
+ |
+98 | / for x in 0..10 { // never loops
+99 | | match x {
+100 | | 1 => break,
+101 | | _ => return,
+102 | | }
+103 | | }
+ | |_____^
+ |
+ = note: `-D never-loop` implied by `-D warnings`
+
error: aborting due to previous error(s)
error: Could not compile `clippy_tests`.
projects / rust.git / commitdiff