diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-07-25 16:48:17 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-07-25 16:48:17 +0200 |
| commit | 36214e9838bfecd30c6554f99e012bea3681c7ae (patch) | |
| tree | f4eba59410fc85392bd172b24c84b123a4d5a935 | |
| parent | 6d674685ae4e9156dbb6ecd3aa38d87864ecab3e (diff) | |
| parent | 5f3a6e1805dc59417fbc79734b97f4dc532d4484 (diff) | |
| download | rust-36214e9838bfecd30c6554f99e012bea3681c7ae.tar.gz rust-36214e9838bfecd30c6554f99e012bea3681c7ae.zip | |
Rollup merge of #121364 - Urgau:unary_precedence, r=compiler-errors
Implement lint against ambiguous negative literals This PR implements a lint against ambiguous negative literals with a literal and method calls right after it. ## `ambiguous_negative_literals` (deny-by-default) The `ambiguous_negative_literals` lint checks for cases that are confusing between a negative literal and a negation that's not part of the literal. ### Example ```rust,compile_fail -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1 ``` ### Explanation Method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs. <details> <summary>Old proposed lint</summary> ## `ambiguous_unary_precedence` (deny-by-default) The `ambiguous_unary_precedence` lint checks for use the negative unary operator with a literal and method calls. ### Example ```rust -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1 ``` ### Explanation Unary operations take precedence on binary operations and method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs. </details> ----- Note: This is a strip down version of https://github.com/rust-lang/rust/pull/117161, without the binary op precedence. Fixes https://github.com/rust-lang/rust/issues/117155 `@rustbot` labels +I-lang-nominated cc `@scottmcm` r? compiler
| -rw-r--r-- | clippy_lints/src/precedence.rs | 56 | ||||
| -rw-r--r-- | tests/ui/precedence.fixed | 34 | ||||
| -rw-r--r-- | tests/ui/precedence.rs | 34 | ||||
| -rw-r--r-- | tests/ui/precedence.stderr | 32 | ||||
| -rw-r--r-- | tests/ui/unnecessary_cast.fixed | 2 | ||||
| -rw-r--r-- | tests/ui/unnecessary_cast.rs | 2 |
6 files changed, 4 insertions, 156 deletions
diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs index ff83725da69..37f5dd5583b 100644 --- a/clippy_lints/src/precedence.rs +++ b/clippy_lints/src/precedence.rs @@ -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 @@ -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, @@ -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, - ); - } - } } } diff --git a/tests/ui/precedence.fixed b/tests/ui/precedence.fixed index cc87de0d90f..c25c2062ace 100644 --- a/tests/ui/precedence.fixed +++ b/tests/ui/precedence.fixed @@ -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); diff --git a/tests/ui/precedence.rs b/tests/ui/precedence.rs index 00c18d92b5f..dc242ecf4c7 100644 --- a/tests/ui/precedence.rs +++ b/tests/ui/precedence.rs @@ -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); diff --git a/tests/ui/precedence.stderr b/tests/ui/precedence.stderr index 47e61326219..8057c25a5e4 100644 --- a/tests/ui/precedence.stderr +++ b/tests/ui/precedence.stderr @@ -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 diff --git a/tests/ui/unnecessary_cast.fixed b/tests/ui/unnecessary_cast.fixed index 288541362cd..c43e50761bd 100644 --- a/tests/ui/unnecessary_cast.fixed +++ b/tests/ui/unnecessary_cast.fixed @@ -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 } diff --git a/tests/ui/unnecessary_cast.rs b/tests/ui/unnecessary_cast.rs index eef3a42e351..4a5ca231315 100644 --- a/tests/ui/unnecessary_cast.rs +++ b/tests/ui/unnecessary_cast.rs @@ -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 } |
