mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-24 07:44:10 +00:00
Merge pull request #549 from mcarton/ifs
Remove and lint about useless braces in `else { if .. }`
This commit is contained in:
commit
4c14d98c9f
@ -24,7 +24,7 @@ name
|
||||
[chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char
|
||||
[cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended)
|
||||
[cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
|
||||
[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
|
||||
[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` and an `else { if .. } expression can be collapsed to `else if`
|
||||
[cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions
|
||||
[deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver
|
||||
[derive_hash_not_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_not_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly
|
||||
|
@ -148,11 +148,10 @@ fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, mask_value:
|
||||
mask_value,
|
||||
cmp_value));
|
||||
}
|
||||
} else {
|
||||
if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
} else if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
|
||||
}
|
||||
BiBitOr => {
|
||||
if mask_value | cmp_value != cmp_value {
|
||||
@ -177,10 +176,8 @@ fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, mask_value:
|
||||
&format!("incompatible bit mask: `_ & {}` will always be lower than `{}`",
|
||||
mask_value,
|
||||
cmp_value));
|
||||
} else {
|
||||
if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
} else if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
}
|
||||
BiBitOr => {
|
||||
@ -209,10 +206,8 @@ fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, mask_value:
|
||||
&format!("incompatible bit mask: `_ & {}` will never be higher than `{}`",
|
||||
mask_value,
|
||||
cmp_value));
|
||||
} else {
|
||||
if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
} else if mask_value == 0 {
|
||||
span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
|
||||
}
|
||||
}
|
||||
BiBitOr => {
|
||||
|
@ -16,9 +16,11 @@ use rustc::lint::*;
|
||||
use rustc_front::hir::*;
|
||||
use syntax::codemap::Spanned;
|
||||
|
||||
use utils::{in_macro, span_help_and_lint, snippet, snippet_block};
|
||||
use utils::{in_macro, snippet, snippet_block, span_lint_and_then};
|
||||
|
||||
/// **What it does:** This lint checks for nested `if`-statements which can be collapsed by `&&`-combining their conditions. It is `Warn` by default.
|
||||
/// **What it does:** This lint checks for nested `if`-statements which can be collapsed by
|
||||
/// `&&`-combining their conditions and for `else { if .. } expressions that can be collapsed to
|
||||
/// `else if ..`. It is `Warn` by default.
|
||||
///
|
||||
/// **Why is this bad?** Each `if`-statement adds one level of nesting, which makes code look more complex than it really is.
|
||||
///
|
||||
@ -29,7 +31,8 @@ declare_lint! {
|
||||
pub COLLAPSIBLE_IF,
|
||||
Warn,
|
||||
"two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` \
|
||||
can be written as `if x && y { foo() }`"
|
||||
can be written as `if x && y { foo() }` and an `else { if .. } expression can be collapsed to \
|
||||
`else if`"
|
||||
}
|
||||
|
||||
#[derive(Copy,Clone)]
|
||||
@ -50,20 +53,44 @@ impl LateLintPass for CollapsibleIf {
|
||||
}
|
||||
|
||||
fn check_if(cx: &LateContext, e: &Expr) {
|
||||
if let ExprIf(ref check, ref then, None) = e.node {
|
||||
if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), span: sp, ..}) =
|
||||
single_stmt_of_block(then) {
|
||||
if e.span.expn_id != sp.expn_id {
|
||||
return;
|
||||
if let ExprIf(ref check, ref then, ref else_) = e.node {
|
||||
match *else_ {
|
||||
Some(ref else_) => {
|
||||
if_let_chain! {[
|
||||
let ExprBlock(ref block) = else_.node,
|
||||
block.stmts.is_empty(),
|
||||
block.rules == BlockCheckMode::DefaultBlock,
|
||||
let Some(ref else_) = block.expr,
|
||||
let ExprIf(_, _, _) = else_.node
|
||||
], {
|
||||
span_lint_and_then(cx,
|
||||
COLLAPSIBLE_IF,
|
||||
block.span,
|
||||
"this `else { if .. }` block can be collapsed", |db| {
|
||||
db.span_suggestion(block.span, "try",
|
||||
format!("else {}",
|
||||
snippet_block(cx, else_.span, "..")));
|
||||
});
|
||||
}}
|
||||
}
|
||||
None => {
|
||||
if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), span: sp, ..}) =
|
||||
single_stmt_of_block(then) {
|
||||
if e.span.expn_id != sp.expn_id {
|
||||
return;
|
||||
}
|
||||
span_lint_and_then(cx,
|
||||
COLLAPSIBLE_IF,
|
||||
e.span,
|
||||
"this if statement can be collapsed", |db| {
|
||||
db.span_suggestion(e.span, "try",
|
||||
format!("if {} && {} {}",
|
||||
check_to_string(cx, check),
|
||||
check_to_string(cx, check_inner),
|
||||
snippet_block(cx, content.span, "..")));
|
||||
});
|
||||
}
|
||||
}
|
||||
span_help_and_lint(cx,
|
||||
COLLAPSIBLE_IF,
|
||||
e.span,
|
||||
"this if statement can be collapsed",
|
||||
&format!("try\nif {} && {} {}",
|
||||
check_to_string(cx, check),
|
||||
check_to_string(cx, check_inner),
|
||||
snippet_block(cx, content.span, "..")));
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -90,16 +117,14 @@ fn single_stmt_of_block(block: &Block) -> Option<&Expr> {
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
if block.stmts.is_empty() {
|
||||
if let Some(ref p) = block.expr {
|
||||
Some(p)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else if block.stmts.is_empty() {
|
||||
if let Some(ref p) = block.expr {
|
||||
Some(p)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -536,12 +536,10 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
|
||||
Plus
|
||||
})
|
||||
.and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty)))
|
||||
} else if ln {
|
||||
add_neg_int(r64, rty, l64, lty)
|
||||
} else {
|
||||
if ln {
|
||||
add_neg_int(r64, rty, l64, lty)
|
||||
} else {
|
||||
add_neg_int(l64, lty, r64, rty)
|
||||
}
|
||||
add_neg_int(l64, lty, r64, rty)
|
||||
}
|
||||
}
|
||||
// TODO: float (would need bignum library?)
|
||||
|
@ -59,12 +59,10 @@ fn min_max<'a>(cx: &LateContext, expr: &'a Expr) -> Option<(MinMax, Constant, &'
|
||||
|
||||
if match_def_path(cx, def_id, &["core", "cmp", "min"]) {
|
||||
fetch_const(args, Min)
|
||||
} else if match_def_path(cx, def_id, &["core", "cmp", "max"]) {
|
||||
fetch_const(args, Max)
|
||||
} else {
|
||||
if match_def_path(cx, def_id, &["core", "cmp", "max"]) {
|
||||
fetch_const(args, Max)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
|
@ -204,29 +204,28 @@ fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &
|
||||
snippet(cx, lspan, "_"),
|
||||
snippet(cx, expr.span, "..")));
|
||||
note_orig(cx, db, SHADOW_SAME, prev_span);
|
||||
} else if contains_self(name, expr) {
|
||||
let db = span_note_and_lint(cx,
|
||||
SHADOW_REUSE,
|
||||
lspan,
|
||||
&format!("{} is shadowed by {} which reuses the original value",
|
||||
snippet(cx, lspan, "_"),
|
||||
snippet(cx, expr.span, "..")),
|
||||
expr.span,
|
||||
"initialization happens here");
|
||||
note_orig(cx, db, SHADOW_REUSE, prev_span);
|
||||
} else {
|
||||
if contains_self(name, expr) {
|
||||
let db = span_note_and_lint(cx,
|
||||
SHADOW_REUSE,
|
||||
lspan,
|
||||
&format!("{} is shadowed by {} which reuses the original value",
|
||||
snippet(cx, lspan, "_"),
|
||||
snippet(cx, expr.span, "..")),
|
||||
expr.span,
|
||||
"initialization happens here");
|
||||
note_orig(cx, db, SHADOW_REUSE, prev_span);
|
||||
} else {
|
||||
let db = span_note_and_lint(cx,
|
||||
SHADOW_UNRELATED,
|
||||
lspan,
|
||||
&format!("{} is shadowed by {}",
|
||||
snippet(cx, lspan, "_"),
|
||||
snippet(cx, expr.span, "..")),
|
||||
expr.span,
|
||||
"initialization happens here");
|
||||
note_orig(cx, db, SHADOW_UNRELATED, prev_span);
|
||||
}
|
||||
let db = span_note_and_lint(cx,
|
||||
SHADOW_UNRELATED,
|
||||
lspan,
|
||||
&format!("{} is shadowed by {}",
|
||||
snippet(cx, lspan, "_"),
|
||||
snippet(cx, expr.span, "..")),
|
||||
expr.span,
|
||||
"initialization happens here");
|
||||
note_orig(cx, db, SHADOW_UNRELATED, prev_span);
|
||||
}
|
||||
|
||||
} else {
|
||||
let db = span_lint(cx,
|
||||
SHADOW_UNRELATED,
|
||||
|
@ -17,6 +17,30 @@ fn main() {
|
||||
}
|
||||
}
|
||||
|
||||
// Collaspe `else { if .. }` to `else if ..`
|
||||
if x == "hello" {
|
||||
print!("Hello ");
|
||||
} else { //~ERROR: this `else { if .. }`
|
||||
//~| HELP try
|
||||
//~| SUGGESTION else if y == "world"
|
||||
if y == "world" {
|
||||
println!("world!")
|
||||
}
|
||||
}
|
||||
|
||||
if x == "hello" {
|
||||
print!("Hello ");
|
||||
} else { //~ERROR this `else { if .. }`
|
||||
//~| HELP try
|
||||
//~| SUGGESTION else if y == "world"
|
||||
if y == "world" {
|
||||
println!("world")
|
||||
}
|
||||
else {
|
||||
println!("!")
|
||||
}
|
||||
}
|
||||
|
||||
// Works because any if with an else statement cannot be collapsed.
|
||||
if x == "hello" {
|
||||
if y == "world" {
|
||||
|
Loading…
Reference in New Issue
Block a user