Fix wrong suggestion in WHILE_LET_LOOP

Ok, I lied in the title. This basically *removes* the problematic part
but:
  1) it was ugly with big bodies;
  2) it was not indented properly;
  3) it wasn’t very smart (see #675).
This commit is contained in:
mcarton 2016-02-27 22:59:15 +01:00
parent 98eb623043
commit 5fadfb3ea6
2 changed files with 22 additions and 25 deletions

View File

@ -11,7 +11,7 @@
use std::borrow::Cow;
use std::collections::HashMap;
use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro, expr_block,
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};
use utils::{BTREEMAP_PATH, HASHMAP_PATH, LL_PATH, OPTION_PATH, RESULT_PATH, VEC_PATH};
@ -241,20 +241,6 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
// or extract the first expression (if any) from the block
if let Some(inner) = inner_stmt_expr.or_else(|| extract_first_expr(block)) {
if let ExprMatch(ref matchexpr, ref arms, ref source) = inner.node {
// collect the remaining statements below the match
let mut other_stuff = block.stmts
.iter()
.skip(1)
.map(|stmt| snippet(cx, stmt.span, ".."))
.collect::<Vec<Cow<_>>>();
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(snippet(cx, expr.span, ".."));
}
}
// ensure "if let" compatible match structure
match *source {
MatchSource::Normal | MatchSource::IfLetDesugar{..} => {
@ -264,22 +250,20 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
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 ")), "..")
};
// NOTE: we used to make build a body here instead of using
// ellipsis, this was removed because:
// 1) it was ugly with big bodies;
// 2) it was not indented properly;
// 3) it wasnt very smart (see #675).
span_lint_and_then(cx,
WHILE_LET_LOOP,
expr.span,
"this loop could be written as a `while let` loop",
|db| {
let sug = format!("while let {} = {} {}",
let sug = format!("while let {} = {} {{ .. }}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, matchexpr.span, ".."),
loop_body);
snippet(cx, matchexpr.span, ".."));
db.span_suggestion(expr.span, "try", sug);
});
}

View File

@ -66,6 +66,19 @@ fn main() {
println!("{}", x);
}
// #675, this used to have a wrong suggestion
loop {
//~^ERROR this loop could be written as a `while let` loop
//~|HELP try
//~|SUGGESTION while let Some(word) = "".split_whitespace().next() { .. }
let (e, l) = match "".split_whitespace().next() {
Some(word) => (word.is_empty(), word.len()),
None => break
};
let _ = (e, l);
}
let mut iter = 1..20;
while let Option::Some(x) = iter.next() { //~ERROR this loop could be written as a `for` loop
println!("{}", x);