Skip to content

Commit 8936896

Browse files
committed
Added basic lint and tests
1 parent f8308c8 commit 8936896

File tree

5 files changed

+274
-151
lines changed

5 files changed

+274
-151
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,6 +1291,7 @@ Released 2018-09-13
12911291
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
12921292
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
12931293
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
1294+
[`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub
12941295
[`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
12951296
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
12961297
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
use crate::utils::{higher, in_macro, span_lint_and_sugg};
2+
use if_chain::if_chain;
3+
use rustc_ast::ast::LitKind;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, StmtKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
9+
declare_clippy_lint! {
10+
/// **What it does:** Checks for implicit saturating subtraction.
11+
///
12+
/// **Why is this bad?** Simplicity and readability. Instead we can easily use an inbuilt function.
13+
///
14+
/// **Known problems:** None.
15+
///
16+
/// **Example:**
17+
///
18+
/// ```rust
19+
/// let end = 10;
20+
/// let start = 5;
21+
///
22+
/// let mut i = end - start;
23+
///
24+
/// // Bad
25+
/// if i != 0 {
26+
/// i -= 1;
27+
/// }
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// let end = 10;
32+
/// let start = 5;
33+
///
34+
/// let mut i = end - start;
35+
///
36+
/// // Good
37+
/// i.saturating_sub(1);
38+
/// ```
39+
pub IMPLICIT_SATURATING_SUB,
40+
pedantic,
41+
"Perform saturating subtraction instead of implicitly checking lower bound of data type"
42+
}
43+
44+
declare_lint_pass!(ImplicitSaturatingSub => [IMPLICIT_SATURATING_SUB]);
45+
46+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitSaturatingSub {
47+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) {
48+
if in_macro(expr.span) {
49+
return;
50+
}
51+
if_chain! {
52+
if let Some((ref cond, ref then, None)) = higher::if_block(&expr);
53+
// Check if the conditional expression is a binary operation
54+
if let ExprKind::Binary(ref op, ref left, ref right) = cond.kind;
55+
// Ensure that the binary operator is > or !=
56+
if BinOpKind::Ne == op.node || BinOpKind::Gt == op.node;
57+
if let ExprKind::Path(ref cond_path) = left.kind;
58+
// Get the literal on the right hand side
59+
if let ExprKind::Lit(ref lit) = right.kind;
60+
if let LitKind::Int(0, _) = lit.node;
61+
// Check if the true condition block has only one statement
62+
if let ExprKind::Block(ref block, _) = then.kind;
63+
if block.stmts.len() == 1;
64+
// Check if assign operation is done
65+
if let StmtKind::Semi(ref e) = block.stmts[0].kind;
66+
if let ExprKind::AssignOp(ref op1, ref target, ref value) = e.kind;
67+
if BinOpKind::Sub == op1.node;
68+
if let ExprKind::Path(ref assign_path) = target.kind;
69+
// Check if the variable in the condition and assignment statement are the same
70+
if let (QPath::Resolved(_, ref cres_path), QPath::Resolved(_, ref ares_path)) = (cond_path, assign_path);
71+
if cres_path.res == ares_path.res;
72+
if let ExprKind::Lit(ref lit1) = value.kind;
73+
if let LitKind::Int(assign_lit, _) = lit1.node;
74+
then {
75+
// Get the variable name
76+
let var_name = ares_path.segments[0].ident.name.as_str();
77+
let applicability = Applicability::MaybeIncorrect;
78+
span_lint_and_sugg(
79+
cx,
80+
IMPLICIT_SATURATING_SUB,
81+
expr.span,
82+
"Implicitly performing saturating subtraction",
83+
"try",
84+
format!("{}.saturating_sub({});", var_name, assign_lit.to_string()),
85+
applicability
86+
);
87+
}
88+
}
89+
}
90+
}

0 commit comments

Comments
 (0)