Skip to content

Commit cffd186

Browse files
authored
Preserve raw string prefix and escapes (#15694)
## Summary Fixes #9663 and also improves the fixes for [RUF055](https://docs.astral.sh/ruff/rules/unnecessary-regular-expression/) since regular expressions are often written as raw strings. This doesn't include raw f-strings. ## Test Plan Existing snapshots for RUF055 and PT009, plus a new `Generator` test and a regression test for the reported `PIE810` issue.
1 parent 569060f commit cffd186

File tree

7 files changed

+65
-18
lines changed

7 files changed

+65
-18
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE810.py

+6
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,9 @@ def func():
7676

7777
if msg.startswith(y) or msg.endswith(x) or msg.startswith("h"): # OK
7878
print("yes")
79+
80+
81+
def func():
82+
"Regression test for https://github.com/astral-sh/ruff/issues/9663"
83+
if x.startswith("a") or x.startswith("b") or re.match(r"a\.b", x):
84+
print("yes")

crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE810_PIE810.py.snap

+18
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,21 @@ PIE810.py:25:8: PIE810 [*] Call `startswith` once with a `tuple`
142142
26 26 | print("yes")
143143
27 27 |
144144
28 28 | # ok
145+
146+
PIE810.py:83:8: PIE810 [*] Call `startswith` once with a `tuple`
147+
|
148+
81 | def func():
149+
82 | "Regression test for https://github.com/astral-sh/ruff/issues/9663"
150+
83 | if x.startswith("a") or x.startswith("b") or re.match(r"a\.b", x):
151+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PIE810
152+
84 | print("yes")
153+
|
154+
= help: Merge into a single `startswith` call
155+
156+
Unsafe fix
157+
80 80 |
158+
81 81 | def func():
159+
82 82 | "Regression test for https://github.com/astral-sh/ruff/issues/9663"
160+
83 |- if x.startswith("a") or x.startswith("b") or re.match(r"a\.b", x):
161+
83 |+ if x.startswith(("a", "b")) or re.match(r"a\.b", x):
162+
84 84 | print("yes")

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT009.snap

+4-4
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ PT009.py:73:9: PT009 [*] Use a regular `assert` instead of unittest-style `asser
498498
71 71 |
499499
72 72 | def test_assert_regex(self):
500500
73 |- self.assertRegex("abc", r"def") # Error
501-
73 |+ assert re.search("def", "abc") # Error
501+
73 |+ assert re.search(r"def", "abc") # Error
502502
74 74 |
503503
75 75 | def test_assert_not_regex(self):
504504
76 76 | self.assertNotRegex("abc", r"abc") # Error
@@ -518,7 +518,7 @@ PT009.py:76:9: PT009 [*] Use a regular `assert` instead of unittest-style `asser
518518
74 74 |
519519
75 75 | def test_assert_not_regex(self):
520520
76 |- self.assertNotRegex("abc", r"abc") # Error
521-
76 |+ assert not re.search("abc", "abc") # Error
521+
76 |+ assert not re.search(r"abc", "abc") # Error
522522
77 77 |
523523
78 78 | def test_assert_regexp_matches(self):
524524
79 79 | self.assertRegexpMatches("abc", r"def") # Error
@@ -538,7 +538,7 @@ PT009.py:79:9: PT009 [*] Use a regular `assert` instead of unittest-style `asser
538538
77 77 |
539539
78 78 | def test_assert_regexp_matches(self):
540540
79 |- self.assertRegexpMatches("abc", r"def") # Error
541-
79 |+ assert re.search("def", "abc") # Error
541+
79 |+ assert re.search(r"def", "abc") # Error
542542
80 80 |
543543
81 81 | def test_assert_not_regexp_matches(self):
544544
82 82 | self.assertNotRegex("abc", r"abc") # Error
@@ -558,7 +558,7 @@ PT009.py:82:9: PT009 [*] Use a regular `assert` instead of unittest-style `asser
558558
80 80 |
559559
81 81 | def test_assert_not_regexp_matches(self):
560560
82 |- self.assertNotRegex("abc", r"abc") # Error
561-
82 |+ assert not re.search("abc", "abc") # Error
561+
82 |+ assert not re.search(r"abc", "abc") # Error
562562
83 83 |
563563
84 84 | def test_fail_if(self):
564564
85 85 | self.failIf("abc") # Error

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF055_RUF055_0.py.snap

+8-8
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,14 @@ RUF055_0.py:89:1: RUF055 [*] Plain string pattern passed to `re` function
142142
90 | re.sub(r"a", r"\n", "a")
143143
91 | re.sub(r"a", "\a", "a")
144144
|
145-
= help: Replace with `"a".replace("a", "\n")`
145+
= help: Replace with `"a".replace(r"a", "\n")`
146146

147147
ℹ Safe fix
148148
86 86 | # `re.sub(r"a", r"\n", some_string)` is fixed to `some_string.replace("a", "\n")`
149149
87 87 | # *not* `some_string.replace("a", "\\n")`.
150150
88 88 | # We currently emit diagnostics for some of these without fixing them.
151151
89 |-re.sub(r"a", "\n", "a")
152-
89 |+"a".replace("a", "\n")
152+
89 |+"a".replace(r"a", "\n")
153153
90 90 | re.sub(r"a", r"\n", "a")
154154
91 91 | re.sub(r"a", "\a", "a")
155155
92 92 | re.sub(r"a", r"\a", "a")
@@ -172,14 +172,14 @@ RUF055_0.py:91:1: RUF055 [*] Plain string pattern passed to `re` function
172172
| ^^^^^^^^^^^^^^^^^^^^^^^ RUF055
173173
92 | re.sub(r"a", r"\a", "a")
174174
|
175-
= help: Replace with `"a".replace("a", "\x07")`
175+
= help: Replace with `"a".replace(r"a", "\x07")`
176176

177177
ℹ Safe fix
178178
88 88 | # We currently emit diagnostics for some of these without fixing them.
179179
89 89 | re.sub(r"a", "\n", "a")
180180
90 90 | re.sub(r"a", r"\n", "a")
181181
91 |-re.sub(r"a", "\a", "a")
182-
91 |+"a".replace("a", "\x07")
182+
91 |+"a".replace(r"a", "\x07")
183183
92 92 | re.sub(r"a", r"\a", "a")
184184
93 93 |
185185
94 94 | re.sub(r"a", "\?", "a")
@@ -202,14 +202,14 @@ RUF055_0.py:94:1: RUF055 [*] Plain string pattern passed to `re` function
202202
| ^^^^^^^^^^^^^^^^^^^^^^^ RUF055
203203
95 | re.sub(r"a", r"\?", "a")
204204
|
205-
= help: Replace with `"a".replace("a", "\\?")`
205+
= help: Replace with `"a".replace(r"a", "\\?")`
206206

207207
ℹ Safe fix
208208
91 91 | re.sub(r"a", "\a", "a")
209209
92 92 | re.sub(r"a", r"\a", "a")
210210
93 93 |
211211
94 |-re.sub(r"a", "\?", "a")
212-
94 |+"a".replace("a", "\\?")
212+
94 |+"a".replace(r"a", "\\?")
213213
95 95 | re.sub(r"a", r"\?", "a")
214214

215215
RUF055_0.py:95:1: RUF055 [*] Plain string pattern passed to `re` function
@@ -218,11 +218,11 @@ RUF055_0.py:95:1: RUF055 [*] Plain string pattern passed to `re` function
218218
95 | re.sub(r"a", r"\?", "a")
219219
| ^^^^^^^^^^^^^^^^^^^^^^^^ RUF055
220220
|
221-
= help: Replace with `"a".replace("a", "\\?")`
221+
= help: Replace with `"a".replace(r"a", r"\?")`
222222

223223
ℹ Safe fix
224224
92 92 | re.sub(r"a", r"\a", "a")
225225
93 93 |
226226
94 94 | re.sub(r"a", "\?", "a")
227227
95 |-re.sub(r"a", r"\?", "a")
228-
95 |+"a".replace("a", "\\?")
228+
95 |+"a".replace(r"a", r"\?")

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF055_RUF055_1.py.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ RUF055_1.py:17:1: RUF055 [*] Plain string pattern passed to `re` function
2929
17 | re.sub(r"abc", repl, haystack)
3030
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF055
3131
|
32-
= help: Replace with `haystack.replace("abc", repl)`
32+
= help: Replace with `haystack.replace(r"abc", repl)`
3333

3434
ℹ Safe fix
3535
14 14 |
3636
15 15 | # also works for the `repl` argument in sub
3737
16 16 | repl = "new"
3838
17 |-re.sub(r"abc", repl, haystack)
39-
17 |+haystack.replace("abc", repl)
39+
17 |+haystack.replace(r"abc", repl)

crates/ruff_python_ast/src/str.rs

+8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ impl Quote {
2424
}
2525
}
2626

27+
#[inline]
28+
pub const fn as_str(self) -> &'static str {
29+
match self {
30+
Self::Single => "'",
31+
Self::Double => "\"",
32+
}
33+
}
34+
2735
#[must_use]
2836
#[inline]
2937
pub const fn opposite(self) -> Self {

crates/ruff_python_codegen/src/generator.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -1284,10 +1284,19 @@ impl<'a> Generator<'a> {
12841284

12851285
fn unparse_string_literal(&mut self, string_literal: &ast::StringLiteral) {
12861286
let ast::StringLiteral { value, flags, .. } = string_literal;
1287-
if flags.prefix().is_unicode() {
1288-
self.p("u");
1287+
// for raw strings, we don't want to perform the UnicodeEscape in `p_str_repr`, so build the
1288+
// replacement here
1289+
if flags.prefix().is_raw() {
1290+
self.p(flags.prefix().as_str());
1291+
self.p(self.quote.as_str());
1292+
self.p(value);
1293+
self.p(self.quote.as_str());
1294+
} else {
1295+
if flags.prefix().is_unicode() {
1296+
self.p("u");
1297+
}
1298+
self.p_str_repr(value);
12891299
}
1290-
self.p_str_repr(value);
12911300
}
12921301

12931302
fn unparse_string_literal_value(&mut self, value: &ast::StringLiteralValue) {
@@ -1712,14 +1721,20 @@ class Foo:
17121721
assert_eq!(round_trip(r#""hello""#), r#""hello""#);
17131722
assert_eq!(round_trip(r"'hello'"), r#""hello""#);
17141723
assert_eq!(round_trip(r"u'hello'"), r#"u"hello""#);
1715-
assert_eq!(round_trip(r"r'hello'"), r#""hello""#);
1724+
assert_eq!(round_trip(r"r'hello'"), r#"r"hello""#);
17161725
assert_eq!(round_trip(r"b'hello'"), r#"b"hello""#);
17171726
assert_eq!(round_trip(r#"("abc" "def" "ghi")"#), r#""abc" "def" "ghi""#);
17181727
assert_eq!(round_trip(r#""he\"llo""#), r#"'he"llo'"#);
17191728
assert_eq!(round_trip(r#"f"abc{'def'}{1}""#), r#"f"abc{'def'}{1}""#);
17201729
assert_eq!(round_trip(r#"f'abc{"def"}{1}'"#), r#"f"abc{'def'}{1}""#);
17211730
}
17221731

1732+
#[test]
1733+
fn raw() {
1734+
assert_round_trip!(r#"r"a\.b""#); // https://github.com/astral-sh/ruff/issues/9663
1735+
assert_round_trip!(r#"R"a\.b""#);
1736+
}
1737+
17231738
#[test]
17241739
fn self_documenting_fstring() {
17251740
assert_round_trip!(r#"f"{ chr(65) = }""#);

0 commit comments

Comments
 (0)