Add RUF005 "unpack instead of concatenating" check

README.md

@@ -1174,6 +1174,7 @@ For more, see [flake8-no-pep420](https://pypi.org/project/flake8-no-pep420/2.3.0
 | RUF002 | AmbiguousUnicodeCharacterDocstring | Docstring contains ambiguous unicode character '{confusable}' (did you mean '{representant}'?) | 🛠 |
 | RUF003 | AmbiguousUnicodeCharacterComment | Comment contains ambiguous unicode character '{confusable}' (did you mean '{representant}'?) | 🛠 |
 | RUF004 | KeywordArgumentBeforeStarArgument | Keyword argument `{name}` must come after starred arguments |  |
+| RUF005 | UnpackInsteadOfConcatenatingToCollectionLiteral | Consider `{expr}` instead of concatenation |  |
 | RUF100 | UnusedNOQA | Unused blanket `noqa` directive | 🛠 |
 
 <!-- End auto-generated sections. -->

resources/test/fixtures/ruff/RUF005.py

@@ -0,0 +1,22 @@
+class Fun:
+    words = ("how", "fun!")
+
+    def yay(self):
+        return self.words
+
+yay = Fun().yay
+
+foo = [4, 5, 6]
+bar = [1, 2, 3] + foo
+zoob = tuple(bar)
+quux = (7, 8, 9) + zoob
+spam = quux + (10, 11, 12)
+spom = list(spam)
+eggs = spom + [13, 14, 15]
+elatement = ("we all say", ) + yay()
+excitement = ("we all think", ) + Fun().yay()
+astonishment = ("we all feel", ) + Fun.words
+
+chain = ['a', 'b', 'c'] + eggs + list(('yes', 'no', 'pants') + zoob)
+
+baz = () + zoob

ruff.schema.json

@@ -1631,6 +1631,7 @@
         "RUF002",
         "RUF003",
         "RUF004",
+        "RUF005",
         "RUF1",
         "RUF10",
         "RUF100",

src/checkers/ast.rs

@@ -2722,6 +2722,13 @@ where
                         self.diagnostics.push(diagnostic);
                     }
                 }
+                if self
+                    .settings
+                    .rules
+                    .enabled(&Rule::UnpackInsteadOfConcatenatingToCollectionLiteral)
+                {
+                    ruff::rules::unpack_instead_of_concatenating_to_collection_literal(self, expr);
+                }
             }
             ExprKind::UnaryOp { op, operand } => {
                 let check_not_in = self.settings.rules.enabled(&Rule::NotInTest);

src/registry.rs

@@ -421,6 +421,7 @@ ruff_macros::define_rule_mapping!(
     RUF002 => violations::AmbiguousUnicodeCharacterDocstring,
     RUF003 => violations::AmbiguousUnicodeCharacterComment,
     RUF004 => violations::KeywordArgumentBeforeStarArgument,
+    RUF005 => violations::UnpackInsteadOfConcatenatingToCollectionLiteral,
     RUF100 => violations::UnusedNOQA,
 );
 

src/rules/ruff/mod.rs

@@ -14,6 +14,7 @@ mod tests {
     use crate::registry::Rule;
     use crate::settings;
     #[test_case(Rule::KeywordArgumentBeforeStarArgument, Path::new("RUF004.py"); "RUF004")]
+    #[test_case(Rule::UnpackInsteadOfConcatenatingToCollectionLiteral, Path::new("RUF005.py"); "RUF005")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
         let diagnostics = test_path(

src/rules/ruff/rules.rs → src/rules/ruff/rules/ambiguous_unicode_character.rs

@@ -1,10 +1,11 @@
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
-use rustpython_ast::{Expr, ExprKind, Keyword, KeywordData, Location};
 
 use crate::ast::types::Range;
 use crate::fix::Fix;
+use crate::message::Location;
 use crate::registry::{Diagnostic, DiagnosticKind};
+use crate::rules::ruff::rules::Context;
 use crate::settings::{flags, Settings};
 use crate::source_code::Locator;
 use crate::violations;
@@ -1597,13 +1598,6 @@ static CONFUSABLES: Lazy<FxHashMap<u32, u32>> = Lazy::new(|| {
     ])
 });
 
-#[derive(Clone, Copy)]
-pub enum Context {
-    String,
-    Docstring,
-    Comment,
-}
-
 pub fn ambiguous_unicode_character(
     locator: &Locator,
     start: Location,
@@ -1677,28 +1671,3 @@ pub fn ambiguous_unicode_character(
 
     diagnostics
 }
-
-/// RUF004
-pub fn keyword_argument_before_star_argument(
-    args: &[Expr],
-    keywords: &[Keyword],
-) -> Vec<Diagnostic> {
-    let mut diagnostics = vec![];
-    if let Some(arg) = args
-        .iter()
-        .rfind(|arg| matches!(arg.node, ExprKind::Starred { .. }))
-    {
-        for keyword in keywords {
-            if keyword.location < arg.location {
-                let KeywordData { arg, .. } = &keyword.node;
-                if let Some(arg) = arg {
-                    diagnostics.push(Diagnostic::new(
-                        violations::KeywordArgumentBeforeStarArgument(arg.to_string()),
-                        Range::from_located(keyword),
-                    ));
-                }
-            }
-        }
-    }
-    diagnostics
-}

src/rules/ruff/rules/mod.rs

@@ -0,0 +1,43 @@
+use rustpython_ast::{Expr, ExprKind, Keyword, KeywordData};
+
+use crate::ast::types::Range;
+use crate::registry::Diagnostic;
+use crate::violations;
+
+mod ambiguous_unicode_character;
+mod unpack_instead_of_concatenating_to_collection_literal;
+
+pub use ambiguous_unicode_character::ambiguous_unicode_character;
+pub use unpack_instead_of_concatenating_to_collection_literal::unpack_instead_of_concatenating_to_collection_literal;
+
+#[derive(Clone, Copy)]
+pub enum Context {
+    String,
+    Docstring,
+    Comment,
+}
+
+/// RUF004
+pub fn keyword_argument_before_star_argument(
+    args: &[Expr],
+    keywords: &[Keyword],
+) -> Vec<Diagnostic> {
+    let mut diagnostics = vec![];
+    if let Some(arg) = args
+        .iter()
+        .rfind(|arg| matches!(arg.node, ExprKind::Starred { .. }))
+    {
+        for keyword in keywords {
+            if keyword.location < arg.location {
+                let KeywordData { arg, .. } = &keyword.node;
+                if let Some(arg) = arg {
+                    diagnostics.push(Diagnostic::new(
+                        violations::KeywordArgumentBeforeStarArgument(arg.to_string()),
+                        Range::from_located(keyword),
+                    ));
+                }
+            }
+        }
+    }
+    diagnostics
+}

src/rules/ruff/rules/unpack_instead_of_concatenating_to_collection_literal.rs

@@ -0,0 +1,95 @@
+use rustpython_ast::{Expr, ExprContext, ExprKind, Operator};
+
+use crate::ast::helpers::{create_expr, unparse_expr};
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::fix::Fix;
+use crate::registry::Diagnostic;
+use crate::violations;
+
+fn make_splat_elts(
+    splat_element: &Expr,
+    other_elements: &[Expr],
+    splat_at_left: bool,
+) -> Vec<Expr> {
+    let mut new_elts = other_elements.to_owned();
+    let splat = create_expr(ExprKind::Starred {
+        value: Box::from(splat_element.clone()),
+        ctx: ExprContext::Load,
+    });
+    if splat_at_left {
+        new_elts.insert(0, splat);
+    } else {
+        new_elts.push(splat);
+    }
+    new_elts
+}
+
+#[derive(Debug)]
+enum Kind {
+    List,
+    Tuple,
+}
+
+/// RUF005
+/// This suggestion could be unsafe if the non-literal expression in the
+/// expression has overridden the `__add__` (or `__radd__`) magic methods.
+pub fn unpack_instead_of_concatenating_to_collection_literal(checker: &mut Checker, expr: &Expr) {
+    let ExprKind::BinOp { op, left, right } = &expr.node else {
+        return;
+    };
+    if !matches!(op, Operator::Add) {
+        return;
+    }
+
+    // Figure out which way the splat is, and what the kind of the collection is.
+    let (kind, splat_element, other_elements, splat_at_left, ctx) = match (&left.node, &right.node)
+    {
+        (ExprKind::List { elts: l_elts, ctx }, _) => (Kind::List, right, l_elts, false, ctx),
+        (ExprKind::Tuple { elts: l_elts, ctx }, _) => (Kind::Tuple, right, l_elts, false, ctx),
+        (_, ExprKind::List { elts: r_elts, ctx }) => (Kind::List, left, r_elts, true, ctx),
+        (_, ExprKind::Tuple { elts: r_elts, ctx }) => (Kind::Tuple, left, r_elts, true, ctx),
+        _ => return,
+    };
+
+    // We'll be a bit conservative here; only calls, names and attribute accesses
+    // will be considered as splat elements.
+    if !matches!(
+        splat_element.node,
+        ExprKind::Call { .. } | ExprKind::Name { .. } | ExprKind::Attribute { .. }
+    ) {
+        return;
+    }
+
+    let new_expr = match kind {
+        Kind::List => create_expr(ExprKind::List {
+            elts: make_splat_elts(splat_element, other_elements, splat_at_left),
+            ctx: ctx.clone(),
+        }),
+        Kind::Tuple => create_expr(ExprKind::Tuple {
+            elts: make_splat_elts(splat_element, other_elements, splat_at_left),
+            ctx: ctx.clone(),
+        }),
+    };
+
+    let mut new_expr_string = unparse_expr(&new_expr, checker.stylist);
+
+    new_expr_string = match kind {
+        // Wrap the new expression in parentheses if it was a tuple
+        Kind::Tuple => format!("({new_expr_string})"),
+        Kind::List => new_expr_string,
+    };
+
+    let mut diagnostic = Diagnostic::new(
+        violations::UnpackInsteadOfConcatenatingToCollectionLiteral(new_expr_string.clone()),
+        Range::from_located(expr),
+    );
+    if checker.patch(diagnostic.kind.rule()) {
+        diagnostic.amend(Fix::replacement(
+            new_expr_string,
+            expr.location,
+            expr.end_location.unwrap(),
+        ));
+    }
+    checker.diagnostics.push(diagnostic);
+}