Skip to content

Commit

Permalink
Expand expressions to include parentheses in E712
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 14, 2023
1 parent ebda5fc commit 9d0a620
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 5 deletions.
6 changes: 6 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E712.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 24 additions & 2 deletions crates/ruff/src/rules/pycodestyle/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_python_ast::{CmpOp, Expr, Ranged};
use ruff_python_trivia::{first_non_trivia_token, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{TextLen, TextRange};
use unicode_width::UnicodeWidthStr;

Expand All @@ -16,12 +17,13 @@ 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(parenthesized_range(left, locator)));

for (op, comparator) in ops.iter().zip(comparators) {
// Add the operator.
Expand All @@ -39,12 +41,32 @@ pub(super) fn compare(
});

// Add the right side of the comparison.
contents.push_str(locator.slice(comparator.range()));
contents.push_str(locator.slice(parenthesized_range(comparator, locator)));
}

contents
}

/// Returns the [`TextRange`] of a given expression, including parentheses if the expression is
/// parenthesized.
fn parenthesized_range(expr: &Expr, locator: &Locator) -> TextRange {
// First, test if there's a closing parenthesis because it tends to be cheaper.
if let Some(right) = first_non_trivia_token(expr.end(), locator.contents()) {
if right.kind == SimpleTokenKind::RParen {
// Next, test for the opening parenthesis.
let mut tokenizer =
SimpleTokenizer::up_to_without_back_comment(expr.start(), locator.contents())
.skip_trivia();
if let Some(left) = tokenizer.next_back() {
if left.kind == SimpleTokenKind::LParen {
return TextRange::new(left.start(), right.end());
}
}
}
}
expr.range()
}

pub(super) fn is_overlong(
line: &Line,
limit: LineLength,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:`
|
Expand All @@ -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


0 comments on commit 9d0a620

Please sign in to comment.