X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fimplicit_saturating_sub.rs;h=29d59c26d92c4a2f697399ff93df5c753bc89152;hb=a85c8f33ff0da5192bb44ac52cb838f638ad7c03;hp=4088c54623d36a3dc4187589ada0bf02e4345053;hpb=d647696c1f167802c9bb9ffe2c98459b152d563c;p=rust.git diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index 4088c54623d..29d59c26d92 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/clippy_lints/src/implicit_saturating_sub.rs @@ -1,10 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher; -use clippy_utils::SpanlessEq; +use clippy_utils::{higher, is_integer_literal, peel_blocks_with_stmt, SpanlessEq}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{lang_items::LangItem, BinOpKind, Expr, ExprKind, QPath, StmtKind}; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -17,22 +16,26 @@ /// /// ### Example /// ```rust - /// let end: u32 = 10; - /// let start: u32 = 5; - /// + /// # let end: u32 = 10; + /// # let start: u32 = 5; /// let mut i: u32 = end - start; /// - /// // Bad /// if i != 0 { /// i -= 1; /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// # let end: u32 = 10; + /// # let start: u32 = 5; + /// let mut i: u32 = end - start; /// - /// // Good /// i = i.saturating_sub(1); /// ``` #[clippy::version = "1.44.0"] pub IMPLICIT_SATURATING_SUB, - pedantic, + style, "Perform saturating subtraction instead of implicitly checking lower bound of data type" } @@ -49,16 +52,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { // Check if the conditional expression is a binary operation if let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind; - // Ensure that the binary operator is >, != and < + // Ensure that the binary operator is >, !=, or < if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node; - // Check if the true condition block has only one statement - if let ExprKind::Block(block, _) = then.kind; - if block.stmts.len() == 1 && block.expr.is_none(); - // Check if assign operation is done - if let StmtKind::Semi(e) = block.stmts[0].kind; - if let Some(target) = subtracts_one(cx, e); + if let Some(target) = subtracts_one(cx, then); // Extracting out the variable name if let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind; @@ -88,21 +86,13 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { // Get the variable name let var_name = ares_path.segments[0].ident.name.as_str(); - const INT_TYPES: [LangItem; 5] = [ - LangItem::I8, - LangItem::I16, - LangItem::I32, - LangItem::I64, - LangItem::Isize - ]; - match cond_num_val.kind { ExprKind::Lit(ref cond_lit) => { // Check if the constant is zero if let LitKind::Int(0, _) = cond_lit.node { if cx.typeck_results().expr_ty(cond_left).is_signed() { } else { - print_lint_and_sugg(cx, &var_name, expr); + print_lint_and_sugg(cx, var_name, expr); }; } }, @@ -111,10 +101,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if name.ident.as_str() == "MIN"; if let Some(const_id) = cx.typeck_results().type_dependent_def_id(cond_num_val.hir_id); if let Some(impl_id) = cx.tcx.impl_of_method(const_id); - let mut int_ids = INT_TYPES.iter().filter_map(|&ty| cx.tcx.lang_items().require(ty).ok()); - if int_ids.any(|int_id| int_id == impl_id); + if let None = cx.tcx.impl_trait_ref(impl_id); // An inherent impl + if cx.tcx.type_of(impl_id).is_integral(); then { - print_lint_and_sugg(cx, &var_name, expr) + print_lint_and_sugg(cx, var_name, expr) } } }, @@ -124,10 +114,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if name.ident.as_str() == "min_value"; if let Some(func_id) = cx.typeck_results().type_dependent_def_id(func.hir_id); if let Some(impl_id) = cx.tcx.impl_of_method(func_id); - let mut int_ids = INT_TYPES.iter().filter_map(|&ty| cx.tcx.lang_items().require(ty).ok()); - if int_ids.any(|int_id| int_id == impl_id); + if let None = cx.tcx.impl_trait_ref(impl_id); // An inherent impl + if cx.tcx.type_of(impl_id).is_integral(); then { - print_lint_and_sugg(cx, &var_name, expr) + print_lint_and_sugg(cx, var_name, expr) } } }, @@ -138,20 +128,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { } } -fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &Expr<'a>) -> Option<&'a Expr<'a>> { - match expr.kind { +fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { + match peel_blocks_with_stmt(expr).kind { ExprKind::AssignOp(ref op1, target, value) => { - if_chain! { - if BinOpKind::Sub == op1.node; - // Check if literal being subtracted is one - if let ExprKind::Lit(ref lit1) = value.kind; - if let LitKind::Int(1, _) = lit1.node; - then { - Some(target) - } else { - None - } - } + // Check if literal being subtracted is one + (BinOpKind::Sub == op1.node && is_integer_literal(value, 1)).then_some(target) }, ExprKind::Assign(target, value, _) => { if_chain! { @@ -160,8 +141,7 @@ fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &Expr<'a>) -> Option<&'a Expr<' if SpanlessEq::new(cx).eq_expr(left1, target); - if let ExprKind::Lit(ref lit1) = right1.kind; - if let LitKind::Int(1, _) = lit1.node; + if is_integer_literal(right1, 1); then { Some(target) } else { @@ -180,7 +160,7 @@ fn print_lint_and_sugg(cx: &LateContext<'_>, var_name: &str, expr: &Expr<'_>) { expr.span, "implicitly performing saturating subtraction", "try", - format!("{} = {}.saturating_sub({});", var_name, var_name, '1'), + format!("{var_name} = {var_name}.saturating_sub({});", '1'), Applicability::MachineApplicable, ); }