about summary refs log tree commit diff
diff options
context:
space:
mode:
authorllogiq <bogusandre@gmail.com>2025-01-30 20:39:07 +0000
committerGitHub <noreply@github.com>2025-01-30 20:39:07 +0000
commit398a5c2db9d16ed29b322ad431f7b8f2fdf633cd (patch)
treec740bcd5dfb0e0abc966288243d7277e8e63b771
parentf51e18de3057d211a96edb39a71a17ce18f0ebee (diff)
parent8db9ecfd74732f4227421fd9bbf3c7704f594cbe (diff)
downloadrust-398a5c2db9d16ed29b322ad431f7b8f2fdf633cd.tar.gz
rust-398a5c2db9d16ed29b322ad431f7b8f2fdf633cd.zip
New lint: `precedence_bits`, with recent additions to `precedence` (#14115)
Commit 25505302665a707bedee68ca1f3faf2a09f12c00 has extended the
`precedence` lint to include bitmasking and shift operations. The lint
is warn by default, and this generates many hits, especially in embedded
or system code, where it is very idiomatic to use expressions such as `1
<< 3 | 1 << 5` without parentheses.

This commit splits the recent addition into a new lint, which is put
into the "restriction" category, while the original one stays in
"complexity", because mixing bitmasking and arithmetic operations is
less typical.

Fix #14097

changelog: [`precedence_bits`]: new lint
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/precedence.rs68
-rw-r--r--tests/ui/precedence.fixed8
-rw-r--r--tests/ui/precedence.stderr26
-rw-r--r--tests/ui/precedence_bits.fixed35
-rw-r--r--tests/ui/precedence_bits.rs35
-rw-r--r--tests/ui/precedence_bits.stderr29
8 files changed, 153 insertions, 50 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cb149ccfeea..bae091efff0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5954,6 +5954,7 @@ Released 2018-09-13
 [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters
 [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
 [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
+[`precedence_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence_bits
 [`print_in_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_in_format_impl
 [`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
 [`print_stderr`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stderr
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 1ce6f125617..af473b0c2ed 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -627,6 +627,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
     crate::pointers_in_nomem_asm_block::POINTERS_IN_NOMEM_ASM_BLOCK_INFO,
     crate::precedence::PRECEDENCE_INFO,
+    crate::precedence::PRECEDENCE_BITS_INFO,
     crate::ptr::CMP_NULL_INFO,
     crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
     crate::ptr::MUT_FROM_REF_INFO,
diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs
index 421b2b74755..ec6835db897 100644
--- a/clippy_lints/src/precedence.rs
+++ b/clippy_lints/src/precedence.rs
@@ -3,17 +3,14 @@ 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};
+use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
 use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
 
 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
-    /// * mixed usage of bitmasking and bit shifting operators without parentheses
+    /// Checks for operations where precedence may be unclear and suggests to add parentheses.
+    /// It catches a mixed usage of arithmetic and bit shifting/combining operators without parentheses
     ///
     /// ### Why is this bad?
     /// Not everyone knows the precedence of those operators by
@@ -21,15 +18,32 @@ declare_clippy_lint! {
     /// code.
     ///
     /// ### Example
-    /// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
-    /// * `0x2345 & 0xF000 >> 12` equals 5, while `(0x2345 & 0xF000) >> 12` equals 2
+    /// `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
     #[clippy::version = "pre 1.29.0"]
     pub PRECEDENCE,
     complexity,
     "operations where precedence may be unclear"
 }
 
-declare_lint_pass!(Precedence => [PRECEDENCE]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for bit shifting operations combined with bit masking/combining operators
+    /// and suggest using parentheses.
+    ///
+    /// ### Why restrict this?
+    /// Not everyone knows the precedence of those operators by
+    /// heart, so expressions like these may trip others trying to reason about the
+    /// code.
+    ///
+    /// ### Example
+    /// `0x2345 & 0xF000 >> 12` equals 5, while `(0x2345 & 0xF000) >> 12` equals 2
+    #[clippy::version = "1.86.0"]
+    pub PRECEDENCE_BITS,
+    restriction,
+    "operations mixing bit shifting with bit combining/masking"
+}
+
+declare_lint_pass!(Precedence => [PRECEDENCE, PRECEDENCE_BITS]);
 
 impl EarlyLintPass for Precedence {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
@@ -38,10 +52,10 @@ impl EarlyLintPass for Precedence {
         }
 
         if let ExprKind::Binary(Spanned { node: op, .. }, ref left, ref right) = expr.kind {
-            let span_sugg = |expr: &Expr, sugg, appl| {
+            let span_sugg = |lint: &'static Lint, expr: &Expr, sugg, appl| {
                 span_lint_and_sugg(
                     cx,
-                    PRECEDENCE,
+                    lint,
                     expr.span,
                     "operator precedence might not be obvious",
                     "consider parenthesizing your expression",
@@ -57,37 +71,41 @@ impl EarlyLintPass for Precedence {
             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),
+                    Some(left_op @ (Shl | Shr | Add | Div | Mul | Rem | Sub)),
+                    Some(right_op @ (Shl | Shr | Add | Div | Mul | Rem | Sub)),
                 )
-                | (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), Some(Add | Div | Mul | Rem | Sub)) => {
+                | (
+                    Shl | Shr,
+                    Some(left_op @ (Add | Div | Mul | Rem | Sub)),
+                    Some(right_op @ (Add | Div | Mul | Rem | Sub)),
+                ) => {
                     let sugg = format!(
                         "({}) {} ({})",
                         snippet_with_applicability(cx, left.span, "..", &mut applicability),
                         op.as_str(),
                         snippet_with_applicability(cx, right.span, "..", &mut applicability)
                     );
-                    span_sugg(expr, sugg, applicability);
+                    span_sugg(lint_for(&[op, left_op, right_op]), expr, sugg, applicability);
                 },
-                (BitAnd | BitOr | BitXor, Some(Shl | Shr | Add | Div | Mul | Rem | Sub), _)
-                | (Shl | Shr, Some(Add | Div | Mul | Rem | Sub), _) => {
+                (BitAnd | BitOr | BitXor, Some(side_op @ (Shl | Shr | Add | Div | Mul | Rem | Sub)), _)
+                | (Shl | Shr, Some(side_op @ (Add | Div | Mul | Rem | Sub)), _) => {
                     let sugg = format!(
                         "({}) {} {}",
                         snippet_with_applicability(cx, left.span, "..", &mut applicability),
                         op.as_str(),
                         snippet_with_applicability(cx, right.span, "..", &mut applicability)
                     );
-                    span_sugg(expr, sugg, applicability);
+                    span_sugg(lint_for(&[op, side_op]), expr, sugg, applicability);
                 },
-                (BitAnd | BitOr | BitXor, _, Some(Shl | Shr | Add | Div | Mul | Rem | Sub))
-                | (Shl | Shr, _, Some(Add | Div | Mul | Rem | Sub)) => {
+                (BitAnd | BitOr | BitXor, _, Some(side_op @ (Shl | Shr | Add | Div | Mul | Rem | Sub)))
+                | (Shl | Shr, _, Some(side_op @ (Add | Div | Mul | Rem | Sub))) => {
                     let sugg = format!(
                         "{} {} ({})",
                         snippet_with_applicability(cx, left.span, "..", &mut applicability),
                         op.as_str(),
                         snippet_with_applicability(cx, right.span, "..", &mut applicability)
                     );
-                    span_sugg(expr, sugg, applicability);
+                    span_sugg(lint_for(&[op, side_op]), expr, sugg, applicability);
                 },
                 _ => (),
             }
@@ -106,3 +124,11 @@ fn get_bin_opt(expr: &Expr) -> Option<BinOpKind> {
 fn is_bit_op(op: BinOpKind) -> bool {
     matches!(op, BitXor | BitAnd | BitOr | Shl | Shr)
 }
+
+fn lint_for(ops: &[BinOpKind]) -> &'static Lint {
+    if ops.iter().all(|op| is_bit_op(*op)) {
+        PRECEDENCE_BITS
+    } else {
+        PRECEDENCE
+    }
+}
diff --git a/tests/ui/precedence.fixed b/tests/ui/precedence.fixed
index 9864dd2550b..52144a18bac 100644
--- a/tests/ui/precedence.fixed
+++ b/tests/ui/precedence.fixed
@@ -20,10 +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;
+    0x0F00 & 0x00F0 << 4;
+    0x0F00 & 0xF000 >> 4;
+    0x0F00 << 1 ^ 3;
+    0x0F00 << 1 | 2;
 
     let b = 3;
     trip!(b * 8);
diff --git a/tests/ui/precedence.stderr b/tests/ui/precedence.stderr
index 329422cb8a6..68ad5cb4829 100644
--- a/tests/ui/precedence.stderr
+++ b/tests/ui/precedence.stderr
@@ -43,29 +43,5 @@ error: operator precedence might not be obvious
 LL |     3 & 5 - 2;
    |     ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)`
 
-error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:23:5
-   |
-LL |     0x0F00 & 0x00F0 << 4;
-   |     ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0x00F0 << 4)`
-
-error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:24:5
-   |
-LL |     0x0F00 & 0xF000 >> 4;
-   |     ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0xF000 >> 4)`
-
-error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:25:5
-   |
-LL |     0x0F00 << 1 ^ 3;
-   |     ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) ^ 3`
-
-error: operator precedence might not be obvious
-  --> 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
+error: aborting due to 7 previous errors
 
diff --git a/tests/ui/precedence_bits.fixed b/tests/ui/precedence_bits.fixed
new file mode 100644
index 00000000000..82fea0d14e4
--- /dev/null
+++ b/tests/ui/precedence_bits.fixed
@@ -0,0 +1,35 @@
+#![warn(clippy::precedence_bits)]
+#![allow(
+    unused_must_use,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::precedence
+)]
+#![allow(clippy::identity_op)]
+#![allow(clippy::eq_op)]
+
+macro_rules! trip {
+    ($a:expr) => {
+        match $a & 0b1111_1111u8 {
+            0 => println!("a is zero ({})", $a),
+            _ => println!("a is {}", $a),
+        }
+    };
+}
+
+fn main() {
+    1 << 2 + 3;
+    1 + 2 << 3;
+    4 >> 1 + 1;
+    1 + 3 >> 2;
+    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);
+}
diff --git a/tests/ui/precedence_bits.rs b/tests/ui/precedence_bits.rs
new file mode 100644
index 00000000000..9b353308b6e
--- /dev/null
+++ b/tests/ui/precedence_bits.rs
@@ -0,0 +1,35 @@
+#![warn(clippy::precedence_bits)]
+#![allow(
+    unused_must_use,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::precedence
+)]
+#![allow(clippy::identity_op)]
+#![allow(clippy::eq_op)]
+
+macro_rules! trip {
+    ($a:expr) => {
+        match $a & 0b1111_1111u8 {
+            0 => println!("a is zero ({})", $a),
+            _ => println!("a is {}", $a),
+        }
+    };
+}
+
+fn main() {
+    1 << 2 + 3;
+    1 + 2 << 3;
+    4 >> 1 + 1;
+    1 + 3 >> 2;
+    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);
+}
diff --git a/tests/ui/precedence_bits.stderr b/tests/ui/precedence_bits.stderr
new file mode 100644
index 00000000000..f468186b363
--- /dev/null
+++ b/tests/ui/precedence_bits.stderr
@@ -0,0 +1,29 @@
+error: operator precedence might not be obvious
+  --> tests/ui/precedence_bits.rs:28:5
+   |
+LL |     0x0F00 & 0x00F0 << 4;
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0x00F0 << 4)`
+   |
+   = note: `-D clippy::precedence-bits` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::precedence_bits)]`
+
+error: operator precedence might not be obvious
+  --> tests/ui/precedence_bits.rs:29:5
+   |
+LL |     0x0F00 & 0xF000 >> 4;
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `0x0F00 & (0xF000 >> 4)`
+
+error: operator precedence might not be obvious
+  --> tests/ui/precedence_bits.rs:30:5
+   |
+LL |     0x0F00 << 1 ^ 3;
+   |     ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) ^ 3`
+
+error: operator precedence might not be obvious
+  --> tests/ui/precedence_bits.rs:31:5
+   |
+LL |     0x0F00 << 1 | 2;
+   |     ^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(0x0F00 << 1) | 2`
+
+error: aborting due to 4 previous errors
+