Skip to content

Commit

Permalink
Update W605 to check in f-strings
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila committed Sep 14, 2023
1 parent cc91933 commit 59bebae
Show file tree
Hide file tree
Showing 5 changed files with 240 additions and 25 deletions.
46 changes: 46 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/W605_2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Same as `W605_0.py` but using f-strings instead.

#: W605:1:10
regex = f'\.png$'

#: W605:2:1
regex = f'''
\.png$
'''

#: W605:2:6
f(
f'\_'
)

#: W605:4:6
f"""
multi-line
literal
with \_ somewhere
in the middle
"""

#: W605:1:38
value = f'new line\nand invalid escape \_ here'


#: Okay
regex = fr'\.png$'
regex = f'\\.png$'
regex = fr'''
\.png$
'''
regex = fr'''
\\.png$
'''
s = f'\\'
regex = f'\w' # noqa
regex = f'''
\w
''' # noqa

regex = f'\\\_'
value = f'\{{1}}'
# TODO(dhruvmanila): This is currently not detected.
value = f'\{1}'
16 changes: 8 additions & 8 deletions crates/ruff/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ pub(crate) fn check_tokens(

if settings.rules.enabled(Rule::InvalidEscapeSequence) {
for (tok, range) in tokens.iter().flatten() {
if tok.is_string() {
pycodestyle::rules::invalid_escape_sequence(
&mut diagnostics,
locator,
*range,
settings.rules.should_fix(Rule::InvalidEscapeSequence),
);
}
pycodestyle::rules::invalid_escape_sequence(
&mut diagnostics,
locator,
indexer,
tok,
*range,
settings.rules.should_fix(Rule::InvalidEscapeSequence),
);
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mod tests {
#[test_case(Rule::BlankLineWithWhitespace, Path::new("W29.py"))]
#[test_case(Rule::InvalidEscapeSequence, Path::new("W605_0.py"))]
#[test_case(Rule::InvalidEscapeSequence, Path::new("W605_1.py"))]
#[test_case(Rule::InvalidEscapeSequence, Path::new("W605_2.py"))]
#[test_case(Rule::LineTooLong, Path::new("E501.py"))]
#[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
Expand Down
59 changes: 42 additions & 17 deletions crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use memchr::memchr_iter;

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{leading_quote, trailing_quote};
use ruff_python_index::Indexer;
use ruff_python_parser::Tok;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

Expand Down Expand Up @@ -45,23 +46,40 @@ impl AlwaysAutofixableViolation for InvalidEscapeSequence {
pub(crate) fn invalid_escape_sequence(
diagnostics: &mut Vec<Diagnostic>,
locator: &Locator,
range: TextRange,
indexer: &Indexer,
tok: &Tok,
tok_range: TextRange,
autofix: bool,
) {
let text = locator.slice(range);
let (start_offset, body) = match tok {
Tok::FStringMiddle { value, is_raw } => {
if *is_raw {
return;
}
(tok_range.start(), value.as_str())
}
Tok::String {
value,
kind,
triple_quoted,
} => {
if kind.is_raw() {
return;
}

// Determine whether the string is single- or triple-quoted.
let Some(leading_quote) = leading_quote(text) else {
return;
};
let Some(trailing_quote) = trailing_quote(text) else {
return;
};
let body = &text[leading_quote.len()..text.len() - trailing_quote.len()];
let quote_len = if *triple_quoted {
TextSize::new(3)
} else {
TextSize::new(1)
};

if leading_quote.contains(['r', 'R']) {
return;
}
(
tok_range.start() + kind.prefix_len() + quote_len,
value.as_str(),
)
}
_ => return,
};

let mut contains_valid_escape_sequence = false;
let mut invalid_escape_sequence = Vec::new();
Expand Down Expand Up @@ -120,7 +138,7 @@ pub(crate) fn invalid_escape_sequence(
continue;
}

let location = range.start() + leading_quote.text_len() + TextSize::try_from(i).unwrap();
let location = start_offset + TextSize::try_from(i).unwrap();
let range = TextRange::at(location, next_char.text_len() + TextSize::from(1));
invalid_escape_sequence.push(Diagnostic::new(InvalidEscapeSequence(next_char), range));
}
Expand All @@ -135,14 +153,21 @@ pub(crate) fn invalid_escape_sequence(
)));
}
} else {
let tok_start = if tok.is_f_string_middle() {
// SAFETY: If this is a `FStringMiddle` token, then the indexer
// must have the f-string range.
indexer.f_string_range(tok_range.start()).unwrap().start()
} else {
tok_range.start()
};
// Turn into raw string.
for diagnostic in &mut invalid_escape_sequence {
// If necessary, add a space between any leading keyword (`return`, `yield`,
// `assert`, etc.) and the string. For example, `return"foo"` is valid, but
// `returnr"foo"` is not.
diagnostic.set_fix(Fix::automatic(Edit::insertion(
pad_start("r".to_string(), range.start(), locator),
range.start(),
pad_start("r".to_string(), tok_start, locator),
tok_start,
)));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
---
source: crates/ruff/src/rules/pycodestyle/mod.rs
---
W605_2.py:4:11: W605 [*] Invalid escape sequence: `\.`
|
3 | #: W605:1:10
4 | regex = f'\.png$'
| ^^ W605
5 |
6 | #: W605:2:1
|
= help: Add backslash to escape sequence

Fix
1 1 | # Same as `W605_0.py` but using f-strings instead.
2 2 |
3 3 | #: W605:1:10
4 |-regex = f'\.png$'
4 |+regex = rf'\.png$'
5 5 |
6 6 | #: W605:2:1
7 7 | regex = f'''
W605_2.py:8:1: W605 [*] Invalid escape sequence: `\.`
|
6 | #: W605:2:1
7 | regex = f'''
8 | \.png$
| ^^ W605
9 | '''
|
= help: Add backslash to escape sequence

Fix
4 4 | regex = f'\.png$'
5 5 |
6 6 | #: W605:2:1
7 |-regex = f'''
7 |+regex = rf'''
8 8 | \.png$
9 9 | '''
10 10 |

W605_2.py:13:7: W605 [*] Invalid escape sequence: `\_`
|
11 | #: W605:2:6
12 | f(
13 | f'\_'
| ^^ W605
14 | )
|
= help: Add backslash to escape sequence

Fix
10 10 |
11 11 | #: W605:2:6
12 12 | f(
13 |- f'\_'
13 |+ rf'\_'
14 14 | )
15 15 |
16 16 | #: W605:4:6

W605_2.py:20:6: W605 [*] Invalid escape sequence: `\_`
|
18 | multi-line
19 | literal
20 | with \_ somewhere
| ^^ W605
21 | in the middle
22 | """
|
= help: Add backslash to escape sequence

Fix
14 14 | )
15 15 |
16 16 | #: W605:4:6
17 |-f"""
17 |+rf"""
18 18 | multi-line
19 19 | literal
20 20 | with \_ somewhere

W605_2.py:25:40: W605 [*] Invalid escape sequence: `\_`
|
24 | #: W605:1:38
25 | value = f'new line\nand invalid escape \_ here'
| ^^ W605
|
= help: Add backslash to escape sequence

Fix
22 22 | """
23 23 |
24 24 | #: W605:1:38
25 |-value = f'new line\nand invalid escape \_ here'
25 |+value = f'new line\nand invalid escape \\_ here'
26 26 |
27 27 |
28 28 | #: Okay

W605_2.py:43:13: W605 [*] Invalid escape sequence: `\_`
|
41 | ''' # noqa
42 |
43 | regex = f'\\\_'
| ^^ W605
44 | value = f'\{{1}}'
45 | # TODO(dhruvmanila): This is currently not detected.
|
= help: Add backslash to escape sequence

Fix
40 40 | \w
41 41 | ''' # noqa
42 42 |
43 |-regex = f'\\\_'
43 |+regex = f'\\\\_'
44 44 | value = f'\{{1}}'
45 45 | # TODO(dhruvmanila): This is currently not detected.
46 46 | value = f'\{1}'

W605_2.py:44:11: W605 [*] Invalid escape sequence: `\{`
|
43 | regex = f'\\\_'
44 | value = f'\{{1}}'
| ^^ W605
45 | # TODO(dhruvmanila): This is currently not detected.
46 | value = f'\{1}'
|
= help: Add backslash to escape sequence

Fix
41 41 | ''' # noqa
42 42 |
43 43 | regex = f'\\\_'
44 |-value = f'\{{1}}'
44 |+value = rf'\{{1}}'
45 45 | # TODO(dhruvmanila): This is currently not detected.
46 46 | value = f'\{1}'


0 comments on commit 59bebae

Please sign in to comment.