From 51d8b6c6643abb048987bd0befdeb6f46db0f6b5 Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 8 Sep 2022 12:04:55 -0300 Subject: Rename the `arithmetic` lint --- CHANGELOG.md | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_restriction.rs | 2 +- clippy_lints/src/lib.rs | 8 +- clippy_lints/src/operators/arithmetic.rs | 173 --------------------- .../src/operators/arithmetic_side_effects.rs | 173 +++++++++++++++++++++ clippy_lints/src/operators/mod.rs | 10 +- clippy_lints/src/utils/conf.rs | 2 +- src/docs.rs | 2 +- src/docs/arithmetic.txt | 33 ---- src/docs/arithmetic_side_effects.txt | 33 ++++ .../arithmetic_allowed/arithmetic_allowed.rs | 24 --- tests/ui-toml/arithmetic_allowed/clippy.toml | 1 - .../arithmetic_side_effects_allowed.rs | 24 +++ .../arithmetic_side_effects_allowed/clippy.toml | 1 + .../toml_unknown_key/conf_unknown_key.stderr | 2 +- tests/ui/arithmetic.rs | 57 ------- tests/ui/arithmetic.stderr | 22 --- tests/ui/arithmetic_side_effects.rs | 57 +++++++ tests/ui/arithmetic_side_effects.stderr | 22 +++ 20 files changed, 327 insertions(+), 323 deletions(-) delete mode 100644 clippy_lints/src/operators/arithmetic.rs create mode 100644 clippy_lints/src/operators/arithmetic_side_effects.rs delete mode 100644 src/docs/arithmetic.txt create mode 100644 src/docs/arithmetic_side_effects.txt delete mode 100644 tests/ui-toml/arithmetic_allowed/arithmetic_allowed.rs delete mode 100644 tests/ui-toml/arithmetic_allowed/clippy.toml create mode 100644 tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs create mode 100644 tests/ui-toml/arithmetic_side_effects_allowed/clippy.toml delete mode 100644 tests/ui/arithmetic.rs delete mode 100644 tests/ui/arithmetic.stderr create mode 100644 tests/ui/arithmetic_side_effects.rs create mode 100644 tests/ui/arithmetic_side_effects.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 257add86b6e..d847e4c7494 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3583,7 +3583,7 @@ Released 2018-09-13 [`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant -[`arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic +[`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions [`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 13b573beea1..962e6722006 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -437,7 +437,7 @@ store.register_lints(&[ octal_escapes::OCTAL_ESCAPES, only_used_in_recursion::ONLY_USED_IN_RECURSION, operators::ABSURD_EXTREME_COMPARISONS, - operators::ARITHMETIC, + operators::ARITHMETIC_SIDE_EFFECTS, operators::ASSIGN_OP_PATTERN, operators::BAD_BIT_MASK, operators::CMP_NAN, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index dd1e1e1a8e3..6eb9b3d3b9b 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -50,7 +50,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION), LintId::of(module_style::MOD_MODULE_FILES), LintId::of(module_style::SELF_NAMED_MODULE_FILES), - LintId::of(operators::ARITHMETIC), + LintId::of(operators::ARITHMETIC_SIDE_EFFECTS), LintId::of(operators::FLOAT_ARITHMETIC), LintId::of(operators::FLOAT_CMP_CONST), LintId::of(operators::INTEGER_ARITHMETIC), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a26e129f094..7633bf347c3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -544,8 +544,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(utils::internal_lints::MsrvAttrImpl)); } - let arithmetic_allowed = conf.arithmetic_allowed.clone(); - store.register_late_pass(move || Box::new(operators::arithmetic::Arithmetic::new(arithmetic_allowed.clone()))); + let arithmetic_side_effects_allowed = conf.arithmetic_side_effects_allowed.clone(); + store.register_late_pass(move || { + Box::new(operators::arithmetic_side_effects::ArithmeticSideEffects::new( + arithmetic_side_effects_allowed.clone(), + )) + }); store.register_late_pass(|| Box::new(utils::dump_hir::DumpHir)); store.register_late_pass(|| Box::new(utils::author::Author)); let await_holding_invalid_types = conf.await_holding_invalid_types.clone(); diff --git a/clippy_lints/src/operators/arithmetic.rs b/clippy_lints/src/operators/arithmetic.rs deleted file mode 100644 index 27eef92deef..00000000000 --- a/clippy_lints/src/operators/arithmetic.rs +++ /dev/null @@ -1,173 +0,0 @@ -#![allow( - // False positive - clippy::match_same_arms -)] - -use super::ARITHMETIC; -use clippy_utils::{consts::constant_simple, diagnostics::span_lint}; -use rustc_ast as ast; -use rustc_data_structures::fx::FxHashSet; -use rustc_hir as hir; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Ty; -use rustc_session::impl_lint_pass; -use rustc_span::source_map::{Span, Spanned}; - -const HARD_CODED_ALLOWED: &[&str] = &[ - "f32", - "f64", - "std::num::Saturating", - "std::string::String", - "std::num::Wrapping", -]; - -#[derive(Debug)] -pub struct Arithmetic { - allowed: FxHashSet, - // Used to check whether expressions are constants, such as in enum discriminants and consts - const_span: Option, - expr_span: Option, -} - -impl_lint_pass!(Arithmetic => [ARITHMETIC]); - -impl Arithmetic { - #[must_use] - pub fn new(mut allowed: FxHashSet) -> Self { - allowed.extend(HARD_CODED_ALLOWED.iter().copied().map(String::from)); - Self { - allowed, - const_span: None, - expr_span: None, - } - } - - /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that - /// won't overflow. - fn has_valid_assign_op(op: &Spanned, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { - if !Self::is_literal_integer(rhs, rhs_refs) { - return false; - } - if let hir::BinOpKind::Div | hir::BinOpKind::Mul = op.node - && let hir::ExprKind::Lit(ref lit) = rhs.kind - && let ast::LitKind::Int(1, _) = lit.node - { - return true; - } - false - } - - /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment - /// already handled by the CTFE. - fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { - Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs) - } - - /// Checks if the given `expr` has any of the inner `allowed` elements. - fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { - self.allowed.contains( - cx.typeck_results() - .expr_ty(expr) - .to_string() - .split('<') - .next() - .unwrap_or_default(), - ) - } - - /// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references. - fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool { - let is_integral = expr_refs.is_integral(); - let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_)); - is_integral && is_literal - } - - fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { - span_lint(cx, ARITHMETIC, expr.span, "arithmetic detected"); - self.expr_span = Some(expr.span); - } - - /// Manages when the lint should be triggered. Operations in constant environments, hard coded - /// types, custom allowed types and non-constant operations that won't overflow are ignored. - fn manage_bin_ops( - &mut self, - cx: &LateContext<'_>, - expr: &hir::Expr<'_>, - op: &Spanned, - lhs: &hir::Expr<'_>, - rhs: &hir::Expr<'_>, - ) { - if constant_simple(cx, cx.typeck_results(), expr).is_some() { - return; - } - if !matches!( - op.node, - hir::BinOpKind::Add - | hir::BinOpKind::Sub - | hir::BinOpKind::Mul - | hir::BinOpKind::Div - | hir::BinOpKind::Rem - | hir::BinOpKind::Shl - | hir::BinOpKind::Shr - ) { - return; - }; - if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) { - return; - } - let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs(); - let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs(); - let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs); - if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) { - return; - } - self.issue_lint(cx, expr); - } -} - -impl<'tcx> LateLintPass<'tcx> for Arithmetic { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) { - return; - } - match &expr.kind { - hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => { - self.manage_bin_ops(cx, expr, op, lhs, rhs); - }, - hir::ExprKind::Unary(hir::UnOp::Neg, _) => { - if constant_simple(cx, cx.typeck_results(), expr).is_none() { - self.issue_lint(cx, expr); - } - }, - _ => {}, - } - } - - fn check_body(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) { - let body_owner = cx.tcx.hir().body_owner(body.id()); - let body_owner_def_id = cx.tcx.hir().local_def_id(body_owner); - let body_owner_kind = cx.tcx.hir().body_owner_kind(body_owner_def_id); - if let hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) = body_owner_kind { - let body_span = cx.tcx.hir().span_with_body(body_owner); - if let Some(span) = self.const_span && span.contains(body_span) { - return; - } - self.const_span = Some(body_span); - } - } - - fn check_body_post(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) { - let body_owner = cx.tcx.hir().body_owner(body.id()); - let body_span = cx.tcx.hir().span(body_owner); - if let Some(span) = self.const_span && span.contains(body_span) { - return; - } - self.const_span = None; - } - - fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if Some(expr.span) == self.expr_span { - self.expr_span = None; - } - } -} diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs new file mode 100644 index 00000000000..83b69fbb311 --- /dev/null +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -0,0 +1,173 @@ +#![allow( + // False positive + clippy::match_same_arms +)] + +use super::ARITHMETIC_SIDE_EFFECTS; +use clippy_utils::{consts::constant_simple, diagnostics::span_lint}; +use rustc_ast as ast; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Ty; +use rustc_session::impl_lint_pass; +use rustc_span::source_map::{Span, Spanned}; + +const HARD_CODED_ALLOWED: &[&str] = &[ + "f32", + "f64", + "std::num::Saturating", + "std::string::String", + "std::num::Wrapping", +]; + +#[derive(Debug)] +pub struct ArithmeticSideEffects { + allowed: FxHashSet, + // Used to check whether expressions are constants, such as in enum discriminants and consts + const_span: Option, + expr_span: Option, +} + +impl_lint_pass!(ArithmeticSideEffects => [ARITHMETIC_SIDE_EFFECTS]); + +impl ArithmeticSideEffects { + #[must_use] + pub fn new(mut allowed: FxHashSet) -> Self { + allowed.extend(HARD_CODED_ALLOWED.iter().copied().map(String::from)); + Self { + allowed, + const_span: None, + expr_span: None, + } + } + + /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that + /// won't overflow. + fn has_valid_assign_op(op: &Spanned, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { + if !Self::is_literal_integer(rhs, rhs_refs) { + return false; + } + if let hir::BinOpKind::Div | hir::BinOpKind::Mul = op.node + && let hir::ExprKind::Lit(ref lit) = rhs.kind + && let ast::LitKind::Int(1, _) = lit.node + { + return true; + } + false + } + + /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment + /// already handled by the CTFE. + fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool { + Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs) + } + + /// Checks if the given `expr` has any of the inner `allowed` elements. + fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + self.allowed.contains( + cx.typeck_results() + .expr_ty(expr) + .to_string() + .split('<') + .next() + .unwrap_or_default(), + ) + } + + /// Explicit integers like `1` or `i32::MAX`. Does not take into consideration references. + fn is_literal_integer(expr: &hir::Expr<'_>, expr_refs: Ty<'_>) -> bool { + let is_integral = expr_refs.is_integral(); + let is_literal = matches!(expr.kind, hir::ExprKind::Lit(_)); + is_integral && is_literal + } + + fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) { + span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected"); + self.expr_span = Some(expr.span); + } + + /// Manages when the lint should be triggered. Operations in constant environments, hard coded + /// types, custom allowed types and non-constant operations that won't overflow are ignored. + fn manage_bin_ops( + &mut self, + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + op: &Spanned, + lhs: &hir::Expr<'_>, + rhs: &hir::Expr<'_>, + ) { + if constant_simple(cx, cx.typeck_results(), expr).is_some() { + return; + } + if !matches!( + op.node, + hir::BinOpKind::Add + | hir::BinOpKind::Sub + | hir::BinOpKind::Mul + | hir::BinOpKind::Div + | hir::BinOpKind::Rem + | hir::BinOpKind::Shl + | hir::BinOpKind::Shr + ) { + return; + }; + if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) { + return; + } + let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs(); + let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs(); + let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs); + if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) { + return; + } + self.issue_lint(cx, expr); + } +} + +impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if self.expr_span.is_some() || self.const_span.map_or(false, |sp| sp.contains(expr.span)) { + return; + } + match &expr.kind { + hir::ExprKind::Binary(op, lhs, rhs) | hir::ExprKind::AssignOp(op, lhs, rhs) => { + self.manage_bin_ops(cx, expr, op, lhs, rhs); + }, + hir::ExprKind::Unary(hir::UnOp::Neg, _) => { + if constant_simple(cx, cx.typeck_results(), expr).is_none() { + self.issue_lint(cx, expr); + } + }, + _ => {}, + } + } + + fn check_body(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) { + let body_owner = cx.tcx.hir().body_owner(body.id()); + let body_owner_def_id = cx.tcx.hir().local_def_id(body_owner); + let body_owner_kind = cx.tcx.hir().body_owner_kind(body_owner_def_id); + if let hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) = body_owner_kind { + let body_span = cx.tcx.hir().span_with_body(body_owner); + if let Some(span) = self.const_span && span.contains(body_span) { + return; + } + self.const_span = Some(body_span); + } + } + + fn check_body_post(&mut self, cx: &LateContext<'_>, body: &hir::Body<'_>) { + let body_owner = cx.tcx.hir().body_owner(body.id()); + let body_span = cx.tcx.hir().span(body_owner); + if let Some(span) = self.const_span && span.contains(body_span) { + return; + } + self.const_span = None; + } + + fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if Some(expr.span) == self.expr_span { + self.expr_span = None; + } + } +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index c7b0633b79c..c32b4df4f75 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -21,7 +21,7 @@ mod ptr_eq; mod self_assignment; mod verbose_bit_mask; -pub(crate) mod arithmetic; +pub(crate) mod arithmetic_side_effects; use rustc_hir::{Body, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; @@ -92,11 +92,11 @@ declare_clippy_lint! { /// ``` /// /// ### Allowed types - /// Custom allowed types can be specified through the "arithmetic-allowed" filter. + /// Custom allowed types can be specified through the "arithmetic-side-effects-allowed" filter. #[clippy::version = "1.64.0"] - pub ARITHMETIC, + pub ARITHMETIC_SIDE_EFFECTS, restriction, - "any arithmetic expression that could overflow or panic" + "any arithmetic expression that can cause side effects like overflows or panics" } declare_clippy_lint! { @@ -789,7 +789,7 @@ pub struct Operators { } impl_lint_pass!(Operators => [ ABSURD_EXTREME_COMPARISONS, - ARITHMETIC, + ARITHMETIC_SIDE_EFFECTS, INTEGER_ARITHMETIC, FLOAT_ARITHMETIC, ASSIGN_OP_PATTERN, diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 84e65d5fa0b..a8500beb257 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -208,7 +208,7 @@ define_Conf! { /// Lint: Arithmetic. /// /// Suppress checking of the passed type names. - (arithmetic_allowed: rustc_data_structures::fx::FxHashSet = <_>::default()), + (arithmetic_side_effects_allowed: rustc_data_structures::fx::FxHashSet = <_>::default()), /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX. /// /// Suppress lints whenever the suggested change would cause breakage for other crates. diff --git a/src/docs.rs b/src/docs.rs index 69243bf4d9c..f3a5048e7fa 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -26,7 +26,7 @@ docs! { "almost_complete_letter_range", "almost_swapped", "approx_constant", - "arithmetic", + "arithmetic_side_effects", "as_conversions", "as_underscore", "assertions_on_constants", diff --git a/src/docs/arithmetic.txt b/src/docs/arithmetic.txt deleted file mode 100644 index f04125060fb..00000000000 --- a/src/docs/arithmetic.txt +++ /dev/null @@ -1,33 +0,0 @@ -### What it does -Checks any kind of arithmetic operation of any type. - -Operators like `+`, `-`, `*` or `<<` are usually capable of overflowing according to the [Rust -Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), -or can panic (`/`, `%`). - -Known safe built-in types like `Wrapping` or `Saturing`, floats, operations in constant -environments, allowed types and non-constant operations that won't overflow are ignored. - -### Why is this bad? -For integers, overflow will trigger a panic in debug builds or wrap the result in -release mode; division by zero will cause a panic in either mode. As a result, it is -desirable to explicitly call checked, wrapping or saturating arithmetic methods. - -#### Example -``` -// `n` can be any number, including `i32::MAX`. -fn foo(n: i32) -> i32 { - n + 1 -} -``` - -Third-party types can also overflow or present unwanted side-effects. - -#### Example -``` -use rust_decimal::Decimal; -let _n = Decimal::MAX + Decimal::MAX; -``` - -### Allowed types -Custom allowed types can be specified through the "arithmetic-allowed" filter. \ No newline at end of file diff --git a/src/docs/arithmetic_side_effects.txt b/src/docs/arithmetic_side_effects.txt new file mode 100644 index 00000000000..6c7d51a4989 --- /dev/null +++ b/src/docs/arithmetic_side_effects.txt @@ -0,0 +1,33 @@ +### What it does +Checks any kind of arithmetic operation of any type. + +Operators like `+`, `-`, `*` or `<<` are usually capable of overflowing according to the [Rust +Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), +or can panic (`/`, `%`). + +Known safe built-in types like `Wrapping` or `Saturing`, floats, operations in constant +environments, allowed types and non-constant operations that won't overflow are ignored. + +### Why is this bad? +For integers, overflow will trigger a panic in debug builds or wrap the result in +release mode; division by zero will cause a panic in either mode. As a result, it is +desirable to explicitly call checked, wrapping or saturating arithmetic methods. + +#### Example +``` +// `n` can be any number, including `i32::MAX`. +fn foo(n: i32) -> i32 { + n + 1 +} +``` + +Third-party types can also overflow or present unwanted side-effects. + +#### Example +``` +use rust_decimal::Decimal; +let _n = Decimal::MAX + Decimal::MAX; +``` + +### Allowed types +Custom allowed types can be specified through the "arithmetic-side-effects-allowed" filter. \ No newline at end of file diff --git a/tests/ui-toml/arithmetic_allowed/arithmetic_allowed.rs b/tests/ui-toml/arithmetic_allowed/arithmetic_allowed.rs deleted file mode 100644 index 195fabdbf71..00000000000 --- a/tests/ui-toml/arithmetic_allowed/arithmetic_allowed.rs +++ /dev/null @@ -1,24 +0,0 @@ -#![warn(clippy::arithmetic)] - -use core::ops::Add; - -#[derive(Clone, Copy)] -struct Point { - x: i32, - y: i32, -} - -impl Add for Point { - type Output = Self; - - fn add(self, other: Self) -> Self { - todo!() - } -} - -fn main() { - let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 }; - - let point: Point = Point { x: 1, y: 0 }; - let _ = point + point; -} diff --git a/tests/ui-toml/arithmetic_allowed/clippy.toml b/tests/ui-toml/arithmetic_allowed/clippy.toml deleted file mode 100644 index cc40570b12a..00000000000 --- a/tests/ui-toml/arithmetic_allowed/clippy.toml +++ /dev/null @@ -1 +0,0 @@ -arithmetic-allowed = ["Point"] diff --git a/tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs b/tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs new file mode 100644 index 00000000000..1aed09b7c7b --- /dev/null +++ b/tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs @@ -0,0 +1,24 @@ +#![warn(clippy::arithmetic_side_effects)] + +use core::ops::Add; + +#[derive(Clone, Copy)] +struct Point { + x: i32, + y: i32, +} + +impl Add for Point { + type Output = Self; + + fn add(self, other: Self) -> Self { + todo!() + } +} + +fn main() { + let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 }; + + let point: Point = Point { x: 1, y: 0 }; + let _ = point + point; +} diff --git a/tests/ui-toml/arithmetic_side_effects_allowed/clippy.toml b/tests/ui-toml/arithmetic_side_effects_allowed/clippy.toml new file mode 100644 index 00000000000..e736256f29a --- /dev/null +++ b/tests/ui-toml/arithmetic_side_effects_allowed/clippy.toml @@ -0,0 +1 @@ +arithmetic-side-effects-allowed = ["Point"] diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index a52a0b5289f..f27f78d15d3 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -3,7 +3,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie allow-expect-in-tests allow-unwrap-in-tests allowed-scripts - arithmetic-allowed + arithmetic-side-effects-allowed array-size-threshold avoid-breaking-exported-api await-holding-invalid-types diff --git a/tests/ui/arithmetic.rs b/tests/ui/arithmetic.rs deleted file mode 100644 index a9ac46e9a19..00000000000 --- a/tests/ui/arithmetic.rs +++ /dev/null @@ -1,57 +0,0 @@ -#![allow(clippy::assign_op_pattern, clippy::unnecessary_owned_empty_strings)] -#![feature(inline_const, saturating_int_impl)] -#![warn(clippy::arithmetic)] - -use core::num::{Saturating, Wrapping}; - -pub fn hard_coded_allowed() { - let _ = 1f32 + 1f32; - let _ = 1f64 + 1f64; - - let _ = Saturating(0u32) + Saturating(0u32); - let _ = String::new() + ""; - let _ = Wrapping(0u32) + Wrapping(0u32); - - let saturating: Saturating = Saturating(0u32); - let string: String = String::new(); - let wrapping: Wrapping = Wrapping(0u32); - - let inferred_saturating = saturating + saturating; - let inferred_string = string + ""; - let inferred_wrapping = wrapping + wrapping; - - let _ = inferred_saturating + inferred_saturating; - let _ = inferred_string + ""; - let _ = inferred_wrapping + inferred_wrapping; -} - -#[rustfmt::skip] -pub fn non_overflowing_ops() { - const _: i32 = { let mut n = 1; n += 1; n }; - let _ = const { let mut n = 1; n += 1; n }; - - const _: i32 = { let mut n = 1; n = n + 1; n }; - let _ = const { let mut n = 1; n = n + 1; n }; - - const _: i32 = { let mut n = 1; n = 1 + n; n }; - let _ = const { let mut n = 1; n = 1 + n; n }; - - const _: i32 = 1 + 1; - let _ = 1 + 1; - let _ = const { 1 + 1 }; - - let mut _a = 1; - _a *= 1; - _a /= 1; -} - -#[rustfmt::skip] -pub fn overflowing_ops() { - let mut _a = 1; _a += 1; - - let mut _b = 1; _b = _b + 1; - - let mut _c = 1; _c = 1 + _c; -} - -fn main() {} diff --git a/tests/ui/arithmetic.stderr b/tests/ui/arithmetic.stderr deleted file mode 100644 index a51cf6ba600..00000000000 --- a/tests/ui/arithmetic.stderr +++ /dev/null @@ -1,22 +0,0 @@ -error: arithmetic detected - --> $DIR/arithmetic.rs:50:21 - | -LL | let mut _a = 1; _a += 1; - | ^^^^^^^ - | - = note: `-D clippy::arithmetic` implied by `-D warnings` - -error: arithmetic detected - --> $DIR/arithmetic.rs:52:26 - | -LL | let mut _b = 1; _b = _b + 1; - | ^^^^^^ - -error: arithmetic detected - --> $DIR/arithmetic.rs:54:26 - | -LL | let mut _c = 1; _c = 1 + _c; - | ^^^^^^ - -error: aborting due to 3 previous errors - diff --git a/tests/ui/arithmetic_side_effects.rs b/tests/ui/arithmetic_side_effects.rs new file mode 100644 index 00000000000..f5390c74642 --- /dev/null +++ b/tests/ui/arithmetic_side_effects.rs @@ -0,0 +1,57 @@ +#![allow(clippy::assign_op_pattern, clippy::unnecessary_owned_empty_strings)] +#![feature(inline_const, saturating_int_impl)] +#![warn(clippy::arithmetic_side_effects)] + +use core::num::{Saturating, Wrapping}; + +pub fn hard_coded_allowed() { + let _ = 1f32 + 1f32; + let _ = 1f64 + 1f64; + + let _ = Saturating(0u32) + Saturating(0u32); + let _ = String::new() + ""; + let _ = Wrapping(0u32) + Wrapping(0u32); + + let saturating: Saturating = Saturating(0u32); + let string: String = String::new(); + let wrapping: Wrapping = Wrapping(0u32); + + let inferred_saturating = saturating + saturating; + let inferred_string = string + ""; + let inferred_wrapping = wrapping + wrapping; + + let _ = inferred_saturating + inferred_saturating; + let _ = inferred_string + ""; + let _ = inferred_wrapping + inferred_wrapping; +} + +#[rustfmt::skip] +pub fn non_overflowing_ops() { + const _: i32 = { let mut n = 1; n += 1; n }; + let _ = const { let mut n = 1; n += 1; n }; + + const _: i32 = { let mut n = 1; n = n + 1; n }; + let _ = const { let mut n = 1; n = n + 1; n }; + + const _: i32 = { let mut n = 1; n = 1 + n; n }; + let _ = const { let mut n = 1; n = 1 + n; n }; + + const _: i32 = 1 + 1; + let _ = 1 + 1; + let _ = const { 1 + 1 }; + + let mut _a = 1; + _a *= 1; + _a /= 1; +} + +#[rustfmt::skip] +pub fn overflowing_ops() { + let mut _a = 1; _a += 1; + + let mut _b = 1; _b = _b + 1; + + let mut _c = 1; _c = 1 + _c; +} + +fn main() {} diff --git a/tests/ui/arithmetic_side_effects.stderr b/tests/ui/arithmetic_side_effects.stderr new file mode 100644 index 00000000000..6c4c8bdec0f --- /dev/null +++ b/tests/ui/arithmetic_side_effects.stderr @@ -0,0 +1,22 @@ +error: arithmetic detected + --> $DIR/arithmetic_side_effects.rs:50:21 + | +LL | let mut _a = 1; _a += 1; + | ^^^^^^^ + | + = note: `-D clippy::arithmetic-side-effects` implied by `-D warnings` + +error: arithmetic detected + --> $DIR/arithmetic_side_effects.rs:52:26 + | +LL | let mut _b = 1; _b = _b + 1; + | ^^^^^^ + +error: arithmetic detected + --> $DIR/arithmetic_side_effects.rs:54:26 + | +LL | let mut _c = 1; _c = 1 + _c; + | ^^^^^^ + +error: aborting due to 3 previous errors + -- cgit 1.4.1-3-g733a5