From 5ad37b1a4b34af381b93f603e4c948928a8a6124 Mon Sep 17 00:00:00 2001 From: Oussama Date: Sun, 19 Dec 2021 09:48:25 +0100 Subject: [PATCH 1/4] add suggestion for neg_multiply lint --- clippy_lints/src/neg_multiply.rs | 26 ++++++++++++++++++++++---- tests/ui/neg_multiply.stderr | 8 ++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index cb67fab17400..78645e50bffc 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -1,7 +1,8 @@ use clippy_utils::consts::{self, Constant}; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_sugg; use if_chain::if_chain; -use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -18,7 +19,11 @@ declare_clippy_lint! { /// /// ### Example /// ```ignore - /// x * -1 + /// // Bad + /// let a = x * -1; + /// + /// // Good + /// let b = -x; /// ``` #[clippy::version = "pre 1.29.0"] pub NEG_MULTIPLY, @@ -49,8 +54,21 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { if let ExprKind::Lit(ref l) = lit.kind; if consts::lit_to_constant(&l.node, cx.typeck_results().expr_ty_opt(lit)) == Constant::Int(1); if cx.typeck_results().expr_ty(exp).is_integral(); + if let ExprKind::Path(QPath::Resolved(_, var_path)) = exp.kind; + then { - span_lint(cx, NEG_MULTIPLY, span, "negation by multiplying with `-1`"); + let var_name = var_path.segments[0].ident.name.as_str(); + let suggestion = format!("-{var}",var=var_name); + let applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + NEG_MULTIPLY, + span, + "this `multiplication with -1` can be written more succinctly", + "consider using", + suggestion, + applicability, + ); } } } diff --git a/tests/ui/neg_multiply.stderr b/tests/ui/neg_multiply.stderr index ad677f6d6fb9..ce5f46fca56c 100644 --- a/tests/ui/neg_multiply.stderr +++ b/tests/ui/neg_multiply.stderr @@ -1,16 +1,16 @@ -error: negation by multiplying with `-1` +error: this `multiplication with -1` can be written more succinctly --> $DIR/neg_multiply.rs:27:5 | LL | x * -1; - | ^^^^^^ + | ^^^^^^ help: consider using: `-x` | = note: `-D clippy::neg-multiply` implied by `-D warnings` -error: negation by multiplying with `-1` +error: this `multiplication with -1` can be written more succinctly --> $DIR/neg_multiply.rs:29:5 | LL | -1 * x; - | ^^^^^^ + | ^^^^^^ help: consider using: `-x` error: aborting due to 2 previous errors From 88e40bc73db441b931372a11b99c308f4bf0427f Mon Sep 17 00:00:00 2001 From: Oussama Date: Tue, 21 Dec 2021 22:00:14 +0100 Subject: [PATCH 2/4] Add support for suggestion when using an expression --- clippy_lints/src/neg_multiply.rs | 13 +++++----- tests/ui/neg_multiply.fixed | 44 ++++++++++++++++++++++++++++++++ tests/ui/neg_multiply.rs | 9 +++++++ tests/ui/neg_multiply.stderr | 42 ++++++++++++++++++++++++++---- 4 files changed, 96 insertions(+), 12 deletions(-) create mode 100644 tests/ui/neg_multiply.fixed diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index 78645e50bffc..0d05c83ffe45 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -1,8 +1,9 @@ use clippy_utils::consts::{self, Constant}; use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, UnOp}; +use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -28,7 +29,7 @@ declare_clippy_lint! { #[clippy::version = "pre 1.29.0"] pub NEG_MULTIPLY, style, - "multiplying integers with `-1`" + "multiplying integers by `-1`" } declare_lint_pass!(NegMultiply => [NEG_MULTIPLY]); @@ -54,17 +55,15 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { if let ExprKind::Lit(ref l) = lit.kind; if consts::lit_to_constant(&l.node, cx.typeck_results().expr_ty_opt(lit)) == Constant::Int(1); if cx.typeck_results().expr_ty(exp).is_integral(); - if let ExprKind::Path(QPath::Resolved(_, var_path)) = exp.kind; then { - let var_name = var_path.segments[0].ident.name.as_str(); - let suggestion = format!("-{var}",var=var_name); - let applicability = Applicability::MachineApplicable; + let mut applicability = Applicability::MachineApplicable; + let suggestion = format!("-{}", snippet_with_applicability(cx, exp.span, "..", &mut applicability)); span_lint_and_sugg( cx, NEG_MULTIPLY, span, - "this `multiplication with -1` can be written more succinctly", + "this multiplication by -1 can be written more succinctly", "consider using", suggestion, applicability, diff --git a/tests/ui/neg_multiply.fixed b/tests/ui/neg_multiply.fixed new file mode 100644 index 000000000000..8546173cfd54 --- /dev/null +++ b/tests/ui/neg_multiply.fixed @@ -0,0 +1,44 @@ +// run-rustfix +#![warn(clippy::neg_multiply)] +#![allow(clippy::no_effect, clippy::unnecessary_operation)] + +use std::ops::Mul; + +struct X; + +impl Mul for X { + type Output = X; + + fn mul(self, _r: isize) -> Self { + self + } +} + +impl Mul for isize { + type Output = X; + + fn mul(self, _r: X) -> X { + X + } +} + +fn main() { + let x = 0; + + -x; + + -x; + + 100 + -x; + + -(100 + x); + + -17; + + 0xcafe | -0xff00; + + -1 * -1; // should be ok + + X * -1; // should be ok + -1 * X; // should also be ok +} diff --git a/tests/ui/neg_multiply.rs b/tests/ui/neg_multiply.rs index d4a20ce9db1c..9f48e4d9c127 100644 --- a/tests/ui/neg_multiply.rs +++ b/tests/ui/neg_multiply.rs @@ -1,3 +1,4 @@ +// run-rustfix #![warn(clippy::neg_multiply)] #![allow(clippy::no_effect, clippy::unnecessary_operation)] @@ -28,6 +29,14 @@ fn main() { -1 * x; + 100 + x * -1; + + (100 + x) * -1; + + -1 * 17; + + 0xcafe | 0xff00 * -1; + -1 * -1; // should be ok X * -1; // should be ok diff --git a/tests/ui/neg_multiply.stderr b/tests/ui/neg_multiply.stderr index ce5f46fca56c..0a4bc0742958 100644 --- a/tests/ui/neg_multiply.stderr +++ b/tests/ui/neg_multiply.stderr @@ -1,16 +1,48 @@ -error: this `multiplication with -1` can be written more succinctly - --> $DIR/neg_multiply.rs:27:5 +error: operator precedence can trip the unwary + --> $DIR/neg_multiply.rs:38:5 + | +LL | 0xcafe | 0xff00 * -1; + | ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0xcafe | (0xff00 * -1)` + | + = note: `-D clippy::precedence` implied by `-D warnings` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:28:5 | LL | x * -1; | ^^^^^^ help: consider using: `-x` | = note: `-D clippy::neg-multiply` implied by `-D warnings` -error: this `multiplication with -1` can be written more succinctly - --> $DIR/neg_multiply.rs:29:5 +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:30:5 | LL | -1 * x; | ^^^^^^ help: consider using: `-x` -error: aborting due to 2 previous errors +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:32:11 + | +LL | 100 + x * -1; + | ^^^^^^ help: consider using: `-x` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:34:5 + | +LL | (100 + x) * -1; + | ^^^^^^^^^^^^^^ help: consider using: `-(100 + x)` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:36:5 + | +LL | -1 * 17; + | ^^^^^^^ help: consider using: `-17` + +error: this multiplication by -1 can be written more succinctly + --> $DIR/neg_multiply.rs:38:14 + | +LL | 0xcafe | 0xff00 * -1; + | ^^^^^^^^^^^ help: consider using: `-0xff00` + +error: aborting due to 7 previous errors From dce315187264a0c57f5f6fdc12a1878f8b3b6ec1 Mon Sep 17 00:00:00 2001 From: Oussama Date: Thu, 23 Dec 2021 09:22:29 +0100 Subject: [PATCH 3/4] Add allow precedence lint to prevent rustfix from failing --- tests/ui/neg_multiply.fixed | 2 +- tests/ui/neg_multiply.rs | 2 +- tests/ui/neg_multiply.stderr | 10 +--------- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/ui/neg_multiply.fixed b/tests/ui/neg_multiply.fixed index 8546173cfd54..e4aa7fbadfae 100644 --- a/tests/ui/neg_multiply.fixed +++ b/tests/ui/neg_multiply.fixed @@ -1,6 +1,6 @@ // run-rustfix #![warn(clippy::neg_multiply)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] use std::ops::Mul; diff --git a/tests/ui/neg_multiply.rs b/tests/ui/neg_multiply.rs index 9f48e4d9c127..e95da705dca9 100644 --- a/tests/ui/neg_multiply.rs +++ b/tests/ui/neg_multiply.rs @@ -1,6 +1,6 @@ // run-rustfix #![warn(clippy::neg_multiply)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] use std::ops::Mul; diff --git a/tests/ui/neg_multiply.stderr b/tests/ui/neg_multiply.stderr index 0a4bc0742958..42d5dffb6a56 100644 --- a/tests/ui/neg_multiply.stderr +++ b/tests/ui/neg_multiply.stderr @@ -1,11 +1,3 @@ -error: operator precedence can trip the unwary - --> $DIR/neg_multiply.rs:38:5 - | -LL | 0xcafe | 0xff00 * -1; - | ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0xcafe | (0xff00 * -1)` - | - = note: `-D clippy::precedence` implied by `-D warnings` - error: this multiplication by -1 can be written more succinctly --> $DIR/neg_multiply.rs:28:5 | @@ -44,5 +36,5 @@ error: this multiplication by -1 can be written more succinctly LL | 0xcafe | 0xff00 * -1; | ^^^^^^^^^^^ help: consider using: `-0xff00` -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors From 13cc4522860a70a3dc1a4c0e529be40032725c5c Mon Sep 17 00:00:00 2001 From: Oussama Date: Thu, 23 Dec 2021 10:45:16 +0100 Subject: [PATCH 4/4] Add allow unused --- tests/ui/neg_multiply.fixed | 1 + tests/ui/neg_multiply.rs | 1 + tests/ui/neg_multiply.stderr | 12 ++++++------ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/ui/neg_multiply.fixed b/tests/ui/neg_multiply.fixed index e4aa7fbadfae..35af9d6ae317 100644 --- a/tests/ui/neg_multiply.fixed +++ b/tests/ui/neg_multiply.fixed @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::neg_multiply)] #![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] +#![allow(unused)] use std::ops::Mul; diff --git a/tests/ui/neg_multiply.rs b/tests/ui/neg_multiply.rs index e95da705dca9..7dbdb0906cee 100644 --- a/tests/ui/neg_multiply.rs +++ b/tests/ui/neg_multiply.rs @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::neg_multiply)] #![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::precedence)] +#![allow(unused)] use std::ops::Mul; diff --git a/tests/ui/neg_multiply.stderr b/tests/ui/neg_multiply.stderr index 42d5dffb6a56..dbf8fb36938c 100644 --- a/tests/ui/neg_multiply.stderr +++ b/tests/ui/neg_multiply.stderr @@ -1,5 +1,5 @@ error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:28:5 + --> $DIR/neg_multiply.rs:29:5 | LL | x * -1; | ^^^^^^ help: consider using: `-x` @@ -7,31 +7,31 @@ LL | x * -1; = note: `-D clippy::neg-multiply` implied by `-D warnings` error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:30:5 + --> $DIR/neg_multiply.rs:31:5 | LL | -1 * x; | ^^^^^^ help: consider using: `-x` error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:32:11 + --> $DIR/neg_multiply.rs:33:11 | LL | 100 + x * -1; | ^^^^^^ help: consider using: `-x` error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:34:5 + --> $DIR/neg_multiply.rs:35:5 | LL | (100 + x) * -1; | ^^^^^^^^^^^^^^ help: consider using: `-(100 + x)` error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:36:5 + --> $DIR/neg_multiply.rs:37:5 | LL | -1 * 17; | ^^^^^^^ help: consider using: `-17` error: this multiplication by -1 can be written more succinctly - --> $DIR/neg_multiply.rs:38:14 + --> $DIR/neg_multiply.rs:39:14 | LL | 0xcafe | 0xff00 * -1; | ^^^^^^^^^^^ help: consider using: `-0xff00`