Skip to content

Commit 2550530

Browse files
authored
Extend precedence for bitmasking and shift (rust-lang#13743)
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 rust-lang#6632
2 parents 19426bf + 120b841 commit 2550530

File tree

4 files changed

+51
-16
lines changed

4 files changed

+51
-16
lines changed

clippy_lints/src/precedence.rs

+18-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
3+
use rustc_ast::ast::BinOpKind::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Shl, Shr, Sub};
34
use rustc_ast::ast::{BinOpKind, Expr, ExprKind};
45
use rustc_errors::Applicability;
56
use rustc_lint::{EarlyContext, EarlyLintPass};
@@ -12,6 +13,7 @@ declare_clippy_lint! {
1213
/// and suggests to add parentheses. Currently it catches the following:
1314
/// * mixed usage of arithmetic and bit shifting/combining operators without
1415
/// parentheses
16+
/// * mixed usage of bitmasking and bit shifting operators without parentheses
1517
///
1618
/// ### Why is this bad?
1719
/// Not everyone knows the precedence of those operators by
@@ -20,6 +22,7 @@ declare_clippy_lint! {
2022
///
2123
/// ### Example
2224
/// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
25+
/// * `0x2345 & 0xF000 >> 12` equals 5, while `(0x2345 & 0xF000) >> 12` equals 2
2326
#[clippy::version = "pre 1.29.0"]
2427
pub PRECEDENCE,
2528
complexity,
@@ -51,8 +54,13 @@ impl EarlyLintPass for Precedence {
5154
return;
5255
}
5356
let mut applicability = Applicability::MachineApplicable;
54-
match (is_arith_expr(left), is_arith_expr(right)) {
55-
(true, true) => {
57+
match (op, get_bin_opt(left), get_bin_opt(right)) {
58+
(
59+
BitAnd | BitOr | BitXor,
60+
Some(Shl | Shr | Add | Div | Mul | Rem | Sub),
61+
Some(Shl | Shr | Add | Div | Mul | Rem | Sub),
62+
)
63+
| (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), Some(Add | Div | Mul | Rem | Sub)) => {
5664
let sugg = format!(
5765
"({}) {} ({})",
5866
snippet_with_applicability(cx, left.span, "..", &mut applicability),
@@ -61,7 +69,8 @@ impl EarlyLintPass for Precedence {
6169
);
6270
span_sugg(expr, sugg, applicability);
6371
},
64-
(true, false) => {
72+
(BitAnd | BitOr | BitXor, Some(Shl | Shr | Add | Div | Mul | Rem | Sub), _)
73+
| (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), _) => {
6574
let sugg = format!(
6675
"({}) {} {}",
6776
snippet_with_applicability(cx, left.span, "..", &mut applicability),
@@ -70,7 +79,8 @@ impl EarlyLintPass for Precedence {
7079
);
7180
span_sugg(expr, sugg, applicability);
7281
},
73-
(false, true) => {
82+
(BitAnd | BitOr | BitXor, _, Some(Shl | Shr | Add | Div | Mul | Rem | Sub))
83+
| (Shl | Shr, _, Some(Add | Div | Mul | Rem | Sub)) => {
7484
let sugg = format!(
7585
"{} {} ({})",
7686
snippet_with_applicability(cx, left.span, "..", &mut applicability),
@@ -79,27 +89,20 @@ impl EarlyLintPass for Precedence {
7989
);
8090
span_sugg(expr, sugg, applicability);
8191
},
82-
(false, false) => (),
92+
_ => (),
8393
}
8494
}
8595
}
8696
}
8797

88-
fn is_arith_expr(expr: &Expr) -> bool {
98+
fn get_bin_opt(expr: &Expr) -> Option<BinOpKind> {
8999
match expr.kind {
90-
ExprKind::Binary(Spanned { node: op, .. }, _, _) => is_arith_op(op),
91-
_ => false,
100+
ExprKind::Binary(Spanned { node: op, .. }, _, _) => Some(op),
101+
_ => None,
92102
}
93103
}
94104

95105
#[must_use]
96106
fn is_bit_op(op: BinOpKind) -> bool {
97-
use rustc_ast::ast::BinOpKind::{BitAnd, BitOr, BitXor, Shl, Shr};
98107
matches!(op, BitXor | BitAnd | BitOr | Shl | Shr)
99108
}
100-
101-
#[must_use]
102-
fn is_arith_op(op: BinOpKind) -> bool {
103-
use rustc_ast::ast::BinOpKind::{Add, Div, Mul, Rem, Sub};
104-
matches!(op, Add | Sub | Mul | Div | Rem)
105-
}

tests/ui/precedence.fixed

+4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ fn main() {
2020
1 ^ (1 - 1);
2121
3 | (2 - 1);
2222
3 & (5 - 2);
23+
0x0F00 & (0x00F0 << 4);
24+
0x0F00 & (0xF000 >> 4);
25+
(0x0F00 << 1) ^ 3;
26+
(0x0F00 << 1) | 2;
2327

2428
let b = 3;
2529
trip!(b * 8);

tests/ui/precedence.rs

+4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ fn main() {
2020
1 ^ 1 - 1;
2121
3 | 2 - 1;
2222
3 & 5 - 2;
23+
0x0F00 & 0x00F0 << 4;
24+
0x0F00 & 0xF000 >> 4;
25+
0x0F00 << 1 ^ 3;
26+
0x0F00 << 1 | 2;
2327

2428
let b = 3;
2529
trip!(b * 8);

tests/ui/precedence.stderr

+25-1
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,29 @@ error: operator precedence can trip the unwary
4343
LL | 3 & 5 - 2;
4444
| ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)`
4545

46-
error: aborting due to 7 previous errors
46+
error: operator precedence can trip the unwary
47+
--> tests/ui/precedence.rs:23:5
48+
|
49+
LL | 0x0F00 & 0x00F0 << 4;
50+
| ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0x00F0 << 4)`
51+
52+
error: operator precedence can trip the unwary
53+
--> tests/ui/precedence.rs:24:5
54+
|
55+
LL | 0x0F00 & 0xF000 >> 4;
56+
| ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0xF000 >> 4)`
57+
58+
error: operator precedence can trip the unwary
59+
--> tests/ui/precedence.rs:25:5
60+
|
61+
LL | 0x0F00 << 1 ^ 3;
62+
| ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) ^ 3`
63+
64+
error: operator precedence can trip the unwary
65+
--> tests/ui/precedence.rs:26:5
66+
|
67+
LL | 0x0F00 << 1 | 2;
68+
| ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) | 2`
69+
70+
error: aborting due to 11 previous errors
4771

0 commit comments

Comments
 (0)