mirror of https://github.com/astral-sh/ruff
[`RUF`] Add rule to detect empty literal in deque call (`RUF025`) (#15104)
This commit is contained in:
parent
0dbfa8d0e0
commit
0837cdd931
|
|
@ -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
|
||||||
|
|
@ -1117,6 +1117,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::UnnecessaryRound) {
|
if checker.enabled(Rule::UnnecessaryRound) {
|
||||||
ruff::rules::unnecessary_round(checker, call);
|
ruff::rules::unnecessary_round(checker, call);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::UnnecessaryEmptyIterableWithinDequeCall) {
|
||||||
|
ruff::rules::unnecessary_literal_within_deque_call(checker, call);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Expr::Dict(dict) => {
|
Expr::Dict(dict) => {
|
||||||
if checker.any_enabled(&[
|
if checker.any_enabled(&[
|
||||||
|
|
|
||||||
|
|
@ -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, "022") => (RuleGroup::Stable, rules::ruff::rules::UnsortedDunderAll),
|
||||||
(Ruff, "023") => (RuleGroup::Stable, rules::ruff::rules::UnsortedDunderSlots),
|
(Ruff, "023") => (RuleGroup::Stable, rules::ruff::rules::UnsortedDunderSlots),
|
||||||
(Ruff, "024") => (RuleGroup::Stable, rules::ruff::rules::MutableFromkeysValue),
|
(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, "026") => (RuleGroup::Stable, rules::ruff::rules::DefaultFactoryKwarg),
|
||||||
(Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
|
(Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
|
||||||
(Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),
|
(Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),
|
||||||
|
|
|
||||||
|
|
@ -51,6 +51,7 @@ mod tests {
|
||||||
#[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.py"))]
|
#[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.py"))]
|
||||||
#[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
|
#[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
|
||||||
#[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.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::DefaultFactoryKwarg, Path::new("RUF026.py"))]
|
||||||
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
|
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
|
||||||
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))]
|
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))]
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@ pub(crate) use test_rules::*;
|
||||||
pub(crate) use unnecessary_cast_to_int::*;
|
pub(crate) use unnecessary_cast_to_int::*;
|
||||||
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
|
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
|
||||||
pub(crate) use unnecessary_key_check::*;
|
pub(crate) use unnecessary_key_check::*;
|
||||||
|
pub(crate) use unnecessary_literal_within_deque_call::*;
|
||||||
pub(crate) use unnecessary_nested_literal::*;
|
pub(crate) use unnecessary_nested_literal::*;
|
||||||
pub(crate) use unnecessary_regular_expression::*;
|
pub(crate) use unnecessary_regular_expression::*;
|
||||||
pub(crate) use unnecessary_round::*;
|
pub(crate) use unnecessary_round::*;
|
||||||
|
|
@ -89,6 +90,7 @@ pub(crate) mod test_rules;
|
||||||
mod unnecessary_cast_to_int;
|
mod unnecessary_cast_to_int;
|
||||||
mod unnecessary_iterable_allocation_for_first_element;
|
mod unnecessary_iterable_allocation_for_first_element;
|
||||||
mod unnecessary_key_check;
|
mod unnecessary_key_check;
|
||||||
|
mod unnecessary_literal_within_deque_call;
|
||||||
mod unnecessary_nested_literal;
|
mod unnecessary_nested_literal;
|
||||||
mod unnecessary_regular_expression;
|
mod unnecessary_regular_expression;
|
||||||
mod unnecessary_round;
|
mod unnecessary_round;
|
||||||
|
|
|
||||||
|
|
@ -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))
|
||||||
|
}
|
||||||
|
|
@ -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():
|
||||||
|
|
@ -3774,14 +3774,14 @@ pub struct Arguments {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// An entry in the argument list of a function call.
|
/// An entry in the argument list of a function call.
|
||||||
#[derive(Clone, Debug, PartialEq)]
|
#[derive(Copy, Clone, Debug, PartialEq)]
|
||||||
pub enum ArgOrKeyword<'a> {
|
pub enum ArgOrKeyword<'a> {
|
||||||
Arg(&'a Expr),
|
Arg(&'a Expr),
|
||||||
Keyword(&'a Keyword),
|
Keyword(&'a Keyword),
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> ArgOrKeyword<'a> {
|
impl<'a> ArgOrKeyword<'a> {
|
||||||
pub const fn value(&self) -> &'a Expr {
|
pub const fn value(self) -> &'a Expr {
|
||||||
match self {
|
match self {
|
||||||
ArgOrKeyword::Arg(argument) => argument,
|
ArgOrKeyword::Arg(argument) => argument,
|
||||||
ArgOrKeyword::Keyword(keyword) => &keyword.value,
|
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
|
/// argument exists. Used to retrieve argument values that can be provided _either_ as keyword or
|
||||||
/// positional arguments.
|
/// positional arguments.
|
||||||
pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> {
|
pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> {
|
||||||
self.find_keyword(name)
|
self.find_argument(name, position).map(ArgOrKeyword::value)
|
||||||
.map(|keyword| &keyword.value)
|
|
||||||
.or_else(|| self.find_positional(position))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Return the the argument with the given name or at the given position, or `None` if no such
|
/// Return the the argument with the given name or at the given position, or `None` if no such
|
||||||
|
|
|
||||||
|
|
@ -3849,6 +3849,7 @@
|
||||||
"RUF022",
|
"RUF022",
|
||||||
"RUF023",
|
"RUF023",
|
||||||
"RUF024",
|
"RUF024",
|
||||||
|
"RUF025",
|
||||||
"RUF026",
|
"RUF026",
|
||||||
"RUF027",
|
"RUF027",
|
||||||
"RUF028",
|
"RUF028",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue