diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py new file mode 100644 index 0000000000000..aecb3d0b4cdb9 --- /dev/null +++ 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 index 6d00aad227d6e..a312e552636c9 100644 --- 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 index 55453c58210e8..30bc6949f8c88 100644 --- 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 index 7f67af7c1d866..e78c31f065c9b 100644 --- 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 index 241d8a2bf6c83..327d805fd9cc3 100644 --- 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 new file mode 100644 index 0000000000000..85598b371440a --- /dev/null +++ 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/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF025_RUF025.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF025_RUF025.py.snap new file mode 100644 index 0000000000000..a4dc28c0b1aac --- /dev/null +++ b/crates/ruff_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 index 639f8d16ddca3..e844eb0534681 100644 --- 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 index 3c2fdb5e2d87a..b894dbb692d0d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3849,6 +3849,7 @@ "RUF022", "RUF023", "RUF024", + "RUF025", "RUF026", "RUF027", "RUF028",