Skip to content

Commit

Permalink
Extend precedence for bitmasking and shift (#13743)
Browse files Browse the repository at this point in the history
Now we can lint for the expressions like `_&_>>_`, `_<<_^_`, etc. And
will suggest to add parentheses like `_&(_>>_)` and `(_<<_)^_`.
I get implementation suggestions from
[https://github.com/rust-lang/rust-clippy/pull/8735#pullrequestreview-954273477](https://github.com/rust-lang/rust-clippy/pull/8735#pullrequestreview-954273477).
changelog: extended [`precedence`] to lint for bit masking and bit
shifting without parentheses
fixes #6632
  • Loading branch information
Jarcho authored Dec 3, 2024
2 parents 19426bf + 120b841 commit 2550530
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 16 deletions.
33 changes: 18 additions & 15 deletions clippy_lints/src/precedence.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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)
}
4 changes: 4 additions & 0 deletions tests/ui/precedence.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ fn main() {
1 ^ (1 - 1);
3 | (2 - 1);
3 & (5 - 2);
0x0F00 & (0x00F0 << 4);
0x0F00 & (0xF000 >> 4);
(0x0F00 << 1) ^ 3;
(0x0F00 << 1) | 2;

let b = 3;
trip!(b * 8);
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/precedence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ fn main() {
1 ^ 1 - 1;
3 | 2 - 1;
3 & 5 - 2;
0x0F00 & 0x00F0 << 4;
0x0F00 & 0xF000 >> 4;
0x0F00 << 1 ^ 3;
0x0F00 << 1 | 2;

let b = 3;
trip!(b * 8);
Expand Down
26 changes: 25 additions & 1 deletion tests/ui/precedence.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 | 0x0F00 & 0x00F0 << 4;
| ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0x00F0 << 4)`

error: operator precedence can trip the unwary
--> tests/ui/precedence.rs:24:5
|
LL | 0x0F00 & 0xF000 >> 4;
| ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0xF000 >> 4)`

error: operator precedence can trip the unwary
--> tests/ui/precedence.rs:25:5
|
LL | 0x0F00 << 1 ^ 3;
| ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) ^ 3`

error: operator precedence can trip the unwary
--> tests/ui/precedence.rs:26:5
|
LL | 0x0F00 << 1 | 2;
| ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) | 2`

error: aborting due to 11 previous errors

0 comments on commit 2550530

Please sign in to comment.