From c3797044e978e9f5be0a91889baf0330bb12bcaa Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 13 Jul 2023 23:06:04 -0400 Subject: [PATCH] Avoid removing raw strings in comparison fixes --- .../test/fixtures/pycodestyle/E713.py | 1 + .../test/fixtures/pycodestyle/E714.py | 1 + crates/ruff/src/rules/pycodestyle/helpers.rs | 46 ++++++++++++++----- .../pycodestyle/rules/literal_comparisons.rs | 2 +- .../src/rules/pycodestyle/rules/not_tests.rs | 14 +----- ...les__pycodestyle__tests__E714_E714.py.snap | 16 +++++++ 6 files changed, 55 insertions(+), 25 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E713.py b/crates/ruff/resources/test/fixtures/pycodestyle/E713.py index 12b249261d28ac..19bd7165f98c22 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/E713.py +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E713.py @@ -36,3 +36,4 @@ assert (not foo) in bar assert {"x": not foo} in bar assert [42, not foo] in bar +assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None) diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E714.py b/crates/ruff/resources/test/fixtures/pycodestyle/E714.py index f18604d59de9b7..044165e39b0e2c 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/E714.py +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E714.py @@ -36,3 +36,4 @@ assert (not foo) in bar assert {"x": not foo} in bar assert [42, not foo] in bar +assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None) diff --git a/crates/ruff/src/rules/pycodestyle/helpers.rs b/crates/ruff/src/rules/pycodestyle/helpers.rs index ab8b04537a0062..fe7085502a44aa 100644 --- a/crates/ruff/src/rules/pycodestyle/helpers.rs +++ b/crates/ruff/src/rules/pycodestyle/helpers.rs @@ -1,29 +1,51 @@ use ruff_text_size::{TextLen, TextRange}; -use rustpython_parser::ast::{self, CmpOp, Expr}; +use rustpython_parser::ast::{CmpOp, Expr, Ranged}; use unicode_width::UnicodeWidthStr; -use ruff_python_ast::source_code::Generator; +use ruff_python_ast::source_code::Locator; use ruff_python_whitespace::Line; use crate::line_width::{LineLength, LineWidth, TabSize}; -pub(crate) fn is_ambiguous_name(name: &str) -> bool { +pub(super) fn is_ambiguous_name(name: &str) -> bool { name == "l" || name == "I" || name == "O" } -pub(crate) fn compare( +pub(super) fn compare( left: &Expr, ops: &[CmpOp], comparators: &[Expr], - generator: Generator, + locator: &Locator, ) -> String { - let node = ast::ExprCompare { - left: Box::new(left.clone()), - ops: ops.to_vec(), - comparators: comparators.to_vec(), - range: TextRange::default(), - }; - generator.expr(&node.into()) + let start = left.start(); + let end = comparators + .last() + .map_or(left.end(), |comparator| comparator.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())); + + for (op, comparator) in ops.iter().zip(comparators) { + // Add the operator. + contents.push_str(match op { + CmpOp::Eq => " == ", + CmpOp::NotEq => " != ", + CmpOp::Lt => " < ", + CmpOp::LtE => " <= ", + CmpOp::Gt => " > ", + CmpOp::GtE => " >= ", + CmpOp::In => " in ", + CmpOp::NotIn => " not in ", + CmpOp::Is => " is ", + CmpOp::IsNot => " is not ", + }); + + // Add the right side of the comparison. + contents.push_str(locator.slice(comparator.range())); + } + + contents } pub(super) fn is_overlong( diff --git a/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs b/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs index 872c515b0ad07c..2990757434dbc7 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs @@ -276,7 +276,7 @@ pub(crate) fn literal_comparisons( .map(|(idx, op)| bad_ops.get(&idx).unwrap_or(op)) .copied() .collect::>(); - let content = compare(left, &ops, comparators, checker.generator()); + let content = compare(left, &ops, comparators, checker.locator); for diagnostic in &mut diagnostics { diagnostic.set_fix(Fix::suggested(Edit::range_replacement( content.to_string(), diff --git a/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs b/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs index 60ef31a5c4d4bb..d747876f067844 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs @@ -100,12 +100,7 @@ pub(crate) fn not_tests( let mut diagnostic = Diagnostic::new(NotInTest, operand.range()); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - compare( - left, - &[CmpOp::NotIn], - comparators, - checker.generator(), - ), + compare(left, &[CmpOp::NotIn], comparators, checker.locator), expr.range(), ))); } @@ -117,12 +112,7 @@ pub(crate) fn not_tests( let mut diagnostic = Diagnostic::new(NotIsTest, operand.range()); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - compare( - left, - &[CmpOp::IsNot], - comparators, - checker.generator(), - ), + compare(left, &[CmpOp::IsNot], comparators, checker.locator), expr.range(), ))); } diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E714_E714.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E714_E714.py.snap index 8286975b5eb684..37f9a2cc633955 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E714_E714.py.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E714_E714.py.snap @@ -39,4 +39,20 @@ E714.py:5:8: E714 [*] Test for object identity should be `is not` 7 7 | 8 8 | #: Okay +E714.py:39:13: E714 [*] Test for object identity should be `is not` + | +37 | assert {"x": not foo} in bar +38 | assert [42, not foo] in bar +39 | assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E714 + | + = help: Convert to `is not` + +ℹ Fix +36 36 | assert (not foo) in bar +37 37 | assert {"x": not foo} in bar +38 38 | assert [42, not foo] in bar +39 |-assert not (re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is None) + 39 |+assert re.search(r"^.:\\Users\\[^\\]*\\Downloads\\.*") is not None +