Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&[
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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))
}
Original file line number Diff line number Diff line change
@@ -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():
8 changes: 3 additions & 5 deletions crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading