diff options
| -rw-r--r-- | clippy_lints/src/precedence.rs | 33 | ||||
| -rw-r--r-- | tests/ui/precedence.fixed | 4 | ||||
| -rw-r--r-- | tests/ui/precedence.rs | 4 | ||||
| -rw-r--r-- | tests/ui/precedence.stderr | 26 |
4 files changed, 51 insertions, 16 deletions
diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs index 37f5dd5583b..031f0931059 100644 --- a/clippy_lints/src/precedence.rs +++ b/clippy_lints/src/precedence.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; +use rustc_ast::ast::BinOpKind::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Shl, Shr, Sub}; use rustc_ast::ast::{BinOpKind, Expr, ExprKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; @@ -12,6 +13,7 @@ declare_clippy_lint! { /// and suggests to add parentheses. Currently it catches the following: /// * mixed usage of arithmetic and bit shifting/combining operators without /// parentheses + /// * mixed usage of bitmasking and bit shifting operators without parentheses /// /// ### Why is this bad? /// Not everyone knows the precedence of those operators by @@ -20,6 +22,7 @@ declare_clippy_lint! { /// /// ### Example /// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7 + /// * `0x2345 & 0xF000 >> 12` equals 5, while `(0x2345 & 0xF000) >> 12` equals 2 #[clippy::version = "pre 1.29.0"] pub PRECEDENCE, complexity, @@ -51,8 +54,13 @@ impl EarlyLintPass for Precedence { return; } let mut applicability = Applicability::MachineApplicable; - match (is_arith_expr(left), is_arith_expr(right)) { - (true, true) => { + match (op, get_bin_opt(left), get_bin_opt(right)) { + ( + BitAnd | BitOr | BitXor, + Some(Shl | Shr | Add | Div | Mul | Rem | Sub), + Some(Shl | Shr | Add | Div | Mul | Rem | Sub), + ) + | (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), Some(Add | Div | Mul | Rem | Sub)) => { let sugg = format!( "({}) {} ({})", snippet_with_applicability(cx, left.span, "..", &mut applicability), @@ -61,7 +69,8 @@ impl EarlyLintPass for Precedence { ); span_sugg(expr, sugg, applicability); }, - (true, false) => { + (BitAnd | BitOr | BitXor, Some(Shl | Shr | Add | Div | Mul | Rem | Sub), _) + | (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), _) => { let sugg = format!( "({}) {} {}", snippet_with_applicability(cx, left.span, "..", &mut applicability), @@ -70,7 +79,8 @@ impl EarlyLintPass for Precedence { ); span_sugg(expr, sugg, applicability); }, - (false, true) => { + (BitAnd | BitOr | BitXor, _, Some(Shl | Shr | Add | Div | Mul | Rem | Sub)) + | (Shl | Shr, _, Some(Add | Div | Mul | Rem | Sub)) => { let sugg = format!( "{} {} ({})", snippet_with_applicability(cx, left.span, "..", &mut applicability), @@ -79,27 +89,20 @@ impl EarlyLintPass for Precedence { ); span_sugg(expr, sugg, applicability); }, - (false, false) => (), + _ => (), } } } } -fn is_arith_expr(expr: &Expr) -> bool { +fn get_bin_opt(expr: &Expr) -> Option<BinOpKind> { match expr.kind { - ExprKind::Binary(Spanned { node: op, .. }, _, _) => is_arith_op(op), - _ => false, + ExprKind::Binary(Spanned { node: op, .. }, _, _) => Some(op), + _ => None, } } #[must_use] fn is_bit_op(op: BinOpKind) -> bool { - use rustc_ast::ast::BinOpKind::{BitAnd, BitOr, BitXor, Shl, Shr}; matches!(op, BitXor | BitAnd | BitOr | Shl | Shr) } - -#[must_use] -fn is_arith_op(op: BinOpKind) -> bool { - use rustc_ast::ast::BinOpKind::{Add, Div, Mul, Rem, Sub}; - matches!(op, Add | Sub | Mul | Div | Rem) -} diff --git a/tests/ui/precedence.fixed b/tests/ui/precedence.fixed index c25c2062ace..01e8c3fb0bd 100644 --- a/tests/ui/precedence.fixed +++ b/tests/ui/precedence.fixed @@ -20,6 +20,10 @@ fn main() { 1 ^ (1 - 1); 3 | (2 - 1); 3 & (5 - 2); + 12 & (0xF000 << 4); + 12 & (0xF000 >> 4); + (12 << 4) ^ 0xF000; + (12 << 4) | 0xF000; let b = 3; trip!(b * 8); diff --git a/tests/ui/precedence.rs b/tests/ui/precedence.rs index dc242ecf4c7..9937d96592e 100644 --- a/tests/ui/precedence.rs +++ b/tests/ui/precedence.rs @@ -20,6 +20,10 @@ fn main() { 1 ^ 1 - 1; 3 | 2 - 1; 3 & 5 - 2; + 12 & 0xF000 << 4; + 12 & 0xF000 >> 4; + 12 << 4 ^ 0xF000; + 12 << 4 | 0xF000; let b = 3; trip!(b * 8); diff --git a/tests/ui/precedence.stderr b/tests/ui/precedence.stderr index 8057c25a5e4..b1226e97aeb 100644 --- a/tests/ui/precedence.stderr +++ b/tests/ui/precedence.stderr @@ -43,5 +43,29 @@ error: operator precedence can trip the unwary LL | 3 & 5 - 2; | ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)` -error: aborting due to 7 previous errors +error: operator precedence can trip the unwary + --> tests/ui/precedence.rs:23:5 + | +LL | 12 & 0xF000 << 4; + | ^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `12 & (0xF000 << 4)` + +error: operator precedence can trip the unwary + --> tests/ui/precedence.rs:24:5 + | +LL | 12 & 0xF000 >> 4; + | ^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `12 & (0xF000 >> 4)` + +error: operator precedence can trip the unwary + --> tests/ui/precedence.rs:25:5 + | +LL | 12 << 4 ^ 0xF000; + | ^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(12 << 4) ^ 0xF000` + +error: operator precedence can trip the unwary + --> tests/ui/precedence.rs:26:5 + | +LL | 12 << 4 | 0xF000; + | ^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(12 << 4) | 0xF000` + +error: aborting due to 11 previous errors |