Skip to content

Commit cf8d2e3

Browse files
hauntsaninjahauntsaninja
authored
New rule to prevent implicit string concatenation in collections (astral-sh#21972)
This is a common footgun, see the example in astral-sh#13014 (comment) Fixes astral-sh#13014 , fixes astral-sh#13031
1 parent 0290f5d commit cf8d2e3

9 files changed

Lines changed: 341 additions & 0 deletions

File tree

‎_typos.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ extend-exclude = [
44
"crates/ty_vendored/vendor/**/*",
55
"**/resources/**/*",
66
"**/snapshots/**/*",
7+
"crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/collection_literal.rs",
78
# Completion tests tend to have a lot of incomplete
89
# words naturally. It's annoying to have to make all
910
# of them actually words. So just ignore typos here.

‎crates/ruff_linter/resources/test/fixtures/flake8_implicit_str_concat/ISC004.py‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
facts = (
2+
"Lobsters have blue blood.",
3+
"The liver is the only human organ that can fully regenerate itself.",
4+
"Clarinets are made almost entirely out of wood from the mpingo tree."
5+
"In 1971, astronaut Alan Shepard played golf on the moon.",
6+
)
7+
8+
facts = [
9+
"Lobsters have blue blood.",
10+
"The liver is the only human organ that can fully regenerate itself.",
11+
"Clarinets are made almost entirely out of wood from the mpingo tree."
12+
"In 1971, astronaut Alan Shepard played golf on the moon.",
13+
]
14+
15+
facts = {
16+
"Lobsters have blue blood.",
17+
"The liver is the only human organ that can fully regenerate itself.",
18+
"Clarinets are made almost entirely out of wood from the mpingo tree."
19+
"In 1971, astronaut Alan Shepard played golf on the moon.",
20+
}
21+
22+
facts = {
23+
(
24+
"Clarinets are made almost entirely out of wood from the mpingo tree."
25+
"In 1971, astronaut Alan Shepard played golf on the moon."
26+
),
27+
}
28+
29+
facts = (
30+
"Octopuses have three hearts."
31+
# Missing comma here.
32+
"Honey never spoils.",
33+
)
34+
35+
facts = [
36+
"Octopuses have three hearts."
37+
# Missing comma here.
38+
"Honey never spoils.",
39+
]
40+
41+
facts = {
42+
"Octopuses have three hearts."
43+
# Missing comma here.
44+
"Honey never spoils.",
45+
}
46+
47+
facts = (
48+
(
49+
"Clarinets are made almost entirely out of wood from the mpingo tree."
50+
"In 1971, astronaut Alan Shepard played golf on the moon."
51+
),
52+
)
53+
54+
facts = [
55+
(
56+
"Clarinets are made almost entirely out of wood from the mpingo tree."
57+
"In 1971, astronaut Alan Shepard played golf on the moon."
58+
),
59+
]
60+
61+
facts = (
62+
"Lobsters have blue blood.\n"
63+
"The liver is the only human organ that can fully regenerate itself.\n"
64+
"Clarinets are made almost entirely out of wood from the mpingo tree.\n"
65+
"In 1971, astronaut Alan Shepard played golf on the moon.\n"
66+
)

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,13 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
214214
range: _,
215215
node_index: _,
216216
}) => {
217+
if checker.is_rule_enabled(Rule::ImplicitStringConcatenationInCollectionLiteral) {
218+
flake8_implicit_str_concat::rules::implicit_string_concatenation_in_collection_literal(
219+
checker,
220+
expr,
221+
elts,
222+
);
223+
}
217224
if ctx.is_store() {
218225
let check_too_many_expressions =
219226
checker.is_rule_enabled(Rule::ExpressionsInStarAssignment);
@@ -1329,6 +1336,13 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
13291336
}
13301337
}
13311338
Expr::Set(set) => {
1339+
if checker.is_rule_enabled(Rule::ImplicitStringConcatenationInCollectionLiteral) {
1340+
flake8_implicit_str_concat::rules::implicit_string_concatenation_in_collection_literal(
1341+
checker,
1342+
expr,
1343+
&set.elts,
1344+
);
1345+
}
13321346
if checker.is_rule_enabled(Rule::DuplicateValue) {
13331347
flake8_bugbear::rules::duplicate_value(checker, set);
13341348
}

‎crates/ruff_linter/src/codes.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
454454
(Flake8ImplicitStrConcat, "001") => rules::flake8_implicit_str_concat::rules::SingleLineImplicitStringConcatenation,
455455
(Flake8ImplicitStrConcat, "002") => rules::flake8_implicit_str_concat::rules::MultiLineImplicitStringConcatenation,
456456
(Flake8ImplicitStrConcat, "003") => rules::flake8_implicit_str_concat::rules::ExplicitStringConcatenation,
457+
(Flake8ImplicitStrConcat, "004") => rules::flake8_implicit_str_concat::rules::ImplicitStringConcatenationInCollectionLiteral,
457458

458459
// flake8-print
459460
(Flake8Print, "1") => rules::flake8_print::rules::Print,

‎crates/ruff_linter/src/rules/flake8_implicit_str_concat/mod.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ mod tests {
3232
Path::new("ISC_syntax_error_2.py")
3333
)]
3434
#[test_case(Rule::ExplicitStringConcatenation, Path::new("ISC.py"))]
35+
#[test_case(
36+
Rule::ImplicitStringConcatenationInCollectionLiteral,
37+
Path::new("ISC004.py")
38+
)]
3539
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
3640
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
3741
let diagnostics = test_path(

‎crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/collection_literal.rs‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
use ruff_python_ast::token::parenthesized_range;
3+
use ruff_python_ast::{Expr, StringLike};
4+
use ruff_text_size::Ranged;
5+
6+
use crate::checkers::ast::Checker;
7+
use crate::{Edit, Fix, FixAvailability, Violation};
8+
9+
/// ## What it does
10+
/// Checks for implicitly concatenated strings inside list, tuple, and set literals.
11+
///
12+
/// ## Why is this bad?
13+
/// In collection literals, implicit string concatenation is often the result of
14+
/// a missing comma between elements, which can silently merge items together.
15+
///
16+
/// ## Example
17+
/// ```python
18+
/// facts = (
19+
/// "Lobsters have blue blood.",
20+
/// "The liver is the only human organ that can fully regenerate itself.",
21+
/// "Clarinets are made almost entirely out of wood from the mpingo tree."
22+
/// "In 1971, astronaut Alan Shepard played golf on the moon.",
23+
/// )
24+
/// ```
25+
///
26+
/// Instead, you likely intended:
27+
/// ```python
28+
/// facts = (
29+
/// "Lobsters have blue blood.",
30+
/// "The liver is the only human organ that can fully regenerate itself.",
31+
/// "Clarinets are made almost entirely out of wood from the mpingo tree.",
32+
/// "In 1971, astronaut Alan Shepard played golf on the moon.",
33+
/// )
34+
/// ```
35+
///
36+
/// If the concatenation is intentional, wrap it in parentheses to make it
37+
/// explicit:
38+
/// ```python
39+
/// facts = (
40+
/// "Lobsters have blue blood.",
41+
/// "The liver is the only human organ that can fully regenerate itself.",
42+
/// (
43+
/// "Clarinets are made almost entirely out of wood from the mpingo tree."
44+
/// "In 1971, astronaut Alan Shepard played golf on the moon."
45+
/// ),
46+
/// )
47+
/// ```
48+
///
49+
/// ## Fix safety
50+
/// The fix is safe in that it does not change the semantics of your code.
51+
/// However, the issue is that you may often want to change semantics
52+
/// by adding a missing comma.
53+
#[derive(ViolationMetadata)]
54+
#[violation_metadata(preview_since = "0.14.10")]
55+
pub(crate) struct ImplicitStringConcatenationInCollectionLiteral;
56+
57+
impl Violation for ImplicitStringConcatenationInCollectionLiteral {
58+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always;
59+
60+
#[derive_message_formats]
61+
fn message(&self) -> String {
62+
"Unparenthesized implicit string concatenation in collection".to_string()
63+
}
64+
65+
fn fix_title(&self) -> Option<String> {
66+
Some("Wrap implicitly concatenated strings in parentheses".to_string())
67+
}
68+
}
69+
70+
/// ISC004
71+
pub(crate) fn implicit_string_concatenation_in_collection_literal(
72+
checker: &Checker,
73+
expr: &Expr,
74+
elements: &[Expr],
75+
) {
76+
for element in elements {
77+
let Ok(string_like) = StringLike::try_from(element) else {
78+
continue;
79+
};
80+
if !string_like.is_implicit_concatenated() {
81+
continue;
82+
}
83+
if parenthesized_range(
84+
string_like.as_expression_ref(),
85+
expr.into(),
86+
checker.tokens(),
87+
)
88+
.is_some()
89+
{
90+
continue;
91+
}
92+
93+
let mut diagnostic = checker.report_diagnostic(
94+
ImplicitStringConcatenationInCollectionLiteral,
95+
string_like.range(),
96+
);
97+
diagnostic.help("Did you forget a comma?");
98+
diagnostic.set_fix(Fix::unsafe_edits(
99+
Edit::insertion("(".to_string(), string_like.range().start()),
100+
[Edit::insertion(")".to_string(), string_like.range().end())],
101+
));
102+
}
103+
}

‎crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
pub(crate) use collection_literal::*;
12
pub(crate) use explicit::*;
23
pub(crate) use implicit::*;
34

5+
mod collection_literal;
46
mod explicit;
57
mod implicit;

‎crates/ruff_linter/src/rules/flake8_implicit_str_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__ISC004_ISC004.py.snap‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_implicit_str_concat/mod.rs
3+
---
4+
ISC004 [*] Unparenthesized implicit string concatenation in collection
5+
--> ISC004.py:4:5
6+
|
7+
2 | "Lobsters have blue blood.",
8+
3 | "The liver is the only human organ that can fully regenerate itself.",
9+
4 | / "Clarinets are made almost entirely out of wood from the mpingo tree."
10+
5 | | "In 1971, astronaut Alan Shepard played golf on the moon.",
11+
| |______________________________________________________________^
12+
6 | )
13+
|
14+
help: Wrap implicitly concatenated strings in parentheses
15+
help: Did you forget a comma?
16+
1 | facts = (
17+
2 | "Lobsters have blue blood.",
18+
3 | "The liver is the only human organ that can fully regenerate itself.",
19+
- "Clarinets are made almost entirely out of wood from the mpingo tree."
20+
- "In 1971, astronaut Alan Shepard played golf on the moon.",
21+
4 + ("Clarinets are made almost entirely out of wood from the mpingo tree."
22+
5 + "In 1971, astronaut Alan Shepard played golf on the moon."),
23+
6 | )
24+
7 |
25+
8 | facts = [
26+
note: This is an unsafe fix and may change runtime behavior
27+
28+
ISC004 [*] Unparenthesized implicit string concatenation in collection
29+
--> ISC004.py:11:5
30+
|
31+
9 | "Lobsters have blue blood.",
32+
10 | "The liver is the only human organ that can fully regenerate itself.",
33+
11 | / "Clarinets are made almost entirely out of wood from the mpingo tree."
34+
12 | | "In 1971, astronaut Alan Shepard played golf on the moon.",
35+
| |______________________________________________________________^
36+
13 | ]
37+
|
38+
help: Wrap implicitly concatenated strings in parentheses
39+
help: Did you forget a comma?
40+
8 | facts = [
41+
9 | "Lobsters have blue blood.",
42+
10 | "The liver is the only human organ that can fully regenerate itself.",
43+
- "Clarinets are made almost entirely out of wood from the mpingo tree."
44+
- "In 1971, astronaut Alan Shepard played golf on the moon.",
45+
11 + ("Clarinets are made almost entirely out of wood from the mpingo tree."
46+
12 + "In 1971, astronaut Alan Shepard played golf on the moon."),
47+
13 | ]
48+
14 |
49+
15 | facts = {
50+
note: This is an unsafe fix and may change runtime behavior
51+
52+
ISC004 [*] Unparenthesized implicit string concatenation in collection
53+
--> ISC004.py:18:5
54+
|
55+
16 | "Lobsters have blue blood.",
56+
17 | "The liver is the only human organ that can fully regenerate itself.",
57+
18 | / "Clarinets are made almost entirely out of wood from the mpingo tree."
58+
19 | | "In 1971, astronaut Alan Shepard played golf on the moon.",
59+
| |______________________________________________________________^
60+
20 | }
61+
|
62+
help: Wrap implicitly concatenated strings in parentheses
63+
help: Did you forget a comma?
64+
15 | facts = {
65+
16 | "Lobsters have blue blood.",
66+
17 | "The liver is the only human organ that can fully regenerate itself.",
67+
- "Clarinets are made almost entirely out of wood from the mpingo tree."
68+
- "In 1971, astronaut Alan Shepard played golf on the moon.",
69+
18 + ("Clarinets are made almost entirely out of wood from the mpingo tree."
70+
19 + "In 1971, astronaut Alan Shepard played golf on the moon."),
71+
20 | }
72+
21 |
73+
22 | facts = {
74+
note: This is an unsafe fix and may change runtime behavior
75+
76+
ISC004 [*] Unparenthesized implicit string concatenation in collection
77+
--> ISC004.py:30:5
78+
|
79+
29 | facts = (
80+
30 | / "Octopuses have three hearts."
81+
31 | | # Missing comma here.
82+
32 | | "Honey never spoils.",
83+
| |_________________________^
84+
33 | )
85+
|
86+
help: Wrap implicitly concatenated strings in parentheses
87+
help: Did you forget a comma?
88+
27 | }
89+
28 |
90+
29 | facts = (
91+
- "Octopuses have three hearts."
92+
30 + ("Octopuses have three hearts."
93+
31 | # Missing comma here.
94+
- "Honey never spoils.",
95+
32 + "Honey never spoils."),
96+
33 | )
97+
34 |
98+
35 | facts = [
99+
note: This is an unsafe fix and may change runtime behavior
100+
101+
ISC004 [*] Unparenthesized implicit string concatenation in collection
102+
--> ISC004.py:36:5
103+
|
104+
35 | facts = [
105+
36 | / "Octopuses have three hearts."
106+
37 | | # Missing comma here.
107+
38 | | "Honey never spoils.",
108+
| |_________________________^
109+
39 | ]
110+
|
111+
help: Wrap implicitly concatenated strings in parentheses
112+
help: Did you forget a comma?
113+
33 | )
114+
34 |
115+
35 | facts = [
116+
- "Octopuses have three hearts."
117+
36 + ("Octopuses have three hearts."
118+
37 | # Missing comma here.
119+
- "Honey never spoils.",
120+
38 + "Honey never spoils."),
121+
39 | ]
122+
40 |
123+
41 | facts = {
124+
note: This is an unsafe fix and may change runtime behavior
125+
126+
ISC004 [*] Unparenthesized implicit string concatenation in collection
127+
--> ISC004.py:42:5
128+
|
129+
41 | facts = {
130+
42 | / "Octopuses have three hearts."
131+
43 | | # Missing comma here.
132+
44 | | "Honey never spoils.",
133+
| |_________________________^
134+
45 | }
135+
|
136+
help: Wrap implicitly concatenated strings in parentheses
137+
help: Did you forget a comma?
138+
39 | ]
139+
40 |
140+
41 | facts = {
141+
- "Octopuses have three hearts."
142+
42 + ("Octopuses have three hearts."
143+
43 | # Missing comma here.
144+
- "Honey never spoils.",
145+
44 + "Honey never spoils."),
146+
45 | }
147+
46 |
148+
47 | facts = (
149+
note: This is an unsafe fix and may change runtime behavior

‎ruff.schema.json‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)