mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 10:48:32 +00:00
[refurb] Implement repeated-append
rule (FURB113
) (#6702)
## Summary As an initial effort with replicating `refurb` rules (#1348 ), this PR adds support for [FURB113](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/list_extend.py) and adds a new category of checks. ## Test Plan I included a new test + checked that all other tests pass.
This commit is contained in:
parent
1439bb592e
commit
26d53c56a2
13 changed files with 1003 additions and 11 deletions
169
crates/ruff/resources/test/fixtures/refurb/FURB113.py
vendored
Normal file
169
crates/ruff/resources/test/fixtures/refurb/FURB113.py
vendored
Normal file
|
@ -0,0 +1,169 @@
|
|||
from typing import List
|
||||
|
||||
|
||||
def func():
|
||||
pass
|
||||
|
||||
|
||||
nums = []
|
||||
nums2 = []
|
||||
nums3: list[int] = func()
|
||||
nums4: List[int]
|
||||
|
||||
|
||||
class C:
|
||||
def append(self, x):
|
||||
pass
|
||||
|
||||
|
||||
# Errors.
|
||||
|
||||
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums.append(2)
|
||||
pass
|
||||
|
||||
|
||||
# FURB113
|
||||
nums3.append(1)
|
||||
nums3.append(2)
|
||||
pass
|
||||
|
||||
|
||||
# FURB113
|
||||
nums4.append(1)
|
||||
nums4.append(2)
|
||||
pass
|
||||
|
||||
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums2.append(1)
|
||||
nums.append(2)
|
||||
nums.append(3)
|
||||
pass
|
||||
|
||||
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums2.append(1)
|
||||
nums.append(2)
|
||||
# FURB113
|
||||
nums3.append(1)
|
||||
nums.append(3)
|
||||
# FURB113
|
||||
nums4.append(1)
|
||||
nums4.append(2)
|
||||
nums3.append(2)
|
||||
pass
|
||||
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums.append(2)
|
||||
nums.append(3)
|
||||
|
||||
|
||||
if True:
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums.append(2)
|
||||
|
||||
|
||||
if True:
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums.append(2)
|
||||
pass
|
||||
|
||||
|
||||
if True:
|
||||
# FURB113
|
||||
nums.append(1)
|
||||
nums2.append(1)
|
||||
nums.append(2)
|
||||
nums.append(3)
|
||||
|
||||
|
||||
def yes_one(x: list[int]):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
def yes_two(x: List[int]):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
def yes_three(*, x: list[int]):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
def yes_four(x: list[int], /):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
def yes_five(x: list[int], y: list[int]):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
y.append(1)
|
||||
x.append(3)
|
||||
|
||||
|
||||
def yes_six(x: list):
|
||||
# FURB113
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
# Non-errors.
|
||||
|
||||
nums.append(1)
|
||||
pass
|
||||
nums.append(2)
|
||||
|
||||
|
||||
if True:
|
||||
nums.append(1)
|
||||
pass
|
||||
nums.append(2)
|
||||
|
||||
|
||||
nums.append(1)
|
||||
pass
|
||||
|
||||
|
||||
nums.append(1)
|
||||
nums2.append(2)
|
||||
|
||||
|
||||
nums.copy()
|
||||
nums.copy()
|
||||
|
||||
|
||||
c = C()
|
||||
c.append(1)
|
||||
c.append(2)
|
||||
|
||||
|
||||
def not_one(x):
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
def not_two(x: C):
|
||||
x.append(1)
|
||||
x.append(2)
|
||||
|
||||
|
||||
# redefining a list variable with a new type shouldn't confuse ruff.
|
||||
nums2 = C()
|
||||
nums2.append(1)
|
||||
nums2.append(2)
|
|
@ -13,7 +13,7 @@ use crate::rules::{
|
|||
flake8_django, flake8_errmsg, flake8_import_conventions, flake8_pie, flake8_pyi,
|
||||
flake8_pytest_style, flake8_raise, flake8_return, flake8_simplify, flake8_slots,
|
||||
flake8_tidy_imports, flake8_type_checking, mccabe, pandas_vet, pep8_naming, perflint,
|
||||
pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
|
||||
pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, tryceratops,
|
||||
};
|
||||
use crate::settings::types::PythonVersion;
|
||||
|
||||
|
@ -1484,6 +1484,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
if checker.enabled(Rule::AsyncioDanglingTask) {
|
||||
ruff::rules::asyncio_dangling_task(checker, value);
|
||||
}
|
||||
if checker.enabled(Rule::RepeatedAppend) {
|
||||
refurb::rules::repeated_append(checker, stmt);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
|
|
@ -865,6 +865,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8Slots, "001") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInTupleSubclass),
|
||||
(Flake8Slots, "002") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInNamedtupleSubclass),
|
||||
|
||||
// refurb
|
||||
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
|
||||
|
||||
_ => return None,
|
||||
})
|
||||
}
|
||||
|
|
|
@ -196,6 +196,9 @@ pub enum Linter {
|
|||
/// [Perflint](https://pypi.org/project/perflint/)
|
||||
#[prefix = "PERF"]
|
||||
Perflint,
|
||||
/// [refurb](https://pypi.org/project/refurb/)
|
||||
#[prefix = "FURB"]
|
||||
Refurb,
|
||||
/// Ruff-specific rules
|
||||
#[prefix = "RUF"]
|
||||
Ruff,
|
||||
|
|
|
@ -52,5 +52,6 @@ pub mod pyflakes;
|
|||
pub mod pygrep_hooks;
|
||||
pub mod pylint;
|
||||
pub mod pyupgrade;
|
||||
pub mod refurb;
|
||||
pub mod ruff;
|
||||
pub mod tryceratops;
|
||||
|
|
25
crates/ruff/src/rules/refurb/mod.rs
Normal file
25
crates/ruff/src/rules/refurb/mod.rs
Normal file
|
@ -0,0 +1,25 @@
|
|||
//! Rules from [refurb](https://pypi.org/project/refurb/)/
|
||||
pub(crate) mod rules;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
|
||||
use anyhow::Result;
|
||||
use test_case::test_case;
|
||||
|
||||
use crate::registry::Rule;
|
||||
use crate::test::test_path;
|
||||
use crate::{assert_messages, settings};
|
||||
|
||||
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
|
||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||
let diagnostics = test_path(
|
||||
Path::new("refurb").join(path).as_path(),
|
||||
&settings::Settings::for_rule(rule_code),
|
||||
)?;
|
||||
assert_messages!(snapshot, diagnostics);
|
||||
Ok(())
|
||||
}
|
||||
}
|
3
crates/ruff/src/rules/refurb/rules/mod.rs
Normal file
3
crates/ruff/src/rules/refurb/rules/mod.rs
Normal file
|
@ -0,0 +1,3 @@
|
|||
pub(crate) use repeated_append::*;
|
||||
|
||||
mod repeated_append;
|
442
crates/ruff/src/rules/refurb/rules/repeated_append.rs
Normal file
442
crates/ruff/src/rules/refurb/rules/repeated_append.rs
Normal file
|
@ -0,0 +1,442 @@
|
|||
use rustc_hash::FxHashMap;
|
||||
|
||||
use ast::{traversal, ParameterWithDefault, Parameters};
|
||||
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::helpers::map_subscript;
|
||||
use ruff_python_ast::{self as ast, Expr, Stmt};
|
||||
use ruff_python_codegen::Generator;
|
||||
use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
|
||||
use ruff_python_semantic::{Binding, BindingId, BindingKind, DefinitionId, SemanticModel};
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::autofix::snippet::SourceCodeSnippet;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::AsRule;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for consecutive calls to `append`.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Consecutive calls to `append` can be less efficient than batching them into
|
||||
/// a single `extend`. Each `append` resizes the list individually, whereas an
|
||||
/// `extend` can resize the list once for all elements.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// nums = [1, 2, 3]
|
||||
///
|
||||
/// nums.append(4)
|
||||
/// nums.append(5)
|
||||
/// nums.append(6)
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// nums = [1, 2, 3]
|
||||
///
|
||||
/// nums.extend((4, 5, 6))
|
||||
/// ```
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: More on Lists](https://docs.python.org/3/tutorial/datastructures.html#more-on-lists)
|
||||
#[violation]
|
||||
pub struct RepeatedAppend {
|
||||
name: String,
|
||||
replacement: SourceCodeSnippet,
|
||||
}
|
||||
|
||||
impl RepeatedAppend {
|
||||
fn suggestion(&self) -> String {
|
||||
let name = &self.name;
|
||||
self.replacement
|
||||
.full_display()
|
||||
.map_or(format!("{name}.extend(...)"), ToString::to_string)
|
||||
}
|
||||
}
|
||||
|
||||
impl Violation for RepeatedAppend {
|
||||
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
|
||||
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let name = &self.name;
|
||||
let suggestion = self.suggestion();
|
||||
format!("Use `{suggestion}` instead of repeatedly calling `{name}.append()`")
|
||||
}
|
||||
|
||||
fn autofix_title(&self) -> Option<String> {
|
||||
let suggestion = self.suggestion();
|
||||
Some(format!("Replace with `{suggestion}`"))
|
||||
}
|
||||
}
|
||||
|
||||
/// FURB113
|
||||
pub(crate) fn repeated_append(checker: &mut Checker, stmt: &Stmt) {
|
||||
let Some(appends) = match_consecutive_appends(checker.semantic(), stmt) else {
|
||||
return;
|
||||
};
|
||||
|
||||
// No need to proceed if we have less than 1 `append` to work with.
|
||||
if appends.len() <= 1 {
|
||||
return;
|
||||
}
|
||||
|
||||
// group borrows from checker, so we can't directly push into checker.diagnostics
|
||||
let diagnostics: Vec<Diagnostic> = group_appends(appends)
|
||||
.iter()
|
||||
.filter_map(|group| {
|
||||
// Groups with just one element are fine, and shouldn't be replaced by `extend`.
|
||||
if group.appends.len() <= 1 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let replacement = make_suggestion(group, checker.generator());
|
||||
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
RepeatedAppend {
|
||||
name: group.name().to_string(),
|
||||
replacement: SourceCodeSnippet::new(replacement.clone()),
|
||||
},
|
||||
group.range(),
|
||||
);
|
||||
|
||||
// We only suggest a fix when all appends in a group are clumped together. If they're
|
||||
// non-consecutive, fixing them is much more difficult.
|
||||
if checker.patch(diagnostic.kind.rule()) && group.is_consecutive {
|
||||
diagnostic.set_fix(Fix::suggested(Edit::replacement(
|
||||
replacement,
|
||||
group.start(),
|
||||
group.end(),
|
||||
)));
|
||||
}
|
||||
|
||||
Some(diagnostic)
|
||||
})
|
||||
.collect();
|
||||
|
||||
checker.diagnostics.extend(diagnostics);
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct Append<'a> {
|
||||
/// Receiver of the `append` call (aka `self` argument).
|
||||
receiver: &'a ast::ExprName,
|
||||
/// [`BindingId`] that the receiver references.
|
||||
binding_id: BindingId,
|
||||
/// [`Binding`] that the receiver references.
|
||||
binding: &'a Binding<'a>,
|
||||
/// [`Expr`] serving as a sole argument to `append`.
|
||||
argument: &'a Expr,
|
||||
/// The statement containing the `append` call.
|
||||
stmt: &'a Stmt,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct AppendGroup<'a> {
|
||||
/// A sequence of `appends` connected to the same binding.
|
||||
appends: Vec<Append<'a>>,
|
||||
/// `true` when all appends in the group follow one another and don't have other statements in
|
||||
/// between. It is much easier to make fix suggestions for consecutive groups.
|
||||
is_consecutive: bool,
|
||||
}
|
||||
|
||||
impl AppendGroup<'_> {
|
||||
fn name(&self) -> &str {
|
||||
assert!(!self.appends.is_empty());
|
||||
&self.appends.first().unwrap().receiver.id
|
||||
}
|
||||
}
|
||||
|
||||
impl Ranged for AppendGroup<'_> {
|
||||
fn range(&self) -> TextRange {
|
||||
assert!(!self.appends.is_empty());
|
||||
TextRange::new(
|
||||
self.appends.first().unwrap().stmt.start(),
|
||||
self.appends.last().unwrap().stmt.end(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// Match consecutive calls to `append` on list variables starting from the given statement.
|
||||
fn match_consecutive_appends<'a>(
|
||||
semantic: &'a SemanticModel,
|
||||
stmt: &'a Stmt,
|
||||
) -> Option<Vec<Append<'a>>> {
|
||||
// Match the current statement, to see if it's an append.
|
||||
let append = match_append(semantic, stmt)?;
|
||||
|
||||
// In order to match consecutive statements, we need to go to the tree ancestor of the
|
||||
// given statement, find its position there, and match all 'appends' from there.
|
||||
let siblings: &[Stmt] = if semantic.at_top_level() {
|
||||
// If the statement is at the top level, we should go to the parent module.
|
||||
// Module is available in the definitions list.
|
||||
let module = semantic.definitions[DefinitionId::module()].as_module()?;
|
||||
module.python_ast
|
||||
} else {
|
||||
// Otherwise, go to the parent, and take its body as a sequence of siblings.
|
||||
semantic
|
||||
.current_statement_parent()
|
||||
.and_then(|parent| traversal::suite(stmt, parent))?
|
||||
};
|
||||
|
||||
let stmt_index = siblings.iter().position(|sibling| sibling == stmt)?;
|
||||
|
||||
// We shouldn't repeat the same work for many 'appends' that go in a row. Let's check
|
||||
// that this statement is at the beginning of such a group.
|
||||
if stmt_index != 0 && match_append(semantic, &siblings[stmt_index - 1]).is_some() {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Starting from the next statement, let's match all appends and make a vector.
|
||||
Some(
|
||||
std::iter::once(append)
|
||||
.chain(
|
||||
siblings
|
||||
.iter()
|
||||
.skip(stmt_index + 1)
|
||||
.map_while(|sibling| match_append(semantic, sibling)),
|
||||
)
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Group the given appends by the associated bindings.
|
||||
fn group_appends(appends: Vec<Append<'_>>) -> Vec<AppendGroup<'_>> {
|
||||
// We want to go over the given list of appends and group the by receivers.
|
||||
let mut map: FxHashMap<BindingId, AppendGroup> = FxHashMap::default();
|
||||
let mut iter = appends.into_iter();
|
||||
let mut last_binding = {
|
||||
let first_append = iter.next().unwrap();
|
||||
let binding_id = first_append.binding_id;
|
||||
let _ = get_or_add(&mut map, first_append);
|
||||
binding_id
|
||||
};
|
||||
|
||||
for append in iter {
|
||||
let binding_id = append.binding_id;
|
||||
let group = get_or_add(&mut map, append);
|
||||
if binding_id != last_binding {
|
||||
// If the group is not brand new, and the previous group was different,
|
||||
// we should mark it as "non-consecutive".
|
||||
//
|
||||
// We are catching the following situation:
|
||||
// ```python
|
||||
// a.append(1)
|
||||
// a.append(2)
|
||||
// b.append(1)
|
||||
// a.append(3) # <- we are currently here
|
||||
// ```
|
||||
//
|
||||
// So, `a` != `b` and group for `a` already contains appends 1 and 2.
|
||||
// It is only possible if this group got interrupted by at least one
|
||||
// other group and, thus, it is non-consecutive.
|
||||
if group.appends.len() > 1 {
|
||||
group.is_consecutive = false;
|
||||
}
|
||||
|
||||
last_binding = binding_id;
|
||||
}
|
||||
}
|
||||
|
||||
map.into_values().collect()
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn get_or_add<'a, 'b>(
|
||||
map: &'b mut FxHashMap<BindingId, AppendGroup<'a>>,
|
||||
append: Append<'a>,
|
||||
) -> &'b mut AppendGroup<'a> {
|
||||
let group = map.entry(append.binding_id).or_insert(AppendGroup {
|
||||
appends: vec![],
|
||||
is_consecutive: true,
|
||||
});
|
||||
group.appends.push(append);
|
||||
group
|
||||
}
|
||||
|
||||
/// Make fix suggestion for the given group of appends.
|
||||
fn make_suggestion(group: &AppendGroup, generator: Generator) -> String {
|
||||
let appends = &group.appends;
|
||||
|
||||
assert!(!appends.is_empty());
|
||||
let first = appends.first().unwrap();
|
||||
|
||||
assert!(appends
|
||||
.iter()
|
||||
.all(|append| append.binding.source == first.binding.source));
|
||||
|
||||
// Here we construct `var.extend((elt1, elt2, ..., eltN))
|
||||
//
|
||||
// Each eltK comes from an individual `var.append(eltK)`.
|
||||
let elts: Vec<Expr> = appends
|
||||
.iter()
|
||||
.map(|append| append.argument.clone())
|
||||
.collect();
|
||||
// Join all elements into a tuple: `(elt1, elt2, ..., eltN)`
|
||||
let tuple = ast::ExprTuple {
|
||||
elts,
|
||||
ctx: ast::ExprContext::Load,
|
||||
range: TextRange::default(),
|
||||
};
|
||||
// Make `var.extend`.
|
||||
// NOTE: receiver is the same for all appends and that's why we can take the first.
|
||||
let attr = ast::ExprAttribute {
|
||||
value: Box::new(first.receiver.clone().into()),
|
||||
attr: ast::Identifier::new("extend".to_string(), TextRange::default()),
|
||||
ctx: ast::ExprContext::Load,
|
||||
range: TextRange::default(),
|
||||
};
|
||||
// Make the actual call `var.extend((elt1, elt2, ..., eltN))`
|
||||
let call = ast::ExprCall {
|
||||
func: Box::new(attr.into()),
|
||||
arguments: ast::Arguments {
|
||||
args: vec![tuple.into()],
|
||||
keywords: vec![],
|
||||
range: TextRange::default(),
|
||||
},
|
||||
range: TextRange::default(),
|
||||
};
|
||||
// And finally, turn it into a statement.
|
||||
let stmt = ast::StmtExpr {
|
||||
value: Box::new(call.into()),
|
||||
range: TextRange::default(),
|
||||
};
|
||||
generator.stmt(&stmt.into())
|
||||
}
|
||||
|
||||
/// Matches that the given statement is a call to `append` on a list variable.
|
||||
fn match_append<'a>(semantic: &'a SemanticModel, stmt: &'a Stmt) -> Option<Append<'a>> {
|
||||
let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let Expr::Call(ast::ExprCall {
|
||||
func, arguments, ..
|
||||
}) = value.as_ref()
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
|
||||
// `append` should have just one argument, an element to be added.
|
||||
let [argument] = arguments.args.as_slice() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
// The called function should be an attribute, ie `value.attr`.
|
||||
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
// `attr` should be `append` and it shouldn't have any keyword arguments.
|
||||
if attr != "append" || !arguments.keywords.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
// We match only variable references, i.e. `value` should be a name expression.
|
||||
let Expr::Name(receiver @ ast::ExprName { id: name, .. }) = value.as_ref() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
// Now we need to find what is this variable bound to...
|
||||
let scope = semantic.current_scope();
|
||||
let bindings: Vec<BindingId> = scope.get_all(name).collect();
|
||||
|
||||
// Maybe it is too strict of a limitation, but it seems reasonable.
|
||||
let [binding_id] = bindings.as_slice() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let binding = semantic.binding(*binding_id);
|
||||
|
||||
// ...and whether this something is a list.
|
||||
if binding.source.is_none() || !is_list(semantic, binding, name) {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(Append {
|
||||
receiver,
|
||||
binding_id: *binding_id,
|
||||
binding,
|
||||
stmt,
|
||||
argument,
|
||||
})
|
||||
}
|
||||
|
||||
/// Test whether the given binding (and the given name) can be considered a list.
|
||||
/// For this, we check what value might be associated with it through it's initialization and
|
||||
/// what annotation it has (we consider `list` and `typing.List`).
|
||||
///
|
||||
/// NOTE: this function doesn't perform more serious type inference, so it won't be able
|
||||
/// to understand if the value gets initialized from a call to a function always returning
|
||||
/// lists. This also implies no interfile analysis.
|
||||
fn is_list<'a>(semantic: &'a SemanticModel, binding: &'a Binding, name: &str) -> bool {
|
||||
match binding.kind {
|
||||
BindingKind::Assignment => match binding.statement(semantic) {
|
||||
Some(Stmt::Assign(ast::StmtAssign { value, .. })) => {
|
||||
let value_type: ResolvedPythonType = value.as_ref().into();
|
||||
let ResolvedPythonType::Atom(candidate) = value_type else {
|
||||
return false;
|
||||
};
|
||||
matches!(candidate, PythonType::List)
|
||||
}
|
||||
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
|
||||
is_list_annotation(semantic, annotation.as_ref())
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
BindingKind::Argument => match binding.statement(semantic) {
|
||||
Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) => {
|
||||
let Some(parameter) = find_parameter_by_name(parameters.as_ref(), name) else {
|
||||
return false;
|
||||
};
|
||||
let Some(ref annotation) = parameter.parameter.annotation else {
|
||||
return false;
|
||||
};
|
||||
is_list_annotation(semantic, annotation.as_ref())
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
BindingKind::Annotation => match binding.statement(semantic) {
|
||||
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
|
||||
is_list_annotation(semantic, annotation.as_ref())
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn is_list_annotation(semantic: &SemanticModel, annotation: &Expr) -> bool {
|
||||
let value = map_subscript(annotation);
|
||||
match_builtin_list_type(semantic, value) || semantic.match_typing_expr(value, "List")
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn match_builtin_list_type(semantic: &SemanticModel, type_expr: &Expr) -> bool {
|
||||
let Expr::Name(ast::ExprName { id, .. }) = type_expr else {
|
||||
return false;
|
||||
};
|
||||
id == "list" && semantic.is_builtin("list")
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn find_parameter_by_name<'a>(
|
||||
parameters: &'a Parameters,
|
||||
name: &'a str,
|
||||
) -> Option<&'a ParameterWithDefault> {
|
||||
find_parameter_by_name_impl(¶meters.args, name)
|
||||
.or_else(|| find_parameter_by_name_impl(¶meters.posonlyargs, name))
|
||||
.or_else(|| find_parameter_by_name_impl(¶meters.kwonlyargs, name))
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn find_parameter_by_name_impl<'a>(
|
||||
parameters: &'a [ParameterWithDefault],
|
||||
name: &'a str,
|
||||
) -> Option<&'a ParameterWithDefault> {
|
||||
parameters
|
||||
.iter()
|
||||
.find(|arg| arg.parameter.name.as_str() == name)
|
||||
}
|
|
@ -0,0 +1,335 @@
|
|||
---
|
||||
source: crates/ruff/src/rules/refurb/mod.rs
|
||||
---
|
||||
FURB113.py:23:1: FURB113 [*] Use `nums.extend((1, 2))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
22 | # FURB113
|
||||
23 | / nums.append(1)
|
||||
24 | | nums.append(2)
|
||||
| |______________^ FURB113
|
||||
25 | pass
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
20 20 |
|
||||
21 21 |
|
||||
22 22 | # FURB113
|
||||
23 |-nums.append(1)
|
||||
24 |-nums.append(2)
|
||||
23 |+nums.extend((1, 2))
|
||||
25 24 | pass
|
||||
26 25 |
|
||||
27 26 |
|
||||
|
||||
FURB113.py:29:1: FURB113 [*] Use `nums3.extend((1, 2))` instead of repeatedly calling `nums3.append()`
|
||||
|
|
||||
28 | # FURB113
|
||||
29 | / nums3.append(1)
|
||||
30 | | nums3.append(2)
|
||||
| |_______________^ FURB113
|
||||
31 | pass
|
||||
|
|
||||
= help: Replace with `nums3.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
26 26 |
|
||||
27 27 |
|
||||
28 28 | # FURB113
|
||||
29 |-nums3.append(1)
|
||||
30 |-nums3.append(2)
|
||||
29 |+nums3.extend((1, 2))
|
||||
31 30 | pass
|
||||
32 31 |
|
||||
33 32 |
|
||||
|
||||
FURB113.py:35:1: FURB113 [*] Use `nums4.extend((1, 2))` instead of repeatedly calling `nums4.append()`
|
||||
|
|
||||
34 | # FURB113
|
||||
35 | / nums4.append(1)
|
||||
36 | | nums4.append(2)
|
||||
| |_______________^ FURB113
|
||||
37 | pass
|
||||
|
|
||||
= help: Replace with `nums4.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
32 32 |
|
||||
33 33 |
|
||||
34 34 | # FURB113
|
||||
35 |-nums4.append(1)
|
||||
36 |-nums4.append(2)
|
||||
35 |+nums4.extend((1, 2))
|
||||
37 36 | pass
|
||||
38 37 |
|
||||
39 38 |
|
||||
|
||||
FURB113.py:41:1: FURB113 Use `nums.extend((1, 2, 3))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
40 | # FURB113
|
||||
41 | / nums.append(1)
|
||||
42 | | nums2.append(1)
|
||||
43 | | nums.append(2)
|
||||
44 | | nums.append(3)
|
||||
| |______________^ FURB113
|
||||
45 | pass
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2, 3))`
|
||||
|
||||
FURB113.py:49:1: FURB113 Use `nums.extend((1, 2, 3))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
48 | # FURB113
|
||||
49 | / nums.append(1)
|
||||
50 | | nums2.append(1)
|
||||
51 | | nums.append(2)
|
||||
52 | | # FURB113
|
||||
53 | | nums3.append(1)
|
||||
54 | | nums.append(3)
|
||||
| |______________^ FURB113
|
||||
55 | # FURB113
|
||||
56 | nums4.append(1)
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2, 3))`
|
||||
|
||||
FURB113.py:53:1: FURB113 Use `nums3.extend((1, 2))` instead of repeatedly calling `nums3.append()`
|
||||
|
|
||||
51 | nums.append(2)
|
||||
52 | # FURB113
|
||||
53 | / nums3.append(1)
|
||||
54 | | nums.append(3)
|
||||
55 | | # FURB113
|
||||
56 | | nums4.append(1)
|
||||
57 | | nums4.append(2)
|
||||
58 | | nums3.append(2)
|
||||
| |_______________^ FURB113
|
||||
59 | pass
|
||||
|
|
||||
= help: Replace with `nums3.extend((1, 2))`
|
||||
|
||||
FURB113.py:56:1: FURB113 [*] Use `nums4.extend((1, 2))` instead of repeatedly calling `nums4.append()`
|
||||
|
|
||||
54 | nums.append(3)
|
||||
55 | # FURB113
|
||||
56 | / nums4.append(1)
|
||||
57 | | nums4.append(2)
|
||||
| |_______________^ FURB113
|
||||
58 | nums3.append(2)
|
||||
59 | pass
|
||||
|
|
||||
= help: Replace with `nums4.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
53 53 | nums3.append(1)
|
||||
54 54 | nums.append(3)
|
||||
55 55 | # FURB113
|
||||
56 |-nums4.append(1)
|
||||
57 |-nums4.append(2)
|
||||
56 |+nums4.extend((1, 2))
|
||||
58 57 | nums3.append(2)
|
||||
59 58 | pass
|
||||
60 59 |
|
||||
|
||||
FURB113.py:62:1: FURB113 [*] Use `nums.extend((1, 2, 3))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
61 | # FURB113
|
||||
62 | / nums.append(1)
|
||||
63 | | nums.append(2)
|
||||
64 | | nums.append(3)
|
||||
| |______________^ FURB113
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2, 3))`
|
||||
|
||||
ℹ Suggested fix
|
||||
59 59 | pass
|
||||
60 60 |
|
||||
61 61 | # FURB113
|
||||
62 |-nums.append(1)
|
||||
63 |-nums.append(2)
|
||||
64 |-nums.append(3)
|
||||
62 |+nums.extend((1, 2, 3))
|
||||
65 63 |
|
||||
66 64 |
|
||||
67 65 | if True:
|
||||
|
||||
FURB113.py:69:5: FURB113 [*] Use `nums.extend((1, 2))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
67 | if True:
|
||||
68 | # FURB113
|
||||
69 | nums.append(1)
|
||||
| _____^
|
||||
70 | | nums.append(2)
|
||||
| |__________________^ FURB113
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
66 66 |
|
||||
67 67 | if True:
|
||||
68 68 | # FURB113
|
||||
69 |- nums.append(1)
|
||||
70 |- nums.append(2)
|
||||
69 |+ nums.extend((1, 2))
|
||||
71 70 |
|
||||
72 71 |
|
||||
73 72 | if True:
|
||||
|
||||
FURB113.py:75:5: FURB113 [*] Use `nums.extend((1, 2))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
73 | if True:
|
||||
74 | # FURB113
|
||||
75 | nums.append(1)
|
||||
| _____^
|
||||
76 | | nums.append(2)
|
||||
| |__________________^ FURB113
|
||||
77 | pass
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
72 72 |
|
||||
73 73 | if True:
|
||||
74 74 | # FURB113
|
||||
75 |- nums.append(1)
|
||||
76 |- nums.append(2)
|
||||
75 |+ nums.extend((1, 2))
|
||||
77 76 | pass
|
||||
78 77 |
|
||||
79 78 |
|
||||
|
||||
FURB113.py:82:5: FURB113 Use `nums.extend((1, 2, 3))` instead of repeatedly calling `nums.append()`
|
||||
|
|
||||
80 | if True:
|
||||
81 | # FURB113
|
||||
82 | nums.append(1)
|
||||
| _____^
|
||||
83 | | nums2.append(1)
|
||||
84 | | nums.append(2)
|
||||
85 | | nums.append(3)
|
||||
| |__________________^ FURB113
|
||||
|
|
||||
= help: Replace with `nums.extend((1, 2, 3))`
|
||||
|
||||
FURB113.py:90:5: FURB113 [*] Use `x.extend((1, 2))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
88 | def yes_one(x: list[int]):
|
||||
89 | # FURB113
|
||||
90 | x.append(1)
|
||||
| _____^
|
||||
91 | | x.append(2)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
87 87 |
|
||||
88 88 | def yes_one(x: list[int]):
|
||||
89 89 | # FURB113
|
||||
90 |- x.append(1)
|
||||
91 |- x.append(2)
|
||||
90 |+ x.extend((1, 2))
|
||||
92 91 |
|
||||
93 92 |
|
||||
94 93 | def yes_two(x: List[int]):
|
||||
|
||||
FURB113.py:96:5: FURB113 [*] Use `x.extend((1, 2))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
94 | def yes_two(x: List[int]):
|
||||
95 | # FURB113
|
||||
96 | x.append(1)
|
||||
| _____^
|
||||
97 | | x.append(2)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
93 93 |
|
||||
94 94 | def yes_two(x: List[int]):
|
||||
95 95 | # FURB113
|
||||
96 |- x.append(1)
|
||||
97 |- x.append(2)
|
||||
96 |+ x.extend((1, 2))
|
||||
98 97 |
|
||||
99 98 |
|
||||
100 99 | def yes_three(*, x: list[int]):
|
||||
|
||||
FURB113.py:102:5: FURB113 [*] Use `x.extend((1, 2))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
100 | def yes_three(*, x: list[int]):
|
||||
101 | # FURB113
|
||||
102 | x.append(1)
|
||||
| _____^
|
||||
103 | | x.append(2)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
99 99 |
|
||||
100 100 | def yes_three(*, x: list[int]):
|
||||
101 101 | # FURB113
|
||||
102 |- x.append(1)
|
||||
103 |- x.append(2)
|
||||
102 |+ x.extend((1, 2))
|
||||
104 103 |
|
||||
105 104 |
|
||||
106 105 | def yes_four(x: list[int], /):
|
||||
|
||||
FURB113.py:108:5: FURB113 [*] Use `x.extend((1, 2))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
106 | def yes_four(x: list[int], /):
|
||||
107 | # FURB113
|
||||
108 | x.append(1)
|
||||
| _____^
|
||||
109 | | x.append(2)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
105 105 |
|
||||
106 106 | def yes_four(x: list[int], /):
|
||||
107 107 | # FURB113
|
||||
108 |- x.append(1)
|
||||
109 |- x.append(2)
|
||||
108 |+ x.extend((1, 2))
|
||||
110 109 |
|
||||
111 110 |
|
||||
112 111 | def yes_five(x: list[int], y: list[int]):
|
||||
|
||||
FURB113.py:114:5: FURB113 Use `x.extend((1, 2, 3))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
112 | def yes_five(x: list[int], y: list[int]):
|
||||
113 | # FURB113
|
||||
114 | x.append(1)
|
||||
| _____^
|
||||
115 | | x.append(2)
|
||||
116 | | y.append(1)
|
||||
117 | | x.append(3)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2, 3))`
|
||||
|
||||
FURB113.py:122:5: FURB113 [*] Use `x.extend((1, 2))` instead of repeatedly calling `x.append()`
|
||||
|
|
||||
120 | def yes_six(x: list):
|
||||
121 | # FURB113
|
||||
122 | x.append(1)
|
||||
| _____^
|
||||
123 | | x.append(2)
|
||||
| |_______________^ FURB113
|
||||
|
|
||||
= help: Replace with `x.extend((1, 2))`
|
||||
|
||||
ℹ Suggested fix
|
||||
119 119 |
|
||||
120 120 | def yes_six(x: list):
|
||||
121 121 | # FURB113
|
||||
122 |- x.append(1)
|
||||
123 |- x.append(2)
|
||||
122 |+ x.extend((1, 2))
|
||||
124 123 |
|
||||
125 124 |
|
||||
126 125 | # Non-errors.
|
||||
|
||||
|
|
@ -3,6 +3,7 @@ use crate::{self as ast, ExceptHandler, Stmt, Suite};
|
|||
|
||||
/// Given a [`Stmt`] and its parent, return the [`Suite`] that contains the [`Stmt`].
|
||||
pub fn suite<'a>(stmt: &'a Stmt, parent: &'a Stmt) -> Option<&'a Suite> {
|
||||
// TODO: refactor this to work without a parent, ie when `stmt` is at the top level
|
||||
match parent {
|
||||
Stmt::FunctionDef(ast::StmtFunctionDef { body, .. }) => Some(body),
|
||||
Stmt::ClassDef(ast::StmtClassDef { body, .. }) => Some(body),
|
||||
|
|
|
@ -5,6 +5,7 @@ use bitflags::bitflags;
|
|||
|
||||
use ruff_index::{newtype_index, IndexSlice, IndexVec};
|
||||
use ruff_python_ast::call_path::format_call_path;
|
||||
use ruff_python_ast::Stmt;
|
||||
use ruff_source_file::Locator;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
|
@ -181,17 +182,21 @@ impl<'a> Binding<'a> {
|
|||
locator.slice(self.range)
|
||||
}
|
||||
|
||||
/// Returns the statement in which the binding was defined.
|
||||
pub fn statement<'b>(&self, semantic: &'b SemanticModel) -> Option<&'b Stmt> {
|
||||
self.source
|
||||
.map(|statement_id| semantic.statement(statement_id))
|
||||
}
|
||||
|
||||
/// Returns the range of the binding's parent.
|
||||
pub fn parent_range(&self, semantic: &SemanticModel) -> Option<TextRange> {
|
||||
self.source
|
||||
.map(|id| semantic.statement(id))
|
||||
.and_then(|parent| {
|
||||
if parent.is_import_from_stmt() {
|
||||
Some(parent.range())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
self.statement(semantic).and_then(|parent| {
|
||||
if parent.is_import_from_stmt() {
|
||||
Some(parent.range())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub fn as_any_import(&'a self) -> Option<AnyImport<'a>> {
|
||||
|
|
|
@ -117,7 +117,7 @@ impl Ranged for Member<'_> {
|
|||
}
|
||||
|
||||
/// A definition within a Python program.
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, is_macro::Is)]
|
||||
pub enum Definition<'a> {
|
||||
Module(Module<'a>),
|
||||
Member(Member<'a>),
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue