-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #4569 - james9909:add-comparison-chain, r=oli-obk
Add a new lint for comparison chains changelog: Adds a new lint: `comparison_chain`. `comparison_chain` lints all `if` conditional chains where all the conditions are binary comparisons on the same two operands and will suggest a rewrite with `match`. Closes #4531.
- Loading branch information
Showing
13 changed files
with
362 additions
and
107 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| use crate::utils::{if_sequence, parent_node_is_if_expr, span_help_and_lint, SpanlessEq}; | ||
| use rustc::hir::*; | ||
| use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; | ||
| use rustc::{declare_lint_pass, declare_tool_lint}; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks comparison chains written with `if` that can be | ||
| /// rewritten with `match` and `cmp`. | ||
| /// | ||
| /// **Why is this bad?** `if` is not guaranteed to be exhaustive and conditionals can get | ||
| /// repetitive | ||
| /// | ||
| /// **Known problems:** None. | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust,ignore | ||
| /// # fn a() {} | ||
| /// # fn b() {} | ||
| /// # fn c() {} | ||
| /// fn f(x: u8, y: u8) { | ||
| /// if x > y { | ||
| /// a() | ||
| /// } else if x < y { | ||
| /// b() | ||
| /// } else { | ||
| /// c() | ||
| /// } | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// Could be written: | ||
| /// | ||
| /// ```rust,ignore | ||
| /// use std::cmp::Ordering; | ||
| /// # fn a() {} | ||
| /// # fn b() {} | ||
| /// # fn c() {} | ||
| /// fn f(x: u8, y: u8) { | ||
| /// match x.cmp(&y) { | ||
| /// Ordering::Greater => a(), | ||
| /// Ordering::Less => b(), | ||
| /// Ordering::Equal => c() | ||
| /// } | ||
| /// } | ||
| /// ``` | ||
| pub COMPARISON_CHAIN, | ||
| style, | ||
| "`if`s that can be rewritten with `match` and `cmp`" | ||
| } | ||
|
|
||
| declare_lint_pass!(ComparisonChain => [COMPARISON_CHAIN]); | ||
|
|
||
| impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain { | ||
| fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { | ||
| if expr.span.from_expansion() { | ||
| return; | ||
| } | ||
|
|
||
| // We only care about the top-most `if` in the chain | ||
| if parent_node_is_if_expr(expr, cx) { | ||
| return; | ||
| } | ||
|
|
||
| // Check that there exists at least one explicit else condition | ||
| let (conds, _) = if_sequence(expr); | ||
| if conds.len() < 2 { | ||
| return; | ||
| } | ||
|
|
||
| for cond in conds.windows(2) { | ||
| if let ( | ||
| &ExprKind::Binary(ref kind1, ref lhs1, ref rhs1), | ||
| &ExprKind::Binary(ref kind2, ref lhs2, ref rhs2), | ||
| ) = (&cond[0].node, &cond[1].node) | ||
| { | ||
| if !kind_is_cmp(kind1.node) || !kind_is_cmp(kind2.node) { | ||
| return; | ||
| } | ||
|
|
||
| // Check that both sets of operands are equal | ||
| let mut spanless_eq = SpanlessEq::new(cx); | ||
| if (!spanless_eq.eq_expr(lhs1, lhs2) || !spanless_eq.eq_expr(rhs1, rhs2)) | ||
| && (!spanless_eq.eq_expr(lhs1, rhs2) || !spanless_eq.eq_expr(rhs1, lhs2)) | ||
| { | ||
| return; | ||
| } | ||
| } else { | ||
| // We only care about comparison chains | ||
| return; | ||
| } | ||
| } | ||
| span_help_and_lint( | ||
| cx, | ||
| COMPARISON_CHAIN, | ||
| expr.span, | ||
| "`if` chain can be rewritten with `match`", | ||
| "Consider rewriting the `if` chain to use `cmp` and `match`.", | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| fn kind_is_cmp(kind: BinOpKind) -> bool { | ||
| match kind { | ||
| BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq => true, | ||
| _ => false, | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.