From 16e1e565d86fe7e83f15ab47741e534a3a671932 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 13 Sep 2023 11:05:45 +0530 Subject: [PATCH] Update `W605` to check in f-strings --- .../test/fixtures/pycodestyle/W605_2.py | 43 +++++++ crates/ruff/src/checkers/tokens.rs | 16 +-- crates/ruff/src/rules/pycodestyle/mod.rs | 1 + .../rules/invalid_escape_sequence.rs | 59 ++++++--- ...s__pycodestyle__tests__W605_W605_2.py.snap | 119 ++++++++++++++++++ 5 files changed, 213 insertions(+), 25 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pycodestyle/W605_2.py create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W605_W605_2.py.snap diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/W605_2.py b/crates/ruff/resources/test/fixtures/pycodestyle/W605_2.py new file mode 100644 index 00000000000000..16ec5ccd419584 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pycodestyle/W605_2.py @@ -0,0 +1,43 @@ +# 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'\\\_' diff --git a/crates/ruff/src/checkers/tokens.rs b/crates/ruff/src/checkers/tokens.rs index 4c7ecee760bcf1..12bda59ac6a6a4 100644 --- a/crates/ruff/src/checkers/tokens.rs +++ b/crates/ruff/src/checkers/tokens.rs @@ -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), + ); } } diff --git a/crates/ruff/src/rules/pycodestyle/mod.rs b/crates/ruff/src/rules/pycodestyle/mod.rs index c6387432637e84..18b44f63aea024 100644 --- a/crates/ruff/src/rules/pycodestyle/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/mod.rs @@ -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"))] diff --git a/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs b/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs index 8cf0466c5cdb6f..062e3151239d6d 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/invalid_escape_sequence.rs @@ -3,6 +3,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}; @@ -45,23 +47,39 @@ impl AlwaysAutofixableViolation for InvalidEscapeSequence { pub(crate) fn invalid_escape_sequence( diagnostics: &mut Vec, locator: &Locator, - range: TextRange, + indexer: &Indexer, + tok: &Tok, + tok_range: TextRange, autofix: bool, ) { - let text = locator.slice(range); - - // 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 (start_offset, body) = match tok { + Tok::FStringMiddle { value, is_raw } => { + if *is_raw { + return; + } + (tok_range.start(), value.as_str()) + } + Tok::String { .. } => { + let text = locator.slice(tok_range); + + // Determine whether the string is single- or triple-quoted. + let Some(leading_quote) = leading_quote(text) else { + return; + }; + if leading_quote.contains(['r', 'R']) { + return; + } + let Some(trailing_quote) = trailing_quote(text) else { + return; + }; + + ( + tok_range.start() + leading_quote.text_len(), + &text[leading_quote.len()..text.len() - trailing_quote.len()], + ) + } + _ => return, }; - let body = &text[leading_quote.len()..text.len() - trailing_quote.len()]; - - if leading_quote.contains(['r', 'R']) { - return; - } let mut contains_valid_escape_sequence = false; let mut invalid_escape_sequence = Vec::new(); @@ -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)); } @@ -135,14 +153,21 @@ pub(crate) fn invalid_escape_sequence( ))); } } else { + let tok_start = match tok { + Tok::String { .. } => tok_range.start(), + Tok::FStringMiddle { .. } => { + indexer.f_string_range(tok_range.start()).unwrap().start() + } + _ => unreachable!("expected `Tok::String` or `Tok::FStringMiddle`"), + }; // 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, ))); } } diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W605_W605_2.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W605_W605_2.py.snap new file mode 100644 index 00000000000000..3910030852749b --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__W605_W605_2.py.snap @@ -0,0 +1,119 @@ +--- +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 + | + = help: Add backslash to escape sequence + +ℹ Fix +40 40 | \w +41 41 | ''' # noqa +42 42 | +43 |-regex = f'\\\_' + 43 |+regex = f'\\\\_' + +