Skip to content

Commit

Permalink
Remove unary neg from clippy::precedence lint
Browse files Browse the repository at this point in the history
  • Loading branch information
@Urgau
Urgau committed Jul 10, 2024
1 parent 920cbcd commit 5f3a6e1
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 156 deletions.
56 changes: 1 addition & 55 deletions clippy_lints/src/precedence.rs
View file
Original file line number Diff line number Diff line change
@@ -1,38 +1,17 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use rustc_ast::ast::{BinOpKind, Expr, ExprKind, MethodCall, UnOp};
use rustc_ast::token;
use rustc_ast::ast::{BinOpKind, Expr, ExprKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::source_map::Spanned;

const ALLOWED_ODD_FUNCTIONS: [&str; 14] = [
"asin",
"asinh",
"atan",
"atanh",
"cbrt",
"fract",
"round",
"signum",
"sin",
"sinh",
"tan",
"tanh",
"to_degrees",
"to_radians",
];

declare_clippy_lint! {
/// ### What it does
/// Checks for operations where precedence may be unclear
/// and suggests to add parentheses. Currently it catches the following:
/// * mixed usage of arithmetic and bit shifting/combining operators without
/// parentheses
/// * a "negative" numeric literal (which is really a unary `-` followed by a
/// numeric literal)
/// followed by a method call
///
/// ### Why is this bad?
/// Not everyone knows the precedence of those operators by
Expand All @@ -41,7 +20,6 @@ declare_clippy_lint! {
///
/// ### Example
/// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
/// * `-1i32.abs()` equals -1, while `(-1i32).abs()` equals 1
#[clippy::version = "pre 1.29.0"]
pub PRECEDENCE,
complexity,
Expand Down Expand Up @@ -104,38 +82,6 @@ impl EarlyLintPass for Precedence {
(false, false) => (),
}
}

if let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind {
let mut arg = operand;

let mut all_odd = true;
while let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &arg.kind {
let seg_str = seg.ident.name.as_str();
all_odd &= ALLOWED_ODD_FUNCTIONS
.iter()
.any(|odd_function| **odd_function == *seg_str);
arg = receiver;
}

if !all_odd
&& let ExprKind::Lit(lit) = &arg.kind
&& let token::LitKind::Integer | token::LitKind::Float = &lit.kind
{
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
PRECEDENCE,
expr.span,
"unary minus has lower precedence than method call",
"consider adding parentheses to clarify your intent",
format!(
"-({})",
snippet_with_applicability(cx, operand.span, "..", &mut applicability)
),
applicability,
);
}
}
}
}

Expand Down
34 changes: 0 additions & 34 deletions tests/ui/precedence.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,6 @@ fn main() {
1 ^ (1 - 1);
3 | (2 - 1);
3 & (5 - 2);
-(1i32.abs());
-(1f32.abs());

// These should not trigger an error
let _ = (-1i32).abs();
let _ = (-1f32).abs();
let _ = -(1i32).abs();
let _ = -(1f32).abs();
let _ = -(1i32.abs());
let _ = -(1f32.abs());

// Odd functions should not trigger an error
let _ = -1f64.asin();
let _ = -1f64.asinh();
let _ = -1f64.atan();
let _ = -1f64.atanh();
let _ = -1f64.cbrt();
let _ = -1f64.fract();
let _ = -1f64.round();
let _ = -1f64.signum();
let _ = -1f64.sin();
let _ = -1f64.sinh();
let _ = -1f64.tan();
let _ = -1f64.tanh();
let _ = -1f64.to_degrees();
let _ = -1f64.to_radians();

// Chains containing any non-odd function should trigger (issue #5924)
let _ = -(1.0_f64.cos().cos());
let _ = -(1.0_f64.cos().sin());
let _ = -(1.0_f64.sin().cos());

// Chains of odd functions shouldn't trigger
let _ = -1f64.sin().sin();

let b = 3;
trip!(b * 8);
Expand Down
34 changes: 0 additions & 34 deletions tests/ui/precedence.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,6 @@ fn main() {
1 ^ 1 - 1;
3 | 2 - 1;
3 & 5 - 2;
-1i32.abs();
-1f32.abs();

// These should not trigger an error
let _ = (-1i32).abs();
let _ = (-1f32).abs();
let _ = -(1i32).abs();
let _ = -(1f32).abs();
let _ = -(1i32.abs());
let _ = -(1f32.abs());

// Odd functions should not trigger an error
let _ = -1f64.asin();
let _ = -1f64.asinh();
let _ = -1f64.atan();
let _ = -1f64.atanh();
let _ = -1f64.cbrt();
let _ = -1f64.fract();
let _ = -1f64.round();
let _ = -1f64.signum();
let _ = -1f64.sin();
let _ = -1f64.sinh();
let _ = -1f64.tan();
let _ = -1f64.tanh();
let _ = -1f64.to_degrees();
let _ = -1f64.to_radians();

// Chains containing any non-odd function should trigger (issue #5924)
let _ = -1.0_f64.cos().cos();
let _ = -1.0_f64.cos().sin();
let _ = -1.0_f64.sin().cos();

// Chains of odd functions shouldn't trigger
let _ = -1f64.sin().sin();

let b = 3;
trip!(b * 8);
Expand Down
32 changes: 1 addition & 31 deletions tests/ui/precedence.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,35 +43,5 @@ error: operator precedence can trip the unwary
LL | 3 & 5 - 2;
| ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)`

error: unary minus has lower precedence than method call
--> tests/ui/precedence.rs:23:5
|
LL | -1i32.abs();
| ^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1i32.abs())`

error: unary minus has lower precedence than method call
--> tests/ui/precedence.rs:24:5
|
LL | -1f32.abs();
| ^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1f32.abs())`

error: unary minus has lower precedence than method call
--> tests/ui/precedence.rs:51:13
|
LL | let _ = -1.0_f64.cos().cos();
| ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().cos())`

error: unary minus has lower precedence than method call
--> tests/ui/precedence.rs:52:13
|
LL | let _ = -1.0_f64.cos().sin();
| ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().sin())`

error: unary minus has lower precedence than method call
--> tests/ui/precedence.rs:53:13
|
LL | let _ = -1.0_f64.sin().cos();
| ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.sin().cos())`

error: aborting due to 12 previous errors
error: aborting due to 7 previous errors

2 changes: 1 addition & 1 deletion tests/ui/unnecessary_cast.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ mod fixable {

fn issue_9563() {
let _: f64 = (-8.0_f64).exp();
#[allow(clippy::precedence)]
#[allow(ambiguous_negative_literals)]
let _: f64 = -8.0_f64.exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/unnecessary_cast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ mod fixable {

fn issue_9563() {
let _: f64 = (-8.0 as f64).exp();
#[allow(clippy::precedence)]
#[allow(ambiguous_negative_literals)]
let _: f64 = -(8.0 as f64).exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior
}

Expand Down

0 comments on commit 5f3a6e1

Please sign in to comment.