mirror of https://github.com/astral-sh/ruff
Improve handling of mixed raw/non-raw string joins in Flynt rule
Refines the static_join_to_fstring Flynt rule to better handle cases where raw and non-raw strings are joined, avoiding unsafe raw string representations when content could break syntax or change behavior. Updates test fixtures and snapshots to reflect improved diagnostics and suggestions for problematic joins.
This commit is contained in:
Dan
parent
7cd6e036e9
commit
d371dde2bf
|
|
@ -36,7 +36,7 @@ nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
|||
nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
nok18 = "".join((r"", "\\r")) # First is raw, second has carriage return escape - would change behavior
|
||||
nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
|
||||
# Test that all-raw strings still work (should be OK)
|
||||
ok7 = "".join((r"", r"something")) # Both are raw - OK
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@ use ast::FStringFlags;
|
|||
use itertools::Itertools;
|
||||
|
||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags, str::Quote};
|
||||
use ruff_python_ast::{
|
||||
self as ast, Arguments, Expr, StringFlags, str::Quote, str_prefix::StringLiteralPrefix,
|
||||
};
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -74,7 +76,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
|
|||
// If all elements are string constants, join them into a single string.
|
||||
if joinees.iter().all(Expr::is_string_literal_expr) {
|
||||
let mut flags: Option<ast::StringLiteralFlags> = None;
|
||||
let mut first_raw: Option<bool> = None;
|
||||
let mut any_raw = false;
|
||||
|
||||
for expr in joinees {
|
||||
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
|
||||
|
|
@ -83,12 +85,9 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
|
|||
|
||||
if flags.is_none() {
|
||||
flags = Some(curr_flags);
|
||||
first_raw = Some(is_raw);
|
||||
} else if first_raw != Some(is_raw) {
|
||||
// If not all strings have the same raw/non-raw status, bail.
|
||||
// Mixing raw and non-raw strings can cause syntax errors or
|
||||
// behavior changes when creating the joined string.
|
||||
return None;
|
||||
any_raw = is_raw;
|
||||
} else if is_raw {
|
||||
any_raw = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -106,6 +105,21 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
|
|||
|
||||
let mut flags = flags?;
|
||||
|
||||
// If any input was raw but the content cannot be safely represented as a raw string,
|
||||
// use non-raw representation. This handles cases where raw strings would create invalid
|
||||
// syntax or behavior changes.
|
||||
if any_raw && !content.is_empty() {
|
||||
let needs_non_raw = content.contains(['\n', '\r', '\0']) || {
|
||||
// Check if content contains characters that would break raw string syntax
|
||||
let quote_char = flags.quote_str();
|
||||
content.contains(quote_char) || content.contains('\\')
|
||||
};
|
||||
|
||||
if needs_non_raw {
|
||||
flags = flags.with_prefix(StringLiteralPrefix::Empty);
|
||||
}
|
||||
}
|
||||
|
||||
// If the result is a raw string and contains a newline, use triple quotes.
|
||||
if flags.prefix().is_raw() && content.contains(['\n', '\r']) {
|
||||
flags = flags.with_triple_quotes(ruff_python_ast::str::TripleQuotes::Yes);
|
||||
|
|
|
|||
|
|
@ -125,6 +125,47 @@ help: Replace with `f"{secrets.token_urlsafe()}a{secrets.token_hex()}"`
|
|||
13 | nok2 = a.join(["1", "2", "3"]) # Not OK (not a static joiner)
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `'line1\nline2'` instead of string join
|
||||
--> FLY002.py:20:8
|
||||
|
|
||||
18 | nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string)
|
||||
19 | # https://github.com/astral-sh/ruff/issues/19887
|
||||
20 | nok8 = '\n'.join([r'line1','line2'])
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
21 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)
|
||||
|
|
||||
help: Replace with `'line1\nline2'`
|
||||
17 | nok6 = "a".join(x for x in "feefoofum") # Not OK (generator)
|
||||
18 | nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string)
|
||||
19 | # https://github.com/astral-sh/ruff/issues/19887
|
||||
- nok8 = '\n'.join([r'line1','line2'])
|
||||
20 + nok8 = 'line1\nline2'
|
||||
21 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)
|
||||
22 |
|
||||
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `"raw string\n<\"\"\">\n<'''>"` instead of string join
|
||||
--> FLY002.py:21:8
|
||||
|
|
||||
19 | # https://github.com/astral-sh/ruff/issues/19887
|
||||
20 | nok8 = '\n'.join([r'line1','line2'])
|
||||
21 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
22 |
|
||||
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
|
||||
|
|
||||
help: Replace with `"raw string\n<\"\"\">\n<'''>"`
|
||||
18 | nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string)
|
||||
19 | # https://github.com/astral-sh/ruff/issues/19887
|
||||
20 | nok8 = '\n'.join([r'line1','line2'])
|
||||
- nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)
|
||||
21 + nok9 = "raw string\n<\"\"\">\n<'''>" # Not OK (both triple-quote delimiters appear; should bail)
|
||||
22 |
|
||||
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
|
||||
24 | def create_file_public_url(url, filename):
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `f"{url}{filename}"` instead of string join
|
||||
--> FLY002.py:25:11
|
||||
|
|
||||
|
|
@ -187,6 +228,110 @@ help: Replace with `f"{foo}'"`
|
|||
32 |
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `'"'` instead of string join
|
||||
--> FLY002.py:35:9
|
||||
|
|
||||
33 | # Regression test for: https://github.com/astral-sh/ruff/issues/21082
|
||||
34 | # Mixing raw and non-raw strings can cause syntax errors or behavior changes
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
|
|
||||
help: Replace with `'"'`
|
||||
32 |
|
||||
33 | # Regression test for: https://github.com/astral-sh/ruff/issues/21082
|
||||
34 | # Mixing raw and non-raw strings can cause syntax errors or behavior changes
|
||||
- nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
35 + nok14 = '"' # First is raw, second is not - would break syntax
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `"\\"` instead of string join
|
||||
--> FLY002.py:36:9
|
||||
|
|
||||
34 | # Mixing raw and non-raw strings can cause syntax errors or behavior changes
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
|
|
||||
help: Replace with `"\\"`
|
||||
33 | # Regression test for: https://github.com/astral-sh/ruff/issues/21082
|
||||
34 | # Mixing raw and non-raw strings can cause syntax errors or behavior changes
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
- nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
36 + nok15 = "\\" # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `"\x00"` instead of string join
|
||||
--> FLY002.py:37:9
|
||||
|
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
|
|
||||
help: Replace with `"\x00"`
|
||||
34 | # Mixing raw and non-raw strings can cause syntax errors or behavior changes
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
- nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
37 + nok16 = "\x00" # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
40 |
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `"\r"` instead of string join
|
||||
--> FLY002.py:38:9
|
||||
|
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
|
|
||||
help: Replace with `"\r"`
|
||||
35 | nok14 = "".join((r"", '"')) # First is raw, second is not - would break syntax
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
- nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
38 + nok17 = "\r" # First is raw, second has carriage return - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
40 |
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `"\\r"` instead of string join
|
||||
--> FLY002.py:39:9
|
||||
|
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
40 |
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
|
|
||||
help: Replace with `"\\r"`
|
||||
36 | nok15 = "".join((r"", "\\")) # First is raw, second has backslash - would break syntax
|
||||
37 | nok16 = "".join((r"", "\0")) # First is raw, second has null byte - would introduce null bytes
|
||||
38 | nok17 = "".join((r"", "\r")) # First is raw, second has carriage return - would change behavior
|
||||
- nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
39 + nok18 = "\\r" # First is raw, second has backslash escape followed by r
|
||||
40 |
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
42 | ok7 = "".join((r"", r"something")) # Both are raw - OK
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `r"something"` instead of string join
|
||||
--> FLY002.py:42:7
|
||||
|
|
||||
|
|
@ -196,7 +341,7 @@ FLY002 [*] Consider `r"something"` instead of string join
|
|||
43 | ok8 = "\n".join((r"line1", r'line2')) # Both are raw - OK
|
||||
|
|
||||
help: Replace with `r"something"`
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has carriage return escape - would change behavior
|
||||
39 | nok18 = "".join((r"", "\\r")) # First is raw, second has backslash escape followed by r
|
||||
40 |
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
- ok7 = "".join((r"", r"something")) # Both are raw - OK
|
||||
|
|
@ -206,7 +351,7 @@ help: Replace with `r"something"`
|
|||
45 | # Test that all-non-raw strings still work (should be OK)
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider f-string instead of string join
|
||||
FLY002 [*] Consider `"line1\nline2"` instead of string join
|
||||
--> FLY002.py:43:7
|
||||
|
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
|
|
@ -216,16 +361,15 @@ FLY002 [*] Consider f-string instead of string join
|
|||
44 |
|
||||
45 | # Test that all-non-raw strings still work (should be OK)
|
||||
|
|
||||
help: Replace with f-string
|
||||
help: Replace with `"line1\nline2"`
|
||||
40 |
|
||||
41 | # Test that all-raw strings still work (should be OK)
|
||||
42 | ok7 = "".join((r"", r"something")) # Both are raw - OK
|
||||
- ok8 = "\n".join((r"line1", r'line2')) # Both are raw - OK
|
||||
43 + ok8 = r"""line1
|
||||
44 + line2""" # Both are raw - OK
|
||||
45 |
|
||||
46 | # Test that all-non-raw strings still work (should be OK)
|
||||
47 | ok9 = "".join(("", '"')) # Both are non-raw - OK
|
||||
43 + ok8 = "line1\nline2" # Both are raw - OK
|
||||
44 |
|
||||
45 | # Test that all-non-raw strings still work (should be OK)
|
||||
46 | ok9 = "".join(("", '"')) # Both are non-raw - OK
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
FLY002 [*] Consider `'"'` instead of string join
|
||||
|
|
|
|||
Loading…
Reference in New Issue