Skip to content

Commit

Permalink
Correct quick fix message for W605 (#8255)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes the `W605` rule implementation to provide the quickfix
message as
per the fix provided.

## Test Plan

Update snapshots.

fixes: #8155
  • Loading branch information
dhruvmanila authored Oct 26, 2023
1 parent a4dd1e5 commit 4ffd4ed
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_index::Indexer;
use ruff_python_parser::Tok;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use ruff_text_size::{TextLen, TextRange, TextSize};

use crate::fix::edits::pad_start;

Expand All @@ -25,23 +25,51 @@ use crate::fix::edits::pad_start;
/// regex = r"\.png$"
/// ```
///
/// Or, if the string already contains a valid escape sequence:
/// ```python
/// value = "new line\nand invalid escape \_ here"
/// ```
///
/// Use instead:
/// ```python
/// value = "new line\nand invalid escape \\_ here"
/// ```
///
/// ## References
/// - [Python documentation: String and Bytes literals](https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals)
#[violation]
pub struct InvalidEscapeSequence(char);
pub struct InvalidEscapeSequence {
ch: char,
fix_title: FixTitle,
}

impl AlwaysFixableViolation for InvalidEscapeSequence {
#[derive_message_formats]
fn message(&self) -> String {
let InvalidEscapeSequence(char) = self;
format!("Invalid escape sequence: `\\{char}`")
let InvalidEscapeSequence { ch, .. } = self;
format!("Invalid escape sequence: `\\{ch}`")
}

fn fix_title(&self) -> String {
"Add backslash to escape sequence".to_string()
match self.fix_title {
FixTitle::AddBackslash => format!("Add backslash to escape sequence"),
FixTitle::UseRawStringLiteral => format!("Use a raw string literal"),
}
}
}

#[derive(Debug, PartialEq, Eq)]
enum FixTitle {
AddBackslash,
UseRawStringLiteral,
}

#[derive(Debug)]
struct InvalidEscapeChar {
ch: char,
range: TextRange,
}

/// W605
pub(crate) fn invalid_escape_sequence(
diagnostics: &mut Vec<Diagnostic>,
Expand All @@ -67,7 +95,7 @@ pub(crate) fn invalid_escape_sequence(
};

let mut contains_valid_escape_sequence = false;
let mut invalid_escape_sequence = Vec::new();
let mut invalid_escape_chars = Vec::new();

let mut prev = None;
let bytes = token_source_code.as_bytes();
Expand Down Expand Up @@ -154,16 +182,28 @@ pub(crate) fn invalid_escape_sequence(

let location = token_range.start() + 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));
invalid_escape_chars.push(InvalidEscapeChar {
ch: next_char,
range,
});
}

let mut invalid_escape_sequence = Vec::new();
if contains_valid_escape_sequence {
// Escape with backslash.
for diagnostic in &mut invalid_escape_sequence {
diagnostic.set_fix(Fix::safe_edit(Edit::insertion(
for invalid_escape_char in &invalid_escape_chars {
let diagnostic = Diagnostic::new(
InvalidEscapeSequence {
ch: invalid_escape_char.ch,
fix_title: FixTitle::AddBackslash,
},
invalid_escape_char.range,
)
.with_fix(Fix::safe_edit(Edit::insertion(
r"\".to_string(),
diagnostic.start() + TextSize::from(1),
invalid_escape_char.range.start() + TextSize::from(1),
)));
invalid_escape_sequence.push(diagnostic);
}
} else {
let tok_start = if token.is_f_string_middle() {
Expand All @@ -178,14 +218,24 @@ pub(crate) fn invalid_escape_sequence(
token_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::safe_edit(Edit::insertion(
pad_start("r".to_string(), tok_start, locator),
tok_start,
)));
for invalid_escape_char in &invalid_escape_chars {
let diagnostic = Diagnostic::new(
InvalidEscapeSequence {
ch: invalid_escape_char.ch,
fix_title: FixTitle::UseRawStringLiteral,
},
invalid_escape_char.range,
)
.with_fix(
// 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.
Fix::safe_edit(Edit::insertion(
pad_start("r".to_string(), tok_start, locator),
tok_start,
)),
);
invalid_escape_sequence.push(diagnostic);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ W605_0.py:2:10: W605 [*] Invalid escape sequence: `\.`
3 |
4 | #: W605:2:1
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
1 1 | #: W605:1:10
Expand All @@ -27,7 +27,7 @@ W605_0.py:6:1: W605 [*] Invalid escape sequence: `\.`
| ^^ W605
7 | '''
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
2 2 | regex = '\.png$'
Expand All @@ -47,7 +47,7 @@ W605_0.py:11:6: W605 [*] Invalid escape sequence: `\_`
| ^^ W605
12 | )
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
8 8 |
Expand All @@ -68,7 +68,7 @@ W605_0.py:18:6: W605 [*] Invalid escape sequence: `\_`
19 | in the middle
20 | """
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
12 12 | )
Expand Down Expand Up @@ -107,7 +107,7 @@ W605_0.py:28:12: W605 [*] Invalid escape sequence: `\.`
29 |
30 | #: Okay
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
25 25 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ W605_1.py:2:10: W605 [*] Invalid escape sequence: `\.`
3 |
4 | #: W605:2:1
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
1 1 | #: W605:1:10
Expand All @@ -27,7 +27,7 @@ W605_1.py:6:1: W605 [*] Invalid escape sequence: `\.`
| ^^ W605
7 | '''
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
2 2 | regex = '\.png$'
Expand All @@ -47,7 +47,7 @@ W605_1.py:11:6: W605 [*] Invalid escape sequence: `\_`
| ^^ W605
12 | )
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
8 8 |
Expand All @@ -68,7 +68,7 @@ W605_1.py:18:6: W605 [*] Invalid escape sequence: `\_`
19 | in the middle
20 | """
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
12 12 | )
Expand All @@ -89,7 +89,7 @@ W605_1.py:25:12: W605 [*] Invalid escape sequence: `\.`
26 |
27 | #: Okay
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
22 22 |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: crates/ruff/src/rules/pycodestyle/mod.rs
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
W605_2.py:4:11: W605 [*] Invalid escape sequence: `\.`
|
Expand All @@ -9,7 +9,7 @@ W605_2.py:4:11: W605 [*] Invalid escape sequence: `\.`
5 |
6 | #: W605:2:1
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
1 1 | # Same as `W605_0.py` but using f-strings instead.
Expand All @@ -29,7 +29,7 @@ W605_2.py:8:1: W605 [*] Invalid escape sequence: `\.`
| ^^ W605
9 | '''
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
4 4 | regex = f'\.png$'
Expand All @@ -49,7 +49,7 @@ W605_2.py:13:7: W605 [*] Invalid escape sequence: `\_`
| ^^ W605
14 | )
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
10 10 |
Expand All @@ -70,7 +70,7 @@ W605_2.py:20:6: W605 [*] Invalid escape sequence: `\_`
21 | in the middle
22 | """
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
14 14 | )
Expand Down Expand Up @@ -129,7 +129,7 @@ W605_2.py:44:11: W605 [*] Invalid escape sequence: `\{`
45 | value = f'\{1}'
46 | value = f'{1:\}'
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
41 41 | ''' # noqa
Expand All @@ -150,7 +150,7 @@ W605_2.py:45:11: W605 [*] Invalid escape sequence: `\{`
46 | value = f'{1:\}'
47 | value = f"{f"\{1}"}"
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
42 42 |
Expand All @@ -171,7 +171,7 @@ W605_2.py:46:14: W605 [*] Invalid escape sequence: `\}`
47 | value = f"{f"\{1}"}"
48 | value = rf"{f"\{1}"}"
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
43 43 | regex = f'\\\_'
Expand All @@ -191,7 +191,7 @@ W605_2.py:47:14: W605 [*] Invalid escape sequence: `\{`
| ^^ W605
48 | value = rf"{f"\{1}"}"
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
44 44 | value = f'\{{1}}'
Expand All @@ -212,7 +212,7 @@ W605_2.py:48:15: W605 [*] Invalid escape sequence: `\{`
49 |
50 | # Okay
|
= help: Add backslash to escape sequence
= help: Use a raw string literal

Fix
45 45 | value = f'\{1}'
Expand Down

0 comments on commit 4ffd4ed

Please sign in to comment.