Skip to content

Commit

Permalink
Avoid removing raw strings in comparison fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 14, 2023
1 parent 73228e9 commit c379704
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 25 deletions.
1 change: 1 addition & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E713.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
1 change: 1 addition & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E714.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
46 changes: 34 additions & 12 deletions crates/ruff/src/rules/pycodestyle/helpers.rs
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ pub(crate) fn literal_comparisons(
.map(|(idx, op)| bad_ops.get(&idx).unwrap_or(op))
.copied()
.collect::<Vec<_>>();
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(),
Expand Down
14 changes: 2 additions & 12 deletions crates/ruff/src/rules/pycodestyle/rules/not_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)));
}
Expand All @@ -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(),
)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


0 comments on commit c379704

Please sign in to comment.