mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-24 05:33:27 +00:00
[arithmetic-side-effects]: Consider user-provided pairs
This commit is contained in:
Caio
parent
641ced4eb9
commit
1f92f97e5a
9 changed files with 286 additions and 66 deletions
|
|
@ -508,9 +508,20 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
}
|
||||
|
||||
let arithmetic_side_effects_allowed = conf.arithmetic_side_effects_allowed.clone();
|
||||
let arithmetic_side_effects_allowed_binary = conf.arithmetic_side_effects_allowed_binary.clone();
|
||||
let arithmetic_side_effects_allowed_unary = conf.arithmetic_side_effects_allowed_unary.clone();
|
||||
store.register_late_pass(move |_| {
|
||||
Box::new(operators::arithmetic_side_effects::ArithmeticSideEffects::new(
|
||||
arithmetic_side_effects_allowed.clone(),
|
||||
arithmetic_side_effects_allowed
|
||||
.iter()
|
||||
.flat_map(|el| [[el.clone(), "*".to_string()], ["*".to_string(), el.clone()]])
|
||||
.chain(arithmetic_side_effects_allowed_binary.clone())
|
||||
.collect(),
|
||||
arithmetic_side_effects_allowed
|
||||
.iter()
|
||||
.chain(arithmetic_side_effects_allowed_unary.iter())
|
||||
.cloned()
|
||||
.collect(),
|
||||
))
|
||||
});
|
||||
store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir));
|
||||
|
|
|
|||
|
|
@ -5,25 +5,26 @@ use clippy_utils::{
|
|||
peel_hir_expr_refs,
|
||||
};
|
||||
use rustc_ast as ast;
|
||||
use rustc_data_structures::fx::FxHashSet;
|
||||
use rustc_data_structures::fx::{FxHashMap, 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] = &[
|
||||
"&str",
|
||||
"f32",
|
||||
"f64",
|
||||
"std::num::Saturating",
|
||||
"std::num::Wrapping",
|
||||
"std::string::String",
|
||||
const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[
|
||||
["f32", "f32"],
|
||||
["f64", "f64"],
|
||||
["std::num::Saturating", "std::num::Saturating"],
|
||||
["std::num::Wrapping", "std::num::Wrapping"],
|
||||
["std::string::String", "&str"],
|
||||
];
|
||||
const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"];
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ArithmeticSideEffects {
|
||||
allowed: FxHashSet<String>,
|
||||
allowed_binary: FxHashMap<String, FxHashSet<String>>,
|
||||
allowed_unary: FxHashSet<String>,
|
||||
// Used to check whether expressions are constants, such as in enum discriminants and consts
|
||||
const_span: Option<Span>,
|
||||
expr_span: Option<Span>,
|
||||
|
|
@ -33,19 +34,55 @@ impl_lint_pass!(ArithmeticSideEffects => [ARITHMETIC_SIDE_EFFECTS]);
|
|||
|
||||
impl ArithmeticSideEffects {
|
||||
#[must_use]
|
||||
pub fn new(mut allowed: FxHashSet<String>) -> Self {
|
||||
allowed.extend(HARD_CODED_ALLOWED.iter().copied().map(String::from));
|
||||
pub fn new(user_allowed_binary: Vec<[String; 2]>, user_allowed_unary: Vec<String>) -> Self {
|
||||
let mut allowed_binary: FxHashMap<String, FxHashSet<String>> = <_>::default();
|
||||
for [lhs, rhs] in user_allowed_binary.into_iter().chain(
|
||||
HARD_CODED_ALLOWED_BINARY
|
||||
.iter()
|
||||
.copied()
|
||||
.map(|[lhs, rhs]| [lhs.to_string(), rhs.to_string()]),
|
||||
) {
|
||||
allowed_binary.entry(lhs).or_default().insert(rhs);
|
||||
}
|
||||
let allowed_unary = user_allowed_unary
|
||||
.into_iter()
|
||||
.chain(HARD_CODED_ALLOWED_UNARY.iter().copied().map(String::from))
|
||||
.collect();
|
||||
Self {
|
||||
allowed,
|
||||
allowed_binary,
|
||||
allowed_unary,
|
||||
const_span: None,
|
||||
expr_span: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if the given `expr` has any of the inner `allowed` elements.
|
||||
fn is_allowed_ty(&self, ty: Ty<'_>) -> bool {
|
||||
self.allowed
|
||||
.contains(ty.to_string().split('<').next().unwrap_or_default())
|
||||
/// Checks if the lhs and the rhs types of a binary operation like "addition" or
|
||||
/// "multiplication" are present in the inner set of allowed types.
|
||||
fn has_allowed_binary(&self, lhs_ty: Ty<'_>, rhs_ty: Ty<'_>) -> bool {
|
||||
let lhs_ty_string = lhs_ty.to_string();
|
||||
let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or_default();
|
||||
let rhs_ty_string = rhs_ty.to_string();
|
||||
let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or_default();
|
||||
if let Some(rhs_from_specific) = self.allowed_binary.get(lhs_ty_string_elem)
|
||||
&& {
|
||||
let rhs_has_allowed_ty = rhs_from_specific.contains(rhs_ty_string_elem);
|
||||
rhs_has_allowed_ty || rhs_from_specific.contains("*")
|
||||
}
|
||||
{
|
||||
true
|
||||
} else if let Some(rhs_from_glob) = self.allowed_binary.get("*") {
|
||||
rhs_from_glob.contains(rhs_ty_string_elem)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if the type of an unary operation like "negation" is present in the inner set of
|
||||
/// allowed types.
|
||||
fn has_allowed_unary(&self, ty: Ty<'_>) -> bool {
|
||||
let ty_string = ty.to_string();
|
||||
let ty_string_elem = ty_string.split('<').next().unwrap_or_default();
|
||||
self.allowed_unary.contains(ty_string_elem)
|
||||
}
|
||||
|
||||
// For example, 8i32 or &i64::MAX.
|
||||
|
|
@ -97,8 +134,7 @@ impl ArithmeticSideEffects {
|
|||
};
|
||||
let lhs_ty = cx.typeck_results().expr_ty(lhs);
|
||||
let rhs_ty = cx.typeck_results().expr_ty(rhs);
|
||||
let lhs_and_rhs_have_the_same_ty = lhs_ty == rhs_ty;
|
||||
if lhs_and_rhs_have_the_same_ty && self.is_allowed_ty(lhs_ty) && self.is_allowed_ty(rhs_ty) {
|
||||
if self.has_allowed_binary(lhs_ty, rhs_ty) {
|
||||
return;
|
||||
}
|
||||
let has_valid_op = if Self::is_integral(lhs_ty) && Self::is_integral(rhs_ty) {
|
||||
|
|
@ -137,7 +173,7 @@ impl ArithmeticSideEffects {
|
|||
return;
|
||||
}
|
||||
let ty = cx.typeck_results().expr_ty(expr).peel_refs();
|
||||
if self.is_allowed_ty(ty) {
|
||||
if self.has_allowed_unary(ty) {
|
||||
return;
|
||||
}
|
||||
let actual_un_expr = peel_hir_expr_refs(un_expr).0;
|
||||
|
|
|
|||
|
|
@ -90,9 +90,6 @@ declare_clippy_lint! {
|
|||
/// 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.
|
||||
#[clippy::version = "1.64.0"]
|
||||
pub ARITHMETIC_SIDE_EFFECTS,
|
||||
restriction,
|
||||
|
|
|
|||
|
|
@ -205,10 +205,49 @@ macro_rules! define_Conf {
|
|||
}
|
||||
|
||||
define_Conf! {
|
||||
/// Lint: Arithmetic.
|
||||
/// Lint: ARITHMETIC_SIDE_EFFECTS.
|
||||
///
|
||||
/// Suppress checking of the passed type names.
|
||||
/// Suppress checking of the passed type names in all types of operations.
|
||||
///
|
||||
/// If a specific operation is desired, consider using `arithmetic_side_effects_allowed_binary` or `arithmetic_side_effects_allowed_unary` instead.
|
||||
///
|
||||
/// #### Example
|
||||
///
|
||||
/// ```toml
|
||||
/// arithmetic-side-effects-allowed = ["SomeType", "AnotherType"]
|
||||
/// ```
|
||||
///
|
||||
/// #### Noteworthy
|
||||
///
|
||||
/// A type, say `SomeType`, listed in this configuration has the same behavior of `["SomeType" , "*"], ["*", "SomeType"]` in `arithmetic_side_effects_allowed_binary`.
|
||||
(arithmetic_side_effects_allowed: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
|
||||
/// Lint: ARITHMETIC_SIDE_EFFECTS.
|
||||
///
|
||||
/// Suppress checking of the passed type pair names in binary operations like addition or
|
||||
/// multiplication.
|
||||
///
|
||||
/// Supports the "*" wildcard to indicate that a certain type won't trigger the lint regardless
|
||||
/// of the involved counterpart. For example, `["SomeType", "*"]` or `["*", "AnotherType"]`.
|
||||
///
|
||||
/// Pairs are asymmetric, which means that `["SomeType", "AnotherType"]` is not the same as
|
||||
/// `["AnotherType", "SomeType"]`.
|
||||
///
|
||||
/// #### Example
|
||||
///
|
||||
/// ```toml
|
||||
/// arithmetic-side-effects-allowed-binary = [["SomeType" , "f32"], ["AnotherType", "*"]]
|
||||
/// ```
|
||||
(arithmetic_side_effects_allowed_binary: Vec<[String; 2]> = <_>::default()),
|
||||
/// Lint: ARITHMETIC_SIDE_EFFECTS.
|
||||
///
|
||||
/// Suppress checking of the passed type names in unary operations like "negation" (`-`).
|
||||
///
|
||||
/// #### Example
|
||||
///
|
||||
/// ```toml
|
||||
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
|
||||
/// ```
|
||||
(arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::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.
|
||||
|
|
|
|||
|
|
@ -2,32 +2,117 @@
|
|||
|
||||
use core::ops::{Add, Neg};
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct Point {
|
||||
x: i32,
|
||||
y: i32,
|
||||
}
|
||||
macro_rules! create {
|
||||
($name:ident) => {
|
||||
#[allow(clippy::arithmetic_side_effects)]
|
||||
#[derive(Clone, Copy)]
|
||||
struct $name;
|
||||
|
||||
impl Add for Point {
|
||||
type Output = Self;
|
||||
|
||||
fn add(self, other: Self) -> Self {
|
||||
impl Add<$name> for $name {
|
||||
type Output = $name;
|
||||
fn add(self, other: $name) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Neg for Point {
|
||||
type Output = Self;
|
||||
impl Add<i32> for $name {
|
||||
type Output = $name;
|
||||
fn add(self, other: i32) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
|
||||
impl Add<$name> for i32 {
|
||||
type Output = $name;
|
||||
fn add(self, other: $name) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
|
||||
impl Add<i64> for $name {
|
||||
type Output = $name;
|
||||
fn add(self, other: i64) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
|
||||
impl Add<$name> for i64 {
|
||||
type Output = $name;
|
||||
fn add(self, other: $name) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
|
||||
impl Neg for $name {
|
||||
type Output = $name;
|
||||
fn neg(self) -> Self::Output {
|
||||
todo!()
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let _ = Point { x: 1, y: 0 } + Point { x: 2, y: 3 };
|
||||
create!(Foo);
|
||||
create!(Bar);
|
||||
create!(Baz);
|
||||
create!(OutOfNames);
|
||||
|
||||
let point: Point = Point { x: 1, y: 0 };
|
||||
let _ = point + point;
|
||||
let _ = -point;
|
||||
fn lhs_and_rhs_are_equal() {
|
||||
// is explicitly on the list
|
||||
let _ = OutOfNames + OutOfNames;
|
||||
// is explicitly on the list
|
||||
let _ = Foo + Foo;
|
||||
// is implicitly on the list
|
||||
let _ = Bar + Bar;
|
||||
// not on the list
|
||||
let _ = Baz + Baz;
|
||||
}
|
||||
|
||||
fn lhs_is_different() {
|
||||
// is explicitly on the list
|
||||
let _ = 1i32 + OutOfNames;
|
||||
// is explicitly on the list
|
||||
let _ = 1i32 + Foo;
|
||||
// is implicitly on the list
|
||||
let _ = 1i32 + Bar;
|
||||
// not on the list
|
||||
let _ = 1i32 + Baz;
|
||||
|
||||
// not on the list
|
||||
let _ = 1i64 + Foo;
|
||||
// is implicitly on the list
|
||||
let _ = 1i64 + Bar;
|
||||
// not on the list
|
||||
let _ = 1i64 + Baz;
|
||||
}
|
||||
|
||||
fn rhs_is_different() {
|
||||
// is explicitly on the list
|
||||
let _ = OutOfNames + 1i32;
|
||||
// is explicitly on the list
|
||||
let _ = Foo + 1i32;
|
||||
// is implicitly on the list
|
||||
let _ = Bar + 1i32;
|
||||
// not on the list
|
||||
let _ = Baz + 1i32;
|
||||
|
||||
// not on the list
|
||||
let _ = Foo + 1i64;
|
||||
// is implicitly on the list
|
||||
let _ = Bar + 1i64;
|
||||
// not on the list
|
||||
let _ = Baz + 1i64;
|
||||
}
|
||||
|
||||
fn unary() {
|
||||
// is explicitly on the list
|
||||
let _ = -OutOfNames;
|
||||
// is specifically on the list
|
||||
let _ = -Foo;
|
||||
// not on the list
|
||||
let _ = -Bar;
|
||||
// not on the list
|
||||
let _ = -Baz;
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,58 @@
|
|||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:68:13
|
||||
|
|
||||
LL | let _ = Baz + Baz;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:79:13
|
||||
|
|
||||
LL | let _ = 1i32 + Baz;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:82:13
|
||||
|
|
||||
LL | let _ = 1i64 + Foo;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:86:13
|
||||
|
|
||||
LL | let _ = 1i64 + Baz;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:97:13
|
||||
|
|
||||
LL | let _ = Baz + 1i32;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:100:13
|
||||
|
|
||||
LL | let _ = Foo + 1i64;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:104:13
|
||||
|
|
||||
LL | let _ = Baz + 1i64;
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:113:13
|
||||
|
|
||||
LL | let _ = -Bar;
|
||||
| ^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects_allowed.rs:115:13
|
||||
|
|
||||
LL | let _ = -Baz;
|
||||
| ^^^^
|
||||
|
||||
error: aborting due to 9 previous errors
|
||||
|
||||
|
|
@ -1 +1,11 @@
|
|||
arithmetic-side-effects-allowed = ["Point"]
|
||||
arithmetic-side-effects-allowed = [
|
||||
"OutOfNames"
|
||||
]
|
||||
arithmetic-side-effects-allowed-binary = [
|
||||
["Foo", "Foo"],
|
||||
["Foo", "i32"],
|
||||
["i32", "Foo"],
|
||||
["Bar", "*"],
|
||||
["*", "Bar"],
|
||||
]
|
||||
arithmetic-side-effects-allowed-unary = ["Foo"]
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
|
|||
allow-unwrap-in-tests
|
||||
allowed-scripts
|
||||
arithmetic-side-effects-allowed
|
||||
arithmetic-side-effects-allowed-binary
|
||||
arithmetic-side-effects-allowed-unary
|
||||
array-size-threshold
|
||||
avoid-breaking-exported-api
|
||||
await-holding-invalid-types
|
||||
|
|
|
|||
|
|
@ -1,28 +1,10 @@
|
|||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects.rs:78:13
|
||||
|
|
||||
LL | let _ = String::new() + "";
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects.rs:86:27
|
||||
|
|
||||
LL | let inferred_string = string + "";
|
||||
| ^^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects.rs:90:13
|
||||
|
|
||||
LL | let _ = inferred_string + "";
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects.rs:165:5
|
||||
|
|
||||
LL | _n += 1;
|
||||
| ^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
|
||||
|
||||
error: arithmetic operation that can potentially result in unexpected side-effects
|
||||
--> $DIR/arithmetic_side_effects.rs:166:5
|
||||
|
|
@ -348,5 +330,5 @@ error: arithmetic operation that can potentially result in unexpected side-effec
|
|||
LL | _n = -&_n;
|
||||
| ^^^^
|
||||
|
||||
error: aborting due to 58 previous errors
|
||||
error: aborting due to 55 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue