Expand expressions to include parentheses in E712

crates/ruff/resources/test/fixtures/pycodestyle/E712.py

@@ -25,6 +25,12 @@
 if res == True != False:
     pass
 
+if(True) == TrueElement or x == TrueElement:
+    pass
+
+if (yield i) == True:
+      print("even")
+
 #: Okay
 if x not in y:
     pass

crates/ruff/src/rules/pycodestyle/helpers.rs

@@ -1,8 +1,9 @@
-use ruff_python_ast::{CmpOp, Expr, Ranged};
-use ruff_text_size::{TextLen, TextRange};
 use unicode_width::UnicodeWidthStr;
 
+use ruff_python_ast::parenthesize::ParenthesizedExpression;
+use ruff_python_ast::{CmpOp, Expr, Ranged};
 use ruff_source_file::{Line, Locator};
+use ruff_text_size::{TextLen, TextRange};
 
 use crate::line_width::{LineLength, LineWidth, TabSize};
 
@@ -16,12 +17,15 @@ pub(super) fn compare(
     comparators: &[Expr],
     locator: &Locator,
 ) -> String {
+    // If the expression is parenthesized, parenthesize it.
     let start = left.start();
     let end = comparators.last().map_or_else(|| left.end(), Ranged::end);
     let mut contents = String::with_capacity(usize::from(end - start));
 
     // Add the left side of the comparison.
-    contents.push_str(locator.slice(left.range()));
+    contents.push_str(
+        locator.slice(ParenthesizedExpression::from_expr(left.into(), locator.contents()).range()),
+    );
 
     for (op, comparator) in ops.iter().zip(comparators) {
         // Add the operator.
@@ -39,7 +43,9 @@ pub(super) fn compare(
         });
 
         // Add the right side of the comparison.
-        contents.push_str(locator.slice(comparator.range()));
+        contents.push_str(locator.slice(
+            ParenthesizedExpression::from_expr(comparator.into(), locator.contents()).range(),
+        ));
     }
 
     contents

crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E712_E712.py.snap

@@ -181,7 +181,7 @@ E712.py:22:5: E712 [*] Comparison to `True` should be `cond is True` or `if cond
 20 20 | var = 1 if cond == True else -1 if cond == False else cond
 21 21 | #: E712
 22    |-if (True) == TrueElement or x == TrueElement:
-   22 |+if True is TrueElement or x == TrueElement:
+   22 |+if (True) is TrueElement or x == TrueElement:
 23 23 |     pass
 24 24 | 
 25 25 | if res == True != False:
@@ -204,7 +204,7 @@ E712.py:25:11: E712 [*] Comparison to `True` should be `cond is True` or `if con
    25 |+if res is True is not False:
 26 26 |     pass
 27 27 | 
-28 28 | #: Okay
+28 28 | if(True) == TrueElement or x == TrueElement:
 
 E712.py:25:19: E712 [*] Comparison to `False` should be `cond is not False` or `if cond:`
    |
@@ -224,6 +224,46 @@ E712.py:25:19: E712 [*] Comparison to `False` should be `cond is not False` or `
    25 |+if res is True is not False:
 26 26 |     pass
 27 27 | 
-28 28 | #: Okay
+28 28 | if(True) == TrueElement or x == TrueElement:
+
+E712.py:28:4: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
+   |
+26 |     pass
+27 | 
+28 | if(True) == TrueElement or x == TrueElement:
+   |    ^^^^ E712
+29 |     pass
+   |
+   = help: Replace with `cond is True`
+
+ℹ Suggested fix
+25 25 | if res == True != False:
+26 26 |     pass
+27 27 | 
+28    |-if(True) == TrueElement or x == TrueElement:
+   28 |+if(True) is TrueElement or x == TrueElement:
+29 29 |     pass
+30 30 | 
+31 31 | if (yield i) == True:
+
+E712.py:31:17: E712 [*] Comparison to `True` should be `cond is True` or `if cond:`
+   |
+29 |     pass
+30 | 
+31 | if (yield i) == True:
+   |                 ^^^^ E712
+32 |       print("even")
+   |
+   = help: Replace with `cond is True`
+
+ℹ Suggested fix
+28 28 | if(True) == TrueElement or x == TrueElement:
+29 29 |     pass
+30 30 | 
+31    |-if (yield i) == True:
+   31 |+if (yield i) is True:
+32 32 |       print("even")
+33 33 | 
+34 34 | #: Okay
 
 

crates/ruff_python_ast/Cargo.toml

@@ -10,8 +10,6 @@ documentation = { workspace = true }
 repository = { workspace = true }
 license = { workspace = true }
 
-
-
 [lib]
 
 [dependencies]

crates/ruff_python_ast/src/lib.rs

@@ -11,6 +11,7 @@ pub mod identifier;
 pub mod imports;
 pub mod node;
 mod nodes;
+pub mod parenthesize;
 pub mod relocate;
 pub mod statement_visitor;
 pub mod stmt_if;

crates/ruff_python_ast/src/parenthesize.rs

@@ -0,0 +1,54 @@
+use ruff_python_trivia::{first_non_trivia_token, SimpleTokenKind, SimpleTokenizer};
+use ruff_text_size::TextRange;
+
+use crate::node::AnyNodeRef;
+use crate::Ranged;
+
+/// A wrapper around an expression that may be parenthesized.
+#[derive(Debug)]
+pub struct ParenthesizedExpression<'a> {
+    /// The underlying AST node.
+    expr: AnyNodeRef<'a>,
+    /// The range of the expression including parentheses, if the expression is parenthesized;
+    /// or `None`, if the expression is not parenthesized.
+    range: Option<TextRange>,
+}
+
+impl<'a> ParenthesizedExpression<'a> {
+    pub fn from_expr(expr: AnyNodeRef<'a>, contents: &str) -> Self {
+        Self {
+            expr,
+            range: parenthesized_range(expr, contents),
+        }
+    }
+
+    /// Returns `true` if the expression is parenthesized.
+    pub fn is_parenthesized(&self) -> bool {
+        self.range.is_some()
+    }
+}
+
+impl Ranged for ParenthesizedExpression<'_> {
+    fn range(&self) -> TextRange {
+        self.range.unwrap_or_else(|| self.expr.range())
+    }
+}
+
+/// Returns the [`TextRange`] of a given expression including parentheses, if the expression is
+/// parenthesized; or `None`, if the expression is not parenthesized.
+fn parenthesized_range(expr: AnyNodeRef, contents: &str) -> Option<TextRange> {
+    // First, test if there's a closing parenthesis because it tends to be cheaper.
+    let right = first_non_trivia_token(expr.end(), contents)?;
+
+    if right.kind == SimpleTokenKind::RParen {
+        // Next, test for the opening parenthesis.
+        let mut tokenizer =
+            SimpleTokenizer::up_to_without_back_comment(expr.start(), contents).skip_trivia();
+        let left = tokenizer.next_back()?;
+        if left.kind == SimpleTokenKind::LParen {
+            return Some(TextRange::new(left.start(), right.end()));
+        }
+    }
+
+    None
+}

crates/ruff_python_formatter/src/expression/parentheses.rs

@@ -1,8 +1,7 @@
 use ruff_formatter::prelude::tag::Condition;
 use ruff_formatter::{format_args, write, Argument, Arguments};
 use ruff_python_ast::node::AnyNodeRef;
-use ruff_python_ast::Ranged;
-use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
+use ruff_python_ast::parenthesize::ParenthesizedExpression;
 
 use crate::comments::{
     dangling_comments, dangling_open_parenthesis_comments, trailing_comments, SourceComment,
@@ -81,27 +80,7 @@ pub enum Parentheses {
 }
 
 pub(crate) fn is_expression_parenthesized(expr: AnyNodeRef, contents: &str) -> bool {
-    // First test if there's a closing parentheses because it tends to be cheaper.
-    if matches!(
-        first_non_trivia_token(expr.end(), contents),
-        Some(SimpleToken {
-            kind: SimpleTokenKind::RParen,
-            ..
-        })
-    ) {
-        let mut tokenizer =
-            SimpleTokenizer::up_to_without_back_comment(expr.start(), contents).skip_trivia();
-
-        matches!(
-            tokenizer.next_back(),
-            Some(SimpleToken {
-                kind: SimpleTokenKind::LParen,
-                ..
-            })
-        )
-    } else {
-        false
-    }
+    ParenthesizedExpression::from_expr(expr, contents).is_parenthesized()
 }
 
 /// Formats `content` enclosed by the `left` and `right` parentheses. The implementation also ensures

crates/ruff_python_trivia/Cargo.toml

@@ -24,4 +24,3 @@ unic-ucd-ident = "0.9.0"
 insta = { workspace = true }
 ruff_python_ast = { path = "../ruff_python_ast" }
 ruff_python_parser = { path = "../ruff_python_parser" }
-