Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flynt/FLY002.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
nok5 = "a".join([choice("flarp")]) # Not OK (not a simple call)
nok6 = "a".join(x for x in "feefoofum") # Not OK (generator)
nok7 = "a".join([f"foo{8}", "bar"]) # Not OK (contains an f-string)

# https://github.com/astral-sh/ruff/issues/19887
nok8 = '\n'.join([r'line1','line2'])
nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)

# Regression test for: https://github.com/astral-sh/ruff/issues/7197
def create_file_public_url(url, filename):
Expand Down
54 changes: 36 additions & 18 deletions crates/ruff_linter/src/rules/flynt/rules/static_join_to_fstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ast::FStringFlags;
use itertools::Itertools;

use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::{self as ast, Arguments, Expr};
use ruff_python_ast::{self as ast, Arguments, Expr, StringFlags};
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -72,24 +72,42 @@ fn is_static_length(elts: &[Expr]) -> bool {
fn build_fstring(joiner: &str, joinees: &[Expr], flags: FStringFlags) -> Option<Expr> {
// If all elements are string constants, join them into a single string.
if joinees.iter().all(Expr::is_string_literal_expr) {
let mut flags = None;
let node = ast::StringLiteral {
value: joinees
.iter()
.filter_map(|expr| {
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
if flags.is_none() {
// take the flags from the first Expr
flags = Some(value.first_literal_flags());
}
Some(value.to_str())
} else {
None
let mut flags: Option<ast::StringLiteralFlags> = None;
let content = joinees
.iter()
.filter_map(|expr| {
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
if flags.is_none() {
// Take the flags from the first Expr
flags = Some(value.first_literal_flags());
}
})
.join(joiner)
.into_boxed_str(),
flags: flags?,
Some(value.to_str())
} else {
None
}
})
.join(joiner);

let mut flags = flags?;

// 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);

// Prefer a delimiter that doesn't occur in the content; if both occur, bail.
if content.contains(flags.quote_str()) {
flags = flags.with_quote_style(flags.quote_style().opposite());
if content.contains(flags.quote_str()) {
// Both "'''" and "\"\"\"" are present in content; avoid emitting
// an invalid raw triple-quoted literal (or escaping). Bail on the fix.
return None;
}
}
Comment on lines +97 to +105
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something like this would help with #19837 too, but it looks sufficiently different that we should leave it for a separate PR. I think we'd need this kind of check in one of the helpers below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I will try it in a different pull request:)

}

let node = ast::StringLiteral {
value: content.into_boxed_str(),
flags,
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,39 @@ 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 f-string 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 f-string
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 = r'''line1
21 + line2'''
22 | nok9 = '\n'.join([r"raw string", '<""">', "<'''>"]) # Not OK (both triple-quote delimiters appear; should bail)
23 |
24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
note: This is an unsafe fix and may change runtime behavior

FLY002 [*] Consider `f"{url}{filename}"` instead of string join
--> FLY002.py:23:11
--> FLY002.py:25:11
|
21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
22 | def create_file_public_url(url, filename):
23 | return''.join([url, filename])
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
24 | def create_file_public_url(url, filename):
25 | return''.join([url, filename])
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
help: Replace with `f"{url}{filename}"`
20 |
21 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
22 | def create_file_public_url(url, filename):
22 |
23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7197
24 | def create_file_public_url(url, filename):
- return''.join([url, filename])
23 + return f"{url}{filename}"
25 + return f"{url}{filename}"
note: This is an unsafe fix and may change runtime behavior
Loading