handle another type of collapsible if-statement #15027
Conversation
rustbot
added
the
S-waiting-on-review
|
|
I'll take this one as I have already #14906 on my plate. |
| @@ -163,6 +164,54 @@ impl CollapsibleIf { | |||
| } | |||
| } | |||
|
|
|||
| fn check_collapsible_if_nested_if_else(context: &LateContext<'_>, if_expr: &Expr<'_>) { | |||
There was a problem hiding this comment.
Please use the same names as throughout the rest of Clippy:
| fn check_collapsible_if_nested_if_else(context: &LateContext<'_>, if_expr: &Expr<'_>) { | |
| fn check_collapsible_if_nested_if_else(cx: &LateContext<'_>, if_expr: &Expr<'_>) { |
Also, if this doesn't require a self, you should get it out of the CollapsibleIf struct impl.
| let ExprKind::If(cond, then, else_opt) = if_expr.kind else { | ||
| return; | ||
| }; | ||
|
|
||
| let ExprKind::Block(then_block, _) = then.kind else { | ||
| return; | ||
| }; | ||
|
|
||
| let Some(then_expr) = then_block.expr else { | ||
| return; | ||
| }; | ||
|
|
||
| let ExprKind::If(inner_cond, inner_then, _) = then_expr.kind else { | ||
| return; | ||
| }; | ||
|
|
||
| let Some(else_expr) = else_opt else { | ||
| return; | ||
| }; | ||
|
|
||
| let mut spanless_eq = SpanlessEq::new(context); | ||
|
|
||
| if !spanless_eq.eq_expr(inner_then, else_expr) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Please use a if let chain instead of doing 6 early returns, this is as clear and much shorter.
| if_expr.span, | ||
| "collapse else and nested if blocks", | ||
| format!( | ||
| "if !{} || {} {}", |
There was a problem hiding this comment.
Please add a test case with the first condition being a binary operation such as s != "foobar": in this case, the ! will be applied to s which is wrong.
You should use the Sugg interface to properly invert conditions and get parentheses if needed.
| cond.span.get_source_text(context).unwrap().to_owned(), | ||
| inner_cond.span.get_source_text(context).unwrap().to_owned(), | ||
| else_expr.span.get_source_text(context).unwrap().to_owned() |
There was a problem hiding this comment.
You should use the Sugg or snippet interface with proper applicability checking.
rustbot
added
S-waiting-on-author
llogiq
changed the title
clippy to handle another type of collapsible if-statement
fixes #812
changelog: [
collapsible_if]: handle another type of collapsible if-statement