diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E712.py b/crates/ruff/resources/test/fixtures/pycodestyle/E712.py index 818afddaefe951..c0be4d7aa1c473 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/E712.py +++ b/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 diff --git a/crates/ruff/src/rules/pycodestyle/helpers.rs b/crates/ruff/src/rules/pycodestyle/helpers.rs index eee48f336e9a8b..2f456d273b4319 100644 --- a/crates/ruff/src/rules/pycodestyle/helpers.rs +++ b/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 diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E712_E712.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E712_E712.py.snap index e2a9be7b88be81..ba3f1143bd8873 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E712_E712.py.snap +++ b/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 diff --git a/crates/ruff_python_ast/Cargo.toml b/crates/ruff_python_ast/Cargo.toml index b85f7402232679..ac33034c90eef8 100644 --- a/crates/ruff_python_ast/Cargo.toml +++ b/crates/ruff_python_ast/Cargo.toml @@ -10,8 +10,6 @@ documentation = { workspace = true } repository = { workspace = true } license = { workspace = true } - - [lib] [dependencies] diff --git a/crates/ruff_python_ast/src/lib.rs b/crates/ruff_python_ast/src/lib.rs index 3fb4c5f1700592..2bb603decd5106 100644 --- a/crates/ruff_python_ast/src/lib.rs +++ b/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; diff --git a/crates/ruff_python_ast/src/parenthesize.rs b/crates/ruff_python_ast/src/parenthesize.rs new file mode 100644 index 00000000000000..d60284826ca5ff --- /dev/null +++ b/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, +} + +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 { + // 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 +} diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index c9ab19225ab02e..c8a8a36af8c5ef 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/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 diff --git a/crates/ruff_python_trivia/Cargo.toml b/crates/ruff_python_trivia/Cargo.toml index a9c5d4a44e3c80..f0f9861e5ade3e 100644 --- a/crates/ruff_python_trivia/Cargo.toml +++ b/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" } -