Skip to content

Commit

Permalink
Add lint for ifs that could be collapsed
Browse files Browse the repository at this point in the history
"Collapsible" ifs are ones which contain only a then block, and the then
block consists of an if that only has a then block.
  • Loading branch information
mattyhall committed May 29, 2015
1 parent eb421ca commit 7e16822
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 0 deletions.
80 changes: 80 additions & 0 deletions src/collapsible_if.rs
@@ -0,0 +1,80 @@
//! Checks for if expressions that contain only an if expression.
//!
//! For example, the lint would catch:
//!
//! ```
//! if x {
//! if y {
//! println!("Hello world");
//! }
//! }
//! ```
//!
//! This lint is **warn** by default

use rustc::plugin::Registry;
use rustc::lint::*;
use rustc::middle::def::*;
use syntax::ast::*;
use syntax::ptr::P;
use syntax::codemap::{Span, Spanned};
use syntax::print::pprust::expr_to_string;

declare_lint! {
pub COLLAPSIBLE_IF,
Warn,
"Warn on if expressions that can be collapsed"
}

#[derive(Copy,Clone)]
pub struct CollapsibleIf;

impl LintPass for CollapsibleIf {
fn get_lints(&self) -> LintArray {
lint_array!(COLLAPSIBLE_IF)
}

fn check_expr(&mut self, cx: &Context, e: &Expr) {
if let ExprIf(ref check, ref then_block, None) = e.node {
let expr = check_block(then_block);
let expr = match expr {
Some(e) => e,
None => return
};
if let ExprIf(ref check_inner, _, None) = expr.node {
let (check, check_inner) = (check_to_string(check), check_to_string(check_inner));
cx.span_lint(COLLAPSIBLE_IF, e.span,
&format!("This if statement can be collapsed. Try: if {} && {}", check, check_inner));
}
}
}
}

fn requires_brackets(e: &Expr) -> bool {
match e.node {
ExprBinary(Spanned {node: n, ..}, _, _) if n == BiEq => false,
_ => true
}
}

fn check_to_string(e: &Expr) -> String {
if requires_brackets(e) {
format!("({})", expr_to_string(e))
} else {
format!("{}", expr_to_string(e))
}
}

fn check_block(b: &Block) -> Option<&P<Expr>> {
if b.stmts.len() == 1 && b.expr.is_none() {
let stmt = &b.stmts[0];
return match stmt.node {
StmtExpr(ref e, _) => Some(e),
_ => None
};
}
if let Some(ref e) = b.expr {
return Some(e);
}
None
}
3 changes: 3 additions & 0 deletions src/lib.rs
Expand Up @@ -25,6 +25,7 @@ pub mod eta_reduction;
pub mod identity_op;
pub mod mut_mut;
pub mod len_zero;
pub mod collapsible_if;

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
Expand All @@ -45,6 +46,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box mut_mut::MutMut as LintPassObject);
reg.register_lint_pass(box len_zero::LenZero as LintPassObject);
reg.register_lint_pass(box misc::CmpOwned as LintPassObject);
reg.register_lint_pass(box collapsible_if::CollapsibleIf as LintPassObject);

reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
misc::SINGLE_MATCH, misc::STR_TO_STRING,
Expand All @@ -61,5 +63,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
mut_mut::MUT_MUT,
len_zero::LEN_ZERO,
len_zero::LEN_WITHOUT_IS_EMPTY,
collapsible_if::COLLAPSIBLE_IF,
]);
}
37 changes: 37 additions & 0 deletions tests/compile-fail/collapsible_if.rs
@@ -0,0 +1,37 @@
#![feature(plugin)]
#![plugin(clippy)]

#[deny(collapsible_if)]
fn main() {
let x = "hello";
let y = "world";
if x == "hello" { //~ERROR This if statement can be collapsed
if y == "world" {
println!("Hello world!");
}
}

if x == "hello" || x == "world" { //~ERROR This if statement can be collapsed
if y == "world" || y == "hello" {
println!("Hello world!");
}
}

// Works because any if with an else statement cannot be collapsed.
if x == "hello" {
if y == "world" {
println!("Hello world!");
}
} else {
println!("Not Hello world");
}

if x == "hello" {
if y == "world" {
println!("Hello world!");
} else {
println!("Hello something else");
}
}

}

0 comments on commit 7e16822

Please sign in to comment.