[RUF] Add rule to detect empty literal in deque call (RUF025) (#1…

…5104)
astral-sh · Jan 3, 2025 · 0837cdd · 0837cdd
1 parent 0dbfa8d
commit 0837cdd
Show file tree

Hide file tree

Showing 9 changed files with 318 additions and 5 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py
@@ -0,0 +1,58 @@
+from collections import deque
+import collections
+
+
+def f():
+    queue = collections.deque([])  # RUF025
+
+
+def f():
+    queue = collections.deque([], maxlen=10)  # RUF025
+
+
+def f():
+    queue = deque([])  # RUF025
+
+
+def f():
+    queue = deque(())  # RUF025
+
+
+def f():
+    queue = deque({})  # RUF025
+
+
+def f():
+    queue = deque(set())  # RUF025
+
+
+def f():
+    queue = collections.deque([], maxlen=10)  # RUF025
+
+
+def f():
+    class FakeDeque:
+        pass
+
+    deque = FakeDeque
+    queue = deque([])  # Ok
+
+
+def f():
+    class FakeSet:
+        pass
+
+    set = FakeSet
+    queue = deque(set())  # Ok
+
+
+def f():
+    queue = deque([1, 2])  # Ok
+
+
+def f():
+    queue = deque([1, 2], maxlen=10)  # Ok
+
+
+def f():
+    queue = deque()  # Ok
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs
@@ -1117,6 +1117,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::UnnecessaryRound) {
                 ruff::rules::unnecessary_round(checker, call);
             }
+            if checker.enabled(Rule::UnnecessaryEmptyIterableWithinDequeCall) {
+                ruff::rules::unnecessary_literal_within_deque_call(checker, call);
+            }
         }
         Expr::Dict(dict) => {
             if checker.any_enabled(&[

diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
@@ -971,6 +971,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "022") => (RuleGroup::Stable, rules::ruff::rules::UnsortedDunderAll),
         (Ruff, "023") => (RuleGroup::Stable, rules::ruff::rules::UnsortedDunderSlots),
         (Ruff, "024") => (RuleGroup::Stable, rules::ruff::rules::MutableFromkeysValue),
+        (Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryEmptyIterableWithinDequeCall),
         (Ruff, "026") => (RuleGroup::Stable, rules::ruff::rules::DefaultFactoryKwarg),
         (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
         (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),

diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs
@@ -51,6 +51,7 @@ mod tests {
     #[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.py"))]
     #[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
     #[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.py"))]
+    #[test_case(Rule::UnnecessaryEmptyIterableWithinDequeCall, Path::new("RUF025.py"))]
     #[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))]
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))]

diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs
@@ -36,6 +36,7 @@ pub(crate) use test_rules::*;
 pub(crate) use unnecessary_cast_to_int::*;
 pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
 pub(crate) use unnecessary_key_check::*;
+pub(crate) use unnecessary_literal_within_deque_call::*;
 pub(crate) use unnecessary_nested_literal::*;
 pub(crate) use unnecessary_regular_expression::*;
 pub(crate) use unnecessary_round::*;
@@ -89,6 +90,7 @@ pub(crate) mod test_rules;
 mod unnecessary_cast_to_int;
 mod unnecessary_iterable_allocation_for_first_element;
 mod unnecessary_key_check;
+mod unnecessary_literal_within_deque_call;
 mod unnecessary_nested_literal;
 mod unnecessary_regular_expression;
 mod unnecessary_round;

diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs
@@ -0,0 +1,120 @@
+use crate::checkers::ast::Checker;
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::{self as ast, Expr};
+use ruff_text_size::Ranged;
+
+/// ## What it does
+/// Checks for usages of `collections.deque` that have an empty iterable as the first argument.
+///
+/// ## Why is this bad?
+/// It's unnecessary to use an empty literal as a deque's iterable, since this is already the default behavior.
+///
+/// ## Examples
+///
+/// ```python
+/// from collections import deque
+///
+/// queue = deque(set())
+/// queue = deque([], 10)
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// from collections import deque
+///
+/// queue = deque()
+/// queue = deque(maxlen=10)
+/// ```
+///
+/// ## References
+/// - [Python documentation: `collections.deque`](https://docs.python.org/3/library/collections.html#collections.deque)
+#[derive(ViolationMetadata)]
+pub(crate) struct UnnecessaryEmptyIterableWithinDequeCall {
+    has_maxlen: bool,
+}
+
+impl AlwaysFixableViolation for UnnecessaryEmptyIterableWithinDequeCall {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "Unnecessary empty iterable within a deque call".to_string()
+    }
+
+    fn fix_title(&self) -> String {
+        let title = if self.has_maxlen {
+            "Replace with `deque(maxlen=...)`"
+        } else {
+            "Replace with `deque()`"
+        };
+        title.to_string()
+    }
+}
+
+/// RUF025
+pub(crate) fn unnecessary_literal_within_deque_call(checker: &mut Checker, deque: &ast::ExprCall) {
+    let ast::ExprCall {
+        func, arguments, ..
+    } = deque;
+
+    let Some(qualified) = checker.semantic().resolve_qualified_name(func) else {
+        return;
+    };
+    if !matches!(qualified.segments(), ["collections", "deque"]) || arguments.len() > 2 {
+        return;
+    }
+
+    let Some(iterable) = arguments.find_argument_value("iterable", 0) else {
+        return;
+    };
+
+    let maxlen = arguments.find_argument_value("maxlen", 1);
+
+    let is_empty_literal = match iterable {
+        Expr::Dict(dict) => dict.is_empty(),
+        Expr::List(list) => list.is_empty(),
+        Expr::Tuple(tuple) => tuple.is_empty(),
+        Expr::Call(call) => {
+            checker
+                .semantic()
+                .resolve_builtin_symbol(&call.func)
+                // other lints should handle empty list/dict/tuple calls,
+                // but this means that the lint still applies before those are fixed
+                .is_some_and(|name| {
+                    name == "set" || name == "list" || name == "dict" || name == "tuple"
+                })
+                && call.arguments.is_empty()
+        }
+        _ => false,
+    };
+    if !is_empty_literal {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryEmptyIterableWithinDequeCall {
+            has_maxlen: maxlen.is_some(),
+        },
+        deque.range,
+    );
+
+    diagnostic.set_fix(fix_unnecessary_literal_in_deque(checker, deque, maxlen));
+
+    checker.diagnostics.push(diagnostic);
+}
+
+fn fix_unnecessary_literal_in_deque(
+    checker: &Checker,
+    deque: &ast::ExprCall,
+    maxlen: Option<&Expr>,
+) -> Fix {
+    let deque_name = checker.locator().slice(deque.func.range());
+    let deque_str = match maxlen {
+        Some(maxlen) => {
+            let len_str = checker.locator().slice(maxlen);
+            format!("{deque_name}(maxlen={len_str})")
+        }
+        None => format!("{deque_name}()"),
+    };
+    Fix::safe_edit(Edit::range_replacement(deque_str, deque.range))
+}
diff --git a/...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF025_RUF025.py.snap b/...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF025_RUF025.py.snap
@@ -0,0 +1,129 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+snapshot_kind: text
+---
+RUF025.py:6:13: RUF025 [*] Unnecessary empty iterable within a deque call
+  |
+5 | def f():
+6 |     queue = collections.deque([])  # RUF025
+  |             ^^^^^^^^^^^^^^^^^^^^^ RUF025
+  |
+  = help: Replace with `deque()`
+
+ℹ Safe fix
+3 3 | 
+4 4 | 
+5 5 | def f():
+6   |-    queue = collections.deque([])  # RUF025
+  6 |+    queue = collections.deque()  # RUF025
+7 7 | 
+8 8 | 
+9 9 | def f():
+
+RUF025.py:10:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+ 9 | def f():
+10 |     queue = collections.deque([], maxlen=10)  # RUF025
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque(maxlen=...)`
+
+ℹ Safe fix
+7  7  | 
+8  8  | 
+9  9  | def f():
+10    |-    queue = collections.deque([], maxlen=10)  # RUF025
+   10 |+    queue = collections.deque(maxlen=10)  # RUF025
+11 11 | 
+12 12 | 
+13 13 | def f():
+
+RUF025.py:14:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+13 | def f():
+14 |     queue = deque([])  # RUF025
+   |             ^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque()`
+
+ℹ Safe fix
+11 11 | 
+12 12 | 
+13 13 | def f():
+14    |-    queue = deque([])  # RUF025
+   14 |+    queue = deque()  # RUF025
+15 15 | 
+16 16 | 
+17 17 | def f():
+
+RUF025.py:18:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+17 | def f():
+18 |     queue = deque(())  # RUF025
+   |             ^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque()`
+
+ℹ Safe fix
+15 15 | 
+16 16 | 
+17 17 | def f():
+18    |-    queue = deque(())  # RUF025
+   18 |+    queue = deque()  # RUF025
+19 19 | 
+20 20 | 
+21 21 | def f():
+
+RUF025.py:22:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+21 | def f():
+22 |     queue = deque({})  # RUF025
+   |             ^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque()`
+
+ℹ Safe fix
+19 19 | 
+20 20 | 
+21 21 | def f():
+22    |-    queue = deque({})  # RUF025
+   22 |+    queue = deque()  # RUF025
+23 23 | 
+24 24 | 
+25 25 | def f():
+
+RUF025.py:26:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+25 | def f():
+26 |     queue = deque(set())  # RUF025
+   |             ^^^^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque()`
+
+ℹ Safe fix
+23 23 | 
+24 24 | 
+25 25 | def f():
+26    |-    queue = deque(set())  # RUF025
+   26 |+    queue = deque()  # RUF025
+27 27 | 
+28 28 | 
+29 29 | def f():
+
+RUF025.py:30:13: RUF025 [*] Unnecessary empty iterable within a deque call
+   |
+29 | def f():
+30 |     queue = collections.deque([], maxlen=10)  # RUF025
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF025
+   |
+   = help: Replace with `deque(maxlen=...)`
+
+ℹ Safe fix
+27 27 | 
+28 28 | 
+29 29 | def f():
+30    |-    queue = collections.deque([], maxlen=10)  # RUF025
+   30 |+    queue = collections.deque(maxlen=10)  # RUF025
+31 31 | 
+32 32 | 
+33 33 | def f():
diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs
@@ -3774,14 +3774,14 @@ pub struct Arguments {
 }
 
 /// An entry in the argument list of a function call.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum ArgOrKeyword<'a> {
     Arg(&'a Expr),
     Keyword(&'a Keyword),
 }
 
 impl<'a> ArgOrKeyword<'a> {
-    pub const fn value(&self) -> &'a Expr {
+    pub const fn value(self) -> &'a Expr {
         match self {
             ArgOrKeyword::Arg(argument) => argument,
             ArgOrKeyword::Keyword(keyword) => &keyword.value,
@@ -3841,9 +3841,7 @@ impl Arguments {
     /// argument exists. Used to retrieve argument values that can be provided _either_ as keyword or
     /// positional arguments.
     pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> {
-        self.find_keyword(name)
-            .map(|keyword| &keyword.value)
-            .or_else(|| self.find_positional(position))
+        self.find_argument(name, position).map(ArgOrKeyword::value)
     }
 
     /// Return the the argument with the given name or at the given position, or `None` if no such

diff --git a/ruff.schema.json b/ruff.schema.json