mirror of https://github.com/astral-sh/ruff
[`flynt`] Fix f-string quoting for mixed quote joiners (`FLY002`) (#20662)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> Fixes #19837 Track quote usage across the joiner and parts to choose a safe f-string quote or skip the fix when both appear. ## Test Plan <!-- How was it tested? --> Add regression coverage to FLY002.py
This commit is contained in:
parent
92eee816ed
commit
542f080035
|
|
@ -23,3 +23,9 @@ nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote
|
||||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7197
|
# Regression test for: https://github.com/astral-sh/ruff/issues/7197
|
||||||
def create_file_public_url(url, filename):
|
def create_file_public_url(url, filename):
|
||||||
return''.join([url, filename])
|
return''.join([url, filename])
|
||||||
|
|
||||||
|
# Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
nok10 = "".join((foo, '"'))
|
||||||
|
nok11 = ''.join((foo, "'"))
|
||||||
|
nok12 = ''.join([foo, "'", '"'])
|
||||||
|
nok13 = "".join([foo, "'", '"'])
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,7 @@ use ast::FStringFlags;
|
||||||
use itertools::Itertools;
|
use itertools::Itertools;
|
||||||
|
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||||
use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags};
|
use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags, str::Quote};
|
||||||
use ruff_text_size::{Ranged, TextRange};
|
use ruff_text_size::{Ranged, TextRange};
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
use crate::checkers::ast::Checker;
|
||||||
|
|
@ -115,6 +115,8 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut f_string_elements = Vec::with_capacity(joinees.len() * 2);
|
let mut f_string_elements = Vec::with_capacity(joinees.len() * 2);
|
||||||
|
let mut has_single_quote = joiner.contains('\'');
|
||||||
|
let mut has_double_quote = joiner.contains('"');
|
||||||
let mut first = true;
|
let mut first = true;
|
||||||
|
|
||||||
for expr in joinees {
|
for expr in joinees {
|
||||||
|
|
@ -126,14 +128,31 @@ fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<
|
||||||
if !std::mem::take(&mut first) {
|
if !std::mem::take(&mut first) {
|
||||||
f_string_elements.push(helpers::to_interpolated_string_literal_element(joiner));
|
f_string_elements.push(helpers::to_interpolated_string_literal_element(joiner));
|
||||||
}
|
}
|
||||||
f_string_elements.push(helpers::to_interpolated_string_element(expr)?);
|
let element = helpers::to_interpolated_string_element(expr)?;
|
||||||
|
if let ast::InterpolatedStringElement::Literal(ast::InterpolatedStringLiteralElement {
|
||||||
|
value,
|
||||||
|
..
|
||||||
|
}) = &element
|
||||||
|
{
|
||||||
|
has_single_quote |= value.contains('\'');
|
||||||
|
has_double_quote |= value.contains('"');
|
||||||
}
|
}
|
||||||
|
f_string_elements.push(element);
|
||||||
|
}
|
||||||
|
|
||||||
|
let quote = flags.quote_style();
|
||||||
|
let adjusted_quote = match quote {
|
||||||
|
Quote::Single if has_single_quote && !has_double_quote => quote.opposite(),
|
||||||
|
Quote::Double if has_double_quote && !has_single_quote => quote.opposite(),
|
||||||
|
_ if has_double_quote && has_single_quote => return None,
|
||||||
|
_ => quote,
|
||||||
|
};
|
||||||
|
|
||||||
let node = ast::FString {
|
let node = ast::FString {
|
||||||
elements: f_string_elements.into(),
|
elements: f_string_elements.into(),
|
||||||
range: TextRange::default(),
|
range: TextRange::default(),
|
||||||
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
|
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
|
||||||
flags,
|
flags: flags.with_quote_style(adjusted_quote),
|
||||||
};
|
};
|
||||||
Some(node.into())
|
Some(node.into())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -153,6 +153,8 @@ FLY002 [*] Consider `f"{url}{filename}"` instead of string join
|
||||||
24 | def create_file_public_url(url, filename):
|
24 | def create_file_public_url(url, filename):
|
||||||
25 | return''.join([url, filename])
|
25 | return''.join([url, filename])
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
26 |
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
|
|
|
||||||
help: Replace with `f"{url}{filename}"`
|
help: Replace with `f"{url}{filename}"`
|
||||||
22 |
|
22 |
|
||||||
|
|
@ -160,4 +162,47 @@ help: Replace with `f"{url}{filename}"`
|
||||||
24 | def create_file_public_url(url, filename):
|
24 | def create_file_public_url(url, filename):
|
||||||
- return''.join([url, filename])
|
- return''.join([url, filename])
|
||||||
25 + return f"{url}{filename}"
|
25 + return f"{url}{filename}"
|
||||||
|
26 |
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
28 | nok10 = "".join((foo, '"'))
|
||||||
|
note: This is an unsafe fix and may change runtime behavior
|
||||||
|
|
||||||
|
FLY002 [*] Consider `f'{foo}"'` instead of string join
|
||||||
|
--> FLY002.py:28:9
|
||||||
|
|
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
28 | nok10 = "".join((foo, '"'))
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^
|
||||||
|
29 | nok11 = ''.join((foo, "'"))
|
||||||
|
30 | nok12 = ''.join([foo, "'", '"'])
|
||||||
|
|
|
||||||
|
help: Replace with `f'{foo}"'`
|
||||||
|
25 | return''.join([url, filename])
|
||||||
|
26 |
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
- nok10 = "".join((foo, '"'))
|
||||||
|
28 + nok10 = f'{foo}"'
|
||||||
|
29 | nok11 = ''.join((foo, "'"))
|
||||||
|
30 | nok12 = ''.join([foo, "'", '"'])
|
||||||
|
31 | nok13 = "".join([foo, "'", '"'])
|
||||||
|
note: This is an unsafe fix and may change runtime behavior
|
||||||
|
|
||||||
|
FLY002 [*] Consider `f"{foo}'"` instead of string join
|
||||||
|
--> FLY002.py:29:9
|
||||||
|
|
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
28 | nok10 = "".join((foo, '"'))
|
||||||
|
29 | nok11 = ''.join((foo, "'"))
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^
|
||||||
|
30 | nok12 = ''.join([foo, "'", '"'])
|
||||||
|
31 | nok13 = "".join([foo, "'", '"'])
|
||||||
|
|
|
||||||
|
help: Replace with `f"{foo}'"`
|
||||||
|
26 |
|
||||||
|
27 | # Regression test for: https://github.com/astral-sh/ruff/issues/19837
|
||||||
|
28 | nok10 = "".join((foo, '"'))
|
||||||
|
- nok11 = ''.join((foo, "'"))
|
||||||
|
29 + nok11 = f"{foo}'"
|
||||||
|
30 | nok12 = ''.join([foo, "'", '"'])
|
||||||
|
31 | nok13 = "".join([foo, "'", '"'])
|
||||||
note: This is an unsafe fix and may change runtime behavior
|
note: This is an unsafe fix and may change runtime behavior
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue