From 21e9a1285de20f2aaad9b644655fb24f146f3a76 Mon Sep 17 00:00:00 2001 From: Michael Recachinas Date: Sat, 23 Sep 2017 19:32:11 +0100 Subject: [PATCH] Use span_lint_and_then as per feedback --- clippy_lints/src/int_plus_one.rs | 112 +++++++++++++++++++++++++------ tests/ui/int_plus_one.stderr | 20 ++++-- 2 files changed, 106 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/int_plus_one.rs b/clippy_lints/src/int_plus_one.rs index 75538e7946e..d8b056fc29a 100644 --- a/clippy_lints/src/int_plus_one.rs +++ b/clippy_lints/src/int_plus_one.rs @@ -3,7 +3,7 @@ use rustc::lint::*; use syntax::ast::*; -use utils::span_help_and_lint; +use utils::{span_lint_and_then, snippet_opt}; /// **What it does:** Checks for usage of `x >= y + 1` or `x - 1 >= y` (and `<=`) in a block /// @@ -45,6 +45,11 @@ impl LintPass for IntPlusOne { // x + 1 <= y // x <= y - 1 +enum Side { + LHS, + RHS, +} + impl IntPlusOne { #[allow(cast_sign_loss)] fn check_lit(&self, lit: &Lit, target_value: i128) -> bool { @@ -54,62 +59,125 @@ impl IntPlusOne { false } - fn check_binop(&self, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> bool { + fn check_binop(&self, cx: &EarlyContext, block: &Expr, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<(bool, Option)> { match (binop, &lhs.node, &rhs.node) { // case where `x - 1 >= ...` or `-1 + x >= ...` (BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => { match (lhskind.node, &lhslhs.node, &lhsrhs.node) { // `-1 + x` - (BinOpKind::Add, &ExprKind::Lit(ref lit), _) => self.check_lit(lit, -1), + (BinOpKind::Add, &ExprKind::Lit(ref lit), _) => { + let recommendation = self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS); + if self.check_lit(lit, -1) { + self.emit_warning(cx, block, recommendation) + } + }, // `x - 1` - (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) => self.check_lit(lit, 1), - _ => false + (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) => { + let recommendation = self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + } + _ => () } }, // case where `... >= y + 1` or `... >= 1 + y` (BinOpKind::Ge, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) if rhskind.node == BinOpKind::Add => { match (&rhslhs.node, &rhsrhs.node) { // `y + 1` and `1 + y` - (&ExprKind::Lit(ref lit), _)|(_, &ExprKind::Lit(ref lit)) => self.check_lit(lit, 1), - _ => false + (&ExprKind::Lit(ref lit), _) => { + let recommendation = self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + }, + (_, &ExprKind::Lit(ref lit)) => { + let recommendation = self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + }, + _ => () } }, // case where `x + 1 <= ...` or `1 + x <= ...` (BinOpKind::Le, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) if lhskind.node == BinOpKind::Add => { match (&lhslhs.node, &lhsrhs.node) { // `1 + x` and `x + 1` - (&ExprKind::Lit(ref lit), _)|(_, &ExprKind::Lit(ref lit)) => self.check_lit(lit, 1), - _ => false + (&ExprKind::Lit(ref lit), _) => { + let recommendation = self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + }, + (_, &ExprKind::Lit(ref lit)) => { + let recommendation = self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + }, + _ => () } }, // case where `... >= y - 1` or `... >= -1 + y` (BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => { match (rhskind.node, &rhslhs.node, &rhsrhs.node) { // `-1 + y` - (BinOpKind::Add, &ExprKind::Lit(ref lit), _) => self.check_lit(lit, -1), + (BinOpKind::Add, &ExprKind::Lit(ref lit), _) => { + let recommendation = self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS); + if self.check_lit(lit, -1) { + self.emit_warning(cx, block, recommendation) + } + }, // `y - 1` - (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) => self.check_lit(lit, 1), - _ => false + (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) => { + let recommendation = self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS); + if self.check_lit(lit, 1) { + self.emit_warning(cx, block, recommendation) + } + }, + _ => () } }, - _ => false + _ => () } } + fn generate_recommendation(&self, cx: &EarlyContext, binop: BinOpKind, node: &Expr, other_side: &Expr, side: Side) -> Option { + let binop_string = match binop { + BinOpKind::Ge => ">", + BinOpKind::Le => "<", + _ => return None + }; + if let Some(snippet) = snippet_opt(cx, node.span) { + if let Some(other_side_snippet) = snippet_opt(cx, other_side.span) { + let rec = match side { + Side::LHS => Some(format!("{} {} {}", snippet, binop_string, other_side_snippet)), + Side::RHS => Some(format!("{} {} {}", other_side_snippet, binop_string, snippet)), + }; + return rec; + } + } + None + } + + fn emit_warning(&self, cx: &EarlyContext, block: &Expr, recommendation: Option) { + if let Some(rec) = recommendation { + span_lint_and_then(cx, + INT_PLUS_ONE, + block.span, + "Unnecessary `>= y + 1` or `x - 1 >=`", + |db| { + db.span_suggestion(block.span, "change `>= y + 1` to `> y` as shown", rec); + }); + } + } } impl EarlyLintPass for IntPlusOne { fn check_expr(&mut self, cx: &EarlyContext, item: &Expr) { if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = item.node { - if self.check_binop(kind.node, lhs, rhs) { - span_help_and_lint( - cx, - INT_PLUS_ONE, - item.span, - "Unnecessary `>= y + 1` or `x - 1 >=`", - "Consider reducing `x >= y + 1` or `x - 1 >= y` to `x > y`", - ); - } + self.check_binop(cx, item, kind.node, lhs, rhs); } } } diff --git a/tests/ui/int_plus_one.stderr b/tests/ui/int_plus_one.stderr index fd39e038e01..6f69ba9d714 100644 --- a/tests/ui/int_plus_one.stderr +++ b/tests/ui/int_plus_one.stderr @@ -5,7 +5,10 @@ error: Unnecessary `>= y + 1` or `x - 1 >=` | ^^^^^^^^^^ | = note: `-D int-plus-one` implied by `-D warnings` - = help: Consider reducing `x >= y + 1` or `x - 1 >= y` to `x > y` +help: change `>= y + 1` to `> y` as shown + | +10 | x > y; + | ^^^^^ error: Unnecessary `>= y + 1` or `x - 1 >=` --> $DIR/int_plus_one.rs:11:5 @@ -13,7 +16,10 @@ error: Unnecessary `>= y + 1` or `x - 1 >=` 11 | y + 1 <= x; | ^^^^^^^^^^ | - = help: Consider reducing `x >= y + 1` or `x - 1 >= y` to `x > y` +help: change `>= y + 1` to `> y` as shown + | +11 | y < x; + | ^^^^^ error: Unnecessary `>= y + 1` or `x - 1 >=` --> $DIR/int_plus_one.rs:13:5 @@ -21,7 +27,10 @@ error: Unnecessary `>= y + 1` or `x - 1 >=` 13 | x - 1 >= y; | ^^^^^^^^^^ | - = help: Consider reducing `x >= y + 1` or `x - 1 >= y` to `x > y` +help: change `>= y + 1` to `> y` as shown + | +13 | x > y; + | ^^^^^ error: Unnecessary `>= y + 1` or `x - 1 >=` --> $DIR/int_plus_one.rs:14:5 @@ -29,7 +38,10 @@ error: Unnecessary `>= y + 1` or `x - 1 >=` 14 | y <= x - 1; | ^^^^^^^^^^ | - = help: Consider reducing `x >= y + 1` or `x - 1 >= y` to `x > y` +help: change `>= y + 1` to `> y` as shown + | +14 | y < x; + | ^^^^^ error: aborting due to 4 previous errors