Skip to content

Commit

Permalink
Update ISC001, ISC002 to check in f-strings (#7515)
Browse files Browse the repository at this point in the history
## Summary

This PR updates the implicit string concatenation rules, specifically `ISC001`
and `ISC002` to account for the new f-string tokens. `ISC003` checks for
explicit string concatenation and is not affected by PEP 701 because it
is based on AST.

### Implementation

The implementation is based on the boundary tokens of the f-string which are
`FStringStart` and `FStringEnd`. There are 4 cases to look for:
1. `String` followed by `FStringStart`
2. `FStringEnd` followed by `String`
3. `FStringEnd` followed by `FStringStart`
4. `String` followed by `String`

For f-string tokens, we use the `Indexer` to get the entire range of the f-string.
This is the range of the innermost f-string.

## Test Plan

Add new test cases for nested f-strings.
  • Loading branch information
dhruvmanila committed Sep 29, 2023
1 parent 19dc285 commit 363aeb9
Show file tree
Hide file tree
Showing 9 changed files with 420 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,23 @@
_ = foo + bar + "abc"
_ = "abc" + foo + bar
_ = foo + "abc" + bar

# Multiple strings nested inside a f-string
_ = f"a {'b' 'c' 'd'} e"
_ = f"""abc {"def" "ghi"} jkl"""
_ = f"""abc {
"def"
"ghi"
} jkl"""

# Nested f-strings
_ = "a" f"b {f"c" f"d"} e" "f"
_ = f"b {f"c" f"d {f"e" f"f"} g"} h"
_ = f"b {f"abc" \
f"def"} g"

# Explicitly concatenated nested f-strings
_ = f"a {f"first"
+ f"second"} d"
_ = f"a {f"first {f"middle"}"
+ f"second"} d"
1 change: 1 addition & 0 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ pub(crate) fn check_tokens(
tokens,
&settings.flake8_implicit_str_concat,
locator,
indexer,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use itertools::Itertools;
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::Tok;
use ruff_text_size::{Ranged, TextRange};

use ruff_diagnostics::{Diagnostic, Edit, Fix, FixKind, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{leading_quote, trailing_quote};
use ruff_python_index::Indexer;
use ruff_source_file::Locator;

use crate::rules::flake8_implicit_str_concat::settings::Settings;
Expand Down Expand Up @@ -94,6 +96,7 @@ pub(crate) fn implicit(
tokens: &[LexResult],
settings: &Settings,
locator: &Locator,
indexer: &Indexer,
) {
for ((a_tok, a_range), (b_tok, b_range)) in tokens
.iter()
Expand All @@ -103,24 +106,39 @@ pub(crate) fn implicit(
})
.tuple_windows()
{
if a_tok.is_string() && b_tok.is_string() {
if locator.contains_line_break(TextRange::new(a_range.end(), b_range.start())) {
diagnostics.push(Diagnostic::new(
MultiLineImplicitStringConcatenation,
TextRange::new(a_range.start(), b_range.end()),
));
} else {
let mut diagnostic = Diagnostic::new(
SingleLineImplicitStringConcatenation,
TextRange::new(a_range.start(), b_range.end()),
);

if let Some(fix) = concatenate_strings(*a_range, *b_range, locator) {
diagnostic.set_fix(fix);
}

diagnostics.push(diagnostic);
};
let (a_range, b_range) = match (a_tok, b_tok) {
(Tok::String { .. }, Tok::String { .. }) => (*a_range, *b_range),
(Tok::String { .. }, Tok::FStringStart) => (
*a_range,
indexer.fstring_ranges().innermost(b_range.start()).unwrap(),
),
(Tok::FStringEnd, Tok::String { .. }) => (
indexer.fstring_ranges().innermost(a_range.start()).unwrap(),
*b_range,
),
(Tok::FStringEnd, Tok::FStringStart) => (
indexer.fstring_ranges().innermost(a_range.start()).unwrap(),
indexer.fstring_ranges().innermost(b_range.start()).unwrap(),
),
_ => continue,
};

if locator.contains_line_break(TextRange::new(a_range.end(), b_range.start())) {
diagnostics.push(Diagnostic::new(
MultiLineImplicitStringConcatenation,
TextRange::new(a_range.start(), b_range.end()),
));
} else {
let mut diagnostic = Diagnostic::new(
SingleLineImplicitStringConcatenation,
TextRange::new(a_range.start(), b_range.end()),
);

if let Some(fix) = concatenate_strings(a_range, b_range, locator) {
diagnostic.set_fix(fix);
}

diagnostics.push(diagnostic);
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,147 @@ ISC.py:52:5: ISC001 [*] Implicitly concatenated string literals on one line
54 54 | # Single-line explicit concatenation should be ignored.
55 55 | _ = "abc" + "def" + "ghi"

ISC.py:64:10: ISC001 [*] Implicitly concatenated string literals on one line
|
63 | # Multiple strings nested inside a f-string
64 | _ = f"a {'b' 'c' 'd'} e"
| ^^^^^^^ ISC001
65 | _ = f"""abc {"def" "ghi"} jkl"""
66 | _ = f"""abc {
|
= help: Combine string literals

Fix
61 61 | _ = foo + "abc" + bar
62 62 |
63 63 | # Multiple strings nested inside a f-string
64 |-_ = f"a {'b' 'c' 'd'} e"
64 |+_ = f"a {'bc' 'd'} e"
65 65 | _ = f"""abc {"def" "ghi"} jkl"""
66 66 | _ = f"""abc {
67 67 | "def"

ISC.py:64:14: ISC001 [*] Implicitly concatenated string literals on one line
|
63 | # Multiple strings nested inside a f-string
64 | _ = f"a {'b' 'c' 'd'} e"
| ^^^^^^^ ISC001
65 | _ = f"""abc {"def" "ghi"} jkl"""
66 | _ = f"""abc {
|
= help: Combine string literals

Fix
61 61 | _ = foo + "abc" + bar
62 62 |
63 63 | # Multiple strings nested inside a f-string
64 |-_ = f"a {'b' 'c' 'd'} e"
64 |+_ = f"a {'b' 'cd'} e"
65 65 | _ = f"""abc {"def" "ghi"} jkl"""
66 66 | _ = f"""abc {
67 67 | "def"

ISC.py:65:14: ISC001 [*] Implicitly concatenated string literals on one line
|
63 | # Multiple strings nested inside a f-string
64 | _ = f"a {'b' 'c' 'd'} e"
65 | _ = f"""abc {"def" "ghi"} jkl"""
| ^^^^^^^^^^^ ISC001
66 | _ = f"""abc {
67 | "def"
|
= help: Combine string literals

Fix
62 62 |
63 63 | # Multiple strings nested inside a f-string
64 64 | _ = f"a {'b' 'c' 'd'} e"
65 |-_ = f"""abc {"def" "ghi"} jkl"""
65 |+_ = f"""abc {"defghi"} jkl"""
66 66 | _ = f"""abc {
67 67 | "def"
68 68 | "ghi"

ISC.py:72:5: ISC001 Implicitly concatenated string literals on one line
|
71 | # Nested f-strings
72 | _ = "a" f"b {f"c" f"d"} e" "f"
| ^^^^^^^^^^^^^^^^^^^^^^ ISC001
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
74 | _ = f"b {f"abc" \
|
= help: Combine string literals

ISC.py:72:9: ISC001 Implicitly concatenated string literals on one line
|
71 | # Nested f-strings
72 | _ = "a" f"b {f"c" f"d"} e" "f"
| ^^^^^^^^^^^^^^^^^^^^^^ ISC001
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
74 | _ = f"b {f"abc" \
|
= help: Combine string literals

ISC.py:72:14: ISC001 [*] Implicitly concatenated string literals on one line
|
71 | # Nested f-strings
72 | _ = "a" f"b {f"c" f"d"} e" "f"
| ^^^^^^^^^ ISC001
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
74 | _ = f"b {f"abc" \
|
= help: Combine string literals

Fix
69 69 | } jkl"""
70 70 |
71 71 | # Nested f-strings
72 |-_ = "a" f"b {f"c" f"d"} e" "f"
72 |+_ = "a" f"b {f"cd"} e" "f"
73 73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
74 74 | _ = f"b {f"abc" \
75 75 | f"def"} g"

ISC.py:73:10: ISC001 [*] Implicitly concatenated string literals on one line
|
71 | # Nested f-strings
72 | _ = "a" f"b {f"c" f"d"} e" "f"
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
| ^^^^^^^^^^^^^^^^^^^^^^^ ISC001
74 | _ = f"b {f"abc" \
75 | f"def"} g"
|
= help: Combine string literals

Fix
70 70 |
71 71 | # Nested f-strings
72 72 | _ = "a" f"b {f"c" f"d"} e" "f"
73 |-_ = f"b {f"c" f"d {f"e" f"f"} g"} h"
73 |+_ = f"b {f"cd {f"e" f"f"} g"} h"
74 74 | _ = f"b {f"abc" \
75 75 | f"def"} g"
76 76 |

ISC.py:73:20: ISC001 [*] Implicitly concatenated string literals on one line
|
71 | # Nested f-strings
72 | _ = "a" f"b {f"c" f"d"} e" "f"
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
| ^^^^^^^^^ ISC001
74 | _ = f"b {f"abc" \
75 | f"def"} g"
|
= help: Combine string literals

Fix
70 70 |
71 71 | # Nested f-strings
72 72 | _ = "a" f"b {f"c" f"d"} e" "f"
73 |-_ = f"b {f"c" f"d {f"e" f"f"} g"} h"
73 |+_ = f"b {f"c" f"d {f"ef"} g"} h"
74 74 | _ = f"b {f"abc" \
75 75 | f"def"} g"
76 76 |


Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,16 @@ ISC.py:5:5: ISC002 Implicitly concatenated string literals over multiple lines
8 | _ = (
|

ISC.py:74:10: ISC002 Implicitly concatenated string literals over multiple lines
|
72 | _ = "a" f"b {f"c" f"d"} e" "f"
73 | _ = f"b {f"c" f"d {f"e" f"f"} g"} h"
74 | _ = f"b {f"abc" \
| __________^
75 | | f"def"} g"
| |__________^ ISC002
76 |
77 | # Explicitly concatenated nested f-strings
|


Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,25 @@ ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concaten
21 | )
|

ISC.py:78:10: ISC003 Explicitly concatenated string should be implicitly concatenated
|
77 | # Explicitly concatenated nested f-strings
78 | _ = f"a {f"first"
| __________^
79 | | + f"second"} d"
| |_______________^ ISC003
80 | _ = f"a {f"first {f"middle"}"
81 | + f"second"} d"
|

ISC.py:80:10: ISC003 Explicitly concatenated string should be implicitly concatenated
|
78 | _ = f"a {f"first"
79 | + f"second"} d"
80 | _ = f"a {f"first {f"middle"}"
| __________^
81 | | + f"second"} d"
| |_______________^ ISC003
|


0 comments on commit 363aeb9

Please sign in to comment.