[ruff] Support slices in RUF005 (#17078)

## Summary

Teaches `RUF005` to also consider slices for concatenation. Other
indexing (`foo[0] + [7, 8, 9] + bar[1]`) is explicitly not considered.

```diff
 foo = [4, 5, 6]
-bar = [1, 2, 3] + foo
-slicing1 = foo[:1] + [7, 8, 9]
-slicing2 = [7, 8, 9] + bar[1:]
-slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
+bar = [1, 2, 3, *foo]
+slicing1 = [*foo[:1], 7, 8, 9]
+slicing2 = [7, 8, 9, *bar[1:]]
+slicing3 = [*foo[:1], 7, 8, 9, *bar[1:]]
```

## Test Plan

Manually tested (diff above from `ruff check --diff`), snapshot updated.
This commit is contained in:
Aarni Koskela 2025-03-31 16:09:39 +03:00 committed by GitHub
parent 2d7f118f52
commit 491a51960e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 107 additions and 10 deletions

View file

@ -0,0 +1,6 @@
foo = [4, 5, 6]
bar = [1, 2, 3] + foo
slicing1 = foo[:1] + [7, 8, 9]
slicing2 = [7, 8, 9] + bar[1:]
slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.

View file

@ -435,6 +435,7 @@ mod tests {
#[test_case(Rule::ClassWithMixedTypeVars, Path::new("RUF053.py"))]
#[test_case(Rule::IndentedFormFeed, Path::new("RUF054.py"))]
#[test_case(Rule::ImplicitClassVarInDataclass, Path::new("RUF045.py"))]
#[test_case(Rule::CollectionLiteralConcatenation, Path::new("RUF005_slices.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",

View file

@ -89,7 +89,7 @@ enum Type {
}
/// Recursively merge all the tuples and lists in the expression.
fn concatenate_expressions(expr: &Expr) -> Option<(Expr, Type)> {
fn concatenate_expressions(expr: &Expr, should_support_slices: bool) -> Option<(Expr, Type)> {
let Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::Add,
@ -101,18 +101,22 @@ fn concatenate_expressions(expr: &Expr) -> Option<(Expr, Type)> {
};
let new_left = match left.as_ref() {
Expr::BinOp(ast::ExprBinOp { .. }) => match concatenate_expressions(left) {
Some((new_left, _)) => new_left,
None => *left.clone(),
},
Expr::BinOp(ast::ExprBinOp { .. }) => {
match concatenate_expressions(left, should_support_slices) {
Some((new_left, _)) => new_left,
None => *left.clone(),
}
}
_ => *left.clone(),
};
let new_right = match right.as_ref() {
Expr::BinOp(ast::ExprBinOp { .. }) => match concatenate_expressions(right) {
Some((new_right, _)) => new_right,
None => *right.clone(),
},
Expr::BinOp(ast::ExprBinOp { .. }) => {
match concatenate_expressions(right, should_support_slices) {
Some((new_right, _)) => new_right,
None => *right.clone(),
}
}
_ => *right.clone(),
};
@ -139,6 +143,12 @@ fn concatenate_expressions(expr: &Expr) -> Option<(Expr, Type)> {
Expr::Call(_) | Expr::Attribute(_) | Expr::Name(_) => {
make_splat_elts(splat_element, other_elements, splat_at_left)
}
// Subscripts are also considered safe-ish to splat if the indexer is a slice.
Expr::Subscript(ast::ExprSubscript { slice, .. })
if should_support_slices && matches!(&**slice, Expr::Slice(_)) =>
{
make_splat_elts(splat_element, other_elements, splat_at_left)
}
// If the splat element is itself a list/tuple, insert them in the other list/tuple.
Expr::List(ast::ExprList { elts, .. }) if matches!(type_, Type::List) => {
other_elements.iter().chain(elts).cloned().collect()
@ -181,7 +191,9 @@ pub(crate) fn collection_literal_concatenation(checker: &Checker, expr: &Expr) {
return;
}
let Some((new_expr, type_)) = concatenate_expressions(expr) else {
let should_support_slices = checker.settings.preview.is_enabled();
let Some((new_expr, type_)) = concatenate_expressions(expr, should_support_slices) else {
return;
};

View file

@ -0,0 +1,78 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF005_slices.py:2:7: RUF005 [*] Consider `[1, 2, 3, *foo]` instead of concatenation
|
1 | foo = [4, 5, 6]
2 | bar = [1, 2, 3] + foo
| ^^^^^^^^^^^^^^^ RUF005
3 | slicing1 = foo[:1] + [7, 8, 9]
4 | slicing2 = [7, 8, 9] + bar[1:]
|
= help: Replace with `[1, 2, 3, *foo]`
Unsafe fix
1 1 | foo = [4, 5, 6]
2 |-bar = [1, 2, 3] + foo
2 |+bar = [1, 2, 3, *foo]
3 3 | slicing1 = foo[:1] + [7, 8, 9]
4 4 | slicing2 = [7, 8, 9] + bar[1:]
5 5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
RUF005_slices.py:3:12: RUF005 [*] Consider `[*foo[:1], 7, 8, 9]` instead of concatenation
|
1 | foo = [4, 5, 6]
2 | bar = [1, 2, 3] + foo
3 | slicing1 = foo[:1] + [7, 8, 9]
| ^^^^^^^^^^^^^^^^^^^ RUF005
4 | slicing2 = [7, 8, 9] + bar[1:]
5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
|
= help: Replace with `[*foo[:1], 7, 8, 9]`
Unsafe fix
1 1 | foo = [4, 5, 6]
2 2 | bar = [1, 2, 3] + foo
3 |-slicing1 = foo[:1] + [7, 8, 9]
3 |+slicing1 = [*foo[:1], 7, 8, 9]
4 4 | slicing2 = [7, 8, 9] + bar[1:]
5 5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
6 6 | indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.
RUF005_slices.py:4:12: RUF005 [*] Consider `[7, 8, 9, *bar[1:]]` instead of concatenation
|
2 | bar = [1, 2, 3] + foo
3 | slicing1 = foo[:1] + [7, 8, 9]
4 | slicing2 = [7, 8, 9] + bar[1:]
| ^^^^^^^^^^^^^^^^^^^ RUF005
5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
6 | indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.
|
= help: Replace with `[7, 8, 9, *bar[1:]]`
Unsafe fix
1 1 | foo = [4, 5, 6]
2 2 | bar = [1, 2, 3] + foo
3 3 | slicing1 = foo[:1] + [7, 8, 9]
4 |-slicing2 = [7, 8, 9] + bar[1:]
4 |+slicing2 = [7, 8, 9, *bar[1:]]
5 5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
6 6 | indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.
RUF005_slices.py:5:12: RUF005 [*] Consider `[*foo[:1], 7, 8, 9, *bar[1:]]` instead of concatenation
|
3 | slicing1 = foo[:1] + [7, 8, 9]
4 | slicing2 = [7, 8, 9] + bar[1:]
5 | slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF005
6 | indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.
|
= help: Replace with `[*foo[:1], 7, 8, 9, *bar[1:]]`
Unsafe fix
2 2 | bar = [1, 2, 3] + foo
3 3 | slicing1 = foo[:1] + [7, 8, 9]
4 4 | slicing2 = [7, 8, 9] + bar[1:]
5 |-slicing3 = foo[:1] + [7, 8, 9] + bar[1:]
5 |+slicing3 = [*foo[:1], 7, 8, 9, *bar[1:]]
6 6 | indexing = foo[0] + [7, 8, 9] + bar[1] # Not changed; looks a little suspect for concatenation.