mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 02:38:25 +00:00
[perflint] Implement fix for manual-dict-comprehension (PERF403) (#16719)
## Summary This change adds an auto-fix for manual dict comprehensions. It also copies many of the improvements from #13919 (and associated PRs fixing issues with it), and moves some of the utility functions from `manual_list_comprehension.rs` into a separate `helpers.rs` to be used in both. ## Test Plan I added a preview test case to showcase the new fix and added a test case in `PERF403.py` to make sure lines with semicolons function. I didn't yet make similar tests to the ones I added earlier to `PERF401.py`, but the logic is the same, so it might be good to add those to make sure they work.
This commit is contained in:
w0nder1ng
GitHub
parent
5fec1039ed
commit
08221454f6
10 changed files with 908 additions and 123 deletions
|
|
@ -18,7 +18,9 @@ def foo():
|
|||
result = {}
|
||||
for idx, name in enumerate(fruit):
|
||||
if idx % 2:
|
||||
result[idx] = name # Ok (false negative: edge case where `else` is same as `if`)
|
||||
result[idx] = (
|
||||
name # Ok (false negative: edge case where `else` is same as `if`)
|
||||
)
|
||||
else:
|
||||
result[idx] = name
|
||||
|
||||
|
|
@ -85,7 +87,67 @@ def foo():
|
|||
|
||||
def foo():
|
||||
from builtins import dict as SneakyDict
|
||||
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
result = SneakyDict()
|
||||
for idx, name in enumerate(fruit):
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
result: dict[str, int] = {
|
||||
# comment 1
|
||||
}
|
||||
for idx, name in enumerate(
|
||||
fruit # comment 2
|
||||
):
|
||||
# comment 3
|
||||
result[
|
||||
name # comment 4
|
||||
] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
a = 1; result = {}; b = 2
|
||||
for idx, name in enumerate(fruit):
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
result = {"kiwi": 3}
|
||||
for idx, name in enumerate(fruit):
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
(_, result) = (None, {"kiwi": 3})
|
||||
for idx, name in enumerate(fruit):
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
result = {}
|
||||
print(len(result))
|
||||
for idx, name in enumerate(fruit):
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
result = {}
|
||||
for idx, name in enumerate(fruit):
|
||||
if last_idx := idx % 3:
|
||||
result[name] = idx # PERF403
|
||||
|
||||
|
||||
def foo():
|
||||
fruit = ["apple", "pear", "orange"]
|
||||
indices = [0, 1, 2]
|
||||
result = {}
|
||||
for idx, name in indices, fruit:
|
||||
result[name] = idx # PERF403
|
||||
|
|
|
|||
|
|
@ -35,6 +35,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
|
|||
if checker.enabled(Rule::DictIndexMissingItems) {
|
||||
pylint::rules::dict_index_missing_items(checker, stmt_for);
|
||||
}
|
||||
if checker.enabled(Rule::ManualDictComprehension) {
|
||||
perflint::rules::manual_dict_comprehension(checker, stmt_for);
|
||||
}
|
||||
if checker.enabled(Rule::ManualListComprehension) {
|
||||
perflint::rules::manual_list_comprehension(checker, stmt_for);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1319,6 +1319,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
Rule::UnnecessaryEnumerate,
|
||||
Rule::UnusedLoopControlVariable,
|
||||
Rule::YieldInForLoop,
|
||||
Rule::ManualDictComprehension,
|
||||
Rule::ManualListComprehension,
|
||||
]) {
|
||||
checker.analyze.for_loops.push(checker.semantic.snapshot());
|
||||
|
|
@ -1347,9 +1348,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
if checker.enabled(Rule::ManualListCopy) {
|
||||
perflint::rules::manual_list_copy(checker, for_stmt);
|
||||
}
|
||||
if checker.enabled(Rule::ManualDictComprehension) {
|
||||
perflint::rules::manual_dict_comprehension(checker, target, body);
|
||||
}
|
||||
|
||||
if checker.enabled(Rule::ModifiedIteratingSet) {
|
||||
pylint::rules::modified_iterating_set(checker, for_stmt);
|
||||
}
|
||||
|
|
|
|||
98
crates/ruff_linter/src/rules/perflint/helpers.rs
Normal file
98
crates/ruff_linter/src/rules/perflint/helpers.rs
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
use ruff_python_trivia::{
|
||||
BackwardsTokenizer, PythonWhitespace, SimpleToken, SimpleTokenKind, SimpleTokenizer,
|
||||
};
|
||||
use ruff_source_file::LineRanges;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
||||
pub(super) fn comment_strings_in_range<'a>(
|
||||
checker: &'a Checker,
|
||||
range: TextRange,
|
||||
ranges_to_ignore: &[TextRange],
|
||||
) -> Vec<&'a str> {
|
||||
checker
|
||||
.comment_ranges()
|
||||
.comments_in_range(range)
|
||||
.iter()
|
||||
// Ignore comments inside of the append or iterator, since these are preserved
|
||||
.filter(|comment| {
|
||||
!ranges_to_ignore
|
||||
.iter()
|
||||
.any(|to_ignore| to_ignore.contains_range(**comment))
|
||||
})
|
||||
.map(|range| checker.locator().slice(range).trim_whitespace_start())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn semicolon_before_and_after(
|
||||
checker: &Checker,
|
||||
statement: TextRange,
|
||||
) -> (Option<SimpleToken>, Option<SimpleToken>) {
|
||||
// determine whether there's a semicolon either before or after the binding statement.
|
||||
// Since it's a binding statement, we can just check whether there's a semicolon immediately
|
||||
// after the whitespace in front of or behind it
|
||||
let mut after_tokenizer =
|
||||
SimpleTokenizer::starts_at(statement.end(), checker.locator().contents()).skip_trivia();
|
||||
|
||||
let after_semicolon = if after_tokenizer
|
||||
.next()
|
||||
.is_some_and(|token| token.kind() == SimpleTokenKind::Semi)
|
||||
{
|
||||
after_tokenizer.next()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let semicolon_before = BackwardsTokenizer::up_to(
|
||||
statement.start(),
|
||||
checker.locator().contents(),
|
||||
checker.comment_ranges(),
|
||||
)
|
||||
.skip_trivia()
|
||||
.next()
|
||||
.filter(|token| token.kind() == SimpleTokenKind::Semi);
|
||||
|
||||
(semicolon_before, after_semicolon)
|
||||
}
|
||||
|
||||
/// Finds the range necessary to delete a statement (including any semicolons around it).
|
||||
/// Returns the range and whether there were multiple statements on the line
|
||||
pub(super) fn statement_deletion_range(
|
||||
checker: &Checker,
|
||||
statement_range: TextRange,
|
||||
) -> (TextRange, bool) {
|
||||
let locator = checker.locator();
|
||||
// If the binding has multiple statements on its line, the fix would be substantially more complicated
|
||||
let (semicolon_before, after_semicolon) = semicolon_before_and_after(checker, statement_range);
|
||||
|
||||
// If there are multiple binding statements in one line, we don't want to accidentally delete them
|
||||
// Instead, we just delete the binding statement and leave any comments where they are
|
||||
|
||||
match (semicolon_before, after_semicolon) {
|
||||
// ```python
|
||||
// a = []
|
||||
// ```
|
||||
(None, None) => (locator.full_lines_range(statement_range), false),
|
||||
|
||||
// ```python
|
||||
// a = 1; b = []
|
||||
// ^^^^^^^^
|
||||
// a = 1; b = []; c = 3
|
||||
// ^^^^^^^^
|
||||
// ```
|
||||
(Some(semicolon_before), Some(_) | None) => (
|
||||
TextRange::new(semicolon_before.start(), statement_range.end()),
|
||||
true,
|
||||
),
|
||||
|
||||
// ```python
|
||||
// a = []; b = 3
|
||||
// ^^^^^^^
|
||||
// ```
|
||||
(None, Some(after_semicolon)) => (
|
||||
TextRange::new(statement_range.start(), after_semicolon.start()),
|
||||
true,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
//! Rules from [perflint](https://pypi.org/project/perflint/).
|
||||
mod helpers;
|
||||
pub(crate) mod rules;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::Path;
|
||||
|
|
@ -31,7 +31,8 @@ mod tests {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
// TODO: remove this test case when the fix for `perf401` is stabilized
|
||||
// TODO: remove this test case when the fixes for `perf401` and `perf403` are stabilized
|
||||
#[test_case(Rule::ManualDictComprehension, Path::new("PERF403.py"))]
|
||||
#[test_case(Rule::ManualListComprehension, Path::new("PERF401.py"))]
|
||||
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||
let snapshot = format!(
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
|
||||
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||
use ruff_python_ast::comparable::ComparableExpr;
|
||||
use ruff_python_ast::helpers::any_over_expr;
|
||||
use ruff_python_ast::{self as ast, Expr, Stmt};
|
||||
use ruff_python_semantic::analyze::typing::is_dict;
|
||||
use ruff_python_ast::{
|
||||
self as ast, comparable::ComparableExpr, helpers::any_over_expr, Expr, Stmt,
|
||||
};
|
||||
use ruff_python_semantic::{analyze::typing::is_dict, Binding};
|
||||
use ruff_source_file::LineRanges;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::rules::perflint::helpers::{comment_strings_in_range, statement_deletion_range};
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for `for` loops that can be replaced by a dictionary comprehension.
|
||||
|
|
@ -42,17 +45,45 @@ use crate::checkers::ast::Checker;
|
|||
/// result.update({x: y for x, y in pairs if y % 2})
|
||||
/// ```
|
||||
#[derive(ViolationMetadata)]
|
||||
pub(crate) struct ManualDictComprehension;
|
||||
pub(crate) struct ManualDictComprehension {
|
||||
fix_type: DictComprehensionType,
|
||||
is_async: bool,
|
||||
}
|
||||
|
||||
impl Violation for ManualDictComprehension {
|
||||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
|
||||
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"Use a dictionary comprehension instead of a for-loop".to_string()
|
||||
let modifier = if self.is_async { "an async" } else { "a" };
|
||||
|
||||
match self.fix_type {
|
||||
DictComprehensionType::Comprehension => {
|
||||
format!("Use a dictionary comprehension instead of {modifier} for-loop")
|
||||
}
|
||||
DictComprehensionType::Update => {
|
||||
format!("Use `dict.update` instead of {modifier} for-loop")
|
||||
}
|
||||
}
|
||||
}
|
||||
fn fix_title(&self) -> Option<String> {
|
||||
let modifier = if self.is_async { "async " } else { "" };
|
||||
match self.fix_type {
|
||||
DictComprehensionType::Comprehension => Some(format!(
|
||||
"Replace {modifier}for loop with dict comprehension"
|
||||
)),
|
||||
DictComprehensionType::Update => {
|
||||
Some(format!("Replace {modifier}for loop with `dict.update`"))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// PERF403
|
||||
pub(crate) fn manual_dict_comprehension(checker: &Checker, target: &Expr, body: &[Stmt]) {
|
||||
pub(crate) fn manual_dict_comprehension(checker: &Checker, for_stmt: &ast::StmtFor) {
|
||||
let ast::StmtFor { body, target, .. } = for_stmt;
|
||||
let body = body.as_slice();
|
||||
let target = target.as_ref();
|
||||
let (stmt, if_test) = match body {
|
||||
// ```python
|
||||
// for idx, name in enumerate(names):
|
||||
|
|
@ -94,18 +125,42 @@ pub(crate) fn manual_dict_comprehension(checker: &Checker, target: &Expr, body:
|
|||
|
||||
let [Expr::Subscript(ast::ExprSubscript {
|
||||
value: subscript_value,
|
||||
slice,
|
||||
slice: key,
|
||||
..
|
||||
})] = targets.as_slice()
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
// If any references to a target variable are after the loop,
|
||||
// then removing the loop would cause a NameError
|
||||
let any_references_after_for_loop = |target: &Expr| {
|
||||
let target_binding = checker
|
||||
.semantic()
|
||||
.bindings
|
||||
.iter()
|
||||
.find(|binding| target.range() == binding.range);
|
||||
debug_assert!(
|
||||
target_binding.is_some(),
|
||||
"for-loop target binding must exist"
|
||||
);
|
||||
|
||||
let Some(target_binding) = target_binding else {
|
||||
// All uses of this function will early-return if this returns true, so this must early-return the rule
|
||||
return true;
|
||||
};
|
||||
|
||||
target_binding
|
||||
.references()
|
||||
.map(|reference| checker.semantic().reference(reference))
|
||||
.any(|other_reference| other_reference.start() > for_stmt.end())
|
||||
};
|
||||
|
||||
match target {
|
||||
Expr::Tuple(tuple) => {
|
||||
if !tuple
|
||||
.iter()
|
||||
.any(|element| ComparableExpr::from(slice) == ComparableExpr::from(element))
|
||||
.any(|element| ComparableExpr::from(key) == ComparableExpr::from(element))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
|
@ -115,14 +170,24 @@ pub(crate) fn manual_dict_comprehension(checker: &Checker, target: &Expr, body:
|
|||
{
|
||||
return;
|
||||
}
|
||||
// Make sure none of the variables are used outside the for loop
|
||||
if tuple.iter().any(any_references_after_for_loop) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
Expr::Name(_) => {
|
||||
if ComparableExpr::from(slice) != ComparableExpr::from(target) {
|
||||
if ComparableExpr::from(key) != ComparableExpr::from(target) {
|
||||
return;
|
||||
}
|
||||
if ComparableExpr::from(value) != ComparableExpr::from(target) {
|
||||
return;
|
||||
}
|
||||
|
||||
// We know that `target` contains an ExprName, but closures can't take `&impl Ranged`,
|
||||
// so we pass `target` itself instead of the inner ExprName
|
||||
if any_references_after_for_loop(target) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
_ => return,
|
||||
}
|
||||
|
|
@ -164,5 +229,242 @@ pub(crate) fn manual_dict_comprehension(checker: &Checker, target: &Expr, body:
|
|||
return;
|
||||
}
|
||||
|
||||
checker.report_diagnostic(Diagnostic::new(ManualDictComprehension, *range));
|
||||
if checker.settings.preview.is_enabled() {
|
||||
let binding_stmt = binding.statement(checker.semantic());
|
||||
let binding_value = binding_stmt.and_then(|binding_stmt| match binding_stmt {
|
||||
ast::Stmt::AnnAssign(assign) => assign.value.as_deref(),
|
||||
ast::Stmt::Assign(assign) => Some(&assign.value),
|
||||
_ => None,
|
||||
});
|
||||
|
||||
// If the variable is an empty dict literal, then we might be able to replace it with a full dict comprehension.
|
||||
// otherwise, it has to be replaced with a `dict.update`
|
||||
let binding_is_empty_dict =
|
||||
binding_value.is_some_and(|binding_value| match binding_value {
|
||||
// value = {}
|
||||
Expr::Dict(dict_expr) => dict_expr.is_empty(),
|
||||
// value = dict()
|
||||
Expr::Call(call) => {
|
||||
checker
|
||||
.semantic()
|
||||
.resolve_builtin_symbol(&call.func)
|
||||
.is_some_and(|name| name == "dict")
|
||||
&& call.arguments.is_empty()
|
||||
}
|
||||
_ => false,
|
||||
});
|
||||
|
||||
let assignment_in_same_statement = binding.source.is_some_and(|binding_source| {
|
||||
let for_loop_parent = checker.semantic().current_statement_parent_id();
|
||||
let binding_parent = checker.semantic().parent_statement_id(binding_source);
|
||||
for_loop_parent == binding_parent
|
||||
});
|
||||
// If the binding is not a single name expression, it could be replaced with a dict comprehension,
|
||||
// but not necessarily, so this needs to be manually fixed. This does not apply when using an update.
|
||||
let binding_has_one_target = binding_stmt.is_some_and(|binding_stmt| match binding_stmt {
|
||||
ast::Stmt::AnnAssign(_) => true,
|
||||
ast::Stmt::Assign(assign) => assign.targets.len() == 1,
|
||||
_ => false,
|
||||
});
|
||||
// If the binding gets used in between the assignment and the for loop, a comprehension is no longer safe
|
||||
|
||||
// If the binding is after the for loop, then it can't be fixed, and this check would panic,
|
||||
// so we check that they are in the same statement first
|
||||
let binding_unused_between = assignment_in_same_statement
|
||||
&& binding_stmt.is_some_and(|binding_stmt| {
|
||||
let from_assign_to_loop = TextRange::new(binding_stmt.end(), for_stmt.start());
|
||||
// Test if there's any reference to the result dictionary between its definition and the for loop.
|
||||
// If there's at least one, then it's been accessed in the middle somewhere, so it's not safe to change into a comprehension
|
||||
!binding
|
||||
.references()
|
||||
.map(|ref_id| checker.semantic().reference(ref_id).range())
|
||||
.any(|text_range| from_assign_to_loop.contains_range(text_range))
|
||||
});
|
||||
// A dict update works in every context, while a dict comprehension only works when all the criteria are true
|
||||
let fix_type = if binding_is_empty_dict
|
||||
&& assignment_in_same_statement
|
||||
&& binding_has_one_target
|
||||
&& binding_unused_between
|
||||
{
|
||||
DictComprehensionType::Comprehension
|
||||
} else {
|
||||
DictComprehensionType::Update
|
||||
};
|
||||
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
ManualDictComprehension {
|
||||
fix_type,
|
||||
is_async: for_stmt.is_async,
|
||||
},
|
||||
*range,
|
||||
);
|
||||
diagnostic.try_set_optional_fix(|| {
|
||||
Ok(convert_to_dict_comprehension(
|
||||
fix_type,
|
||||
binding,
|
||||
for_stmt,
|
||||
if_test.map(std::convert::AsRef::as_ref),
|
||||
key.as_ref(),
|
||||
value.as_ref(),
|
||||
checker,
|
||||
))
|
||||
});
|
||||
|
||||
checker.report_diagnostic(diagnostic);
|
||||
} else {
|
||||
checker.report_diagnostic(Diagnostic::new(
|
||||
ManualDictComprehension {
|
||||
fix_type: DictComprehensionType::Comprehension,
|
||||
is_async: for_stmt.is_async,
|
||||
},
|
||||
*range,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
fn convert_to_dict_comprehension(
|
||||
fix_type: DictComprehensionType,
|
||||
binding: &Binding,
|
||||
for_stmt: &ast::StmtFor,
|
||||
if_test: Option<&ast::Expr>,
|
||||
key: &Expr,
|
||||
value: &Expr,
|
||||
checker: &Checker,
|
||||
) -> Option<Fix> {
|
||||
let locator = checker.locator();
|
||||
|
||||
let if_str = match if_test {
|
||||
Some(test) => {
|
||||
// If the test is an assignment expression,
|
||||
// we must parenthesize it when it appears
|
||||
// inside the comprehension to avoid a syntax error.
|
||||
//
|
||||
// Notice that we do not need `any_over_expr` here,
|
||||
// since if the assignment expression appears
|
||||
// internally (e.g. as an operand in a boolean
|
||||
// operation) then it will already be parenthesized.
|
||||
if test.is_named_expr() {
|
||||
format!(" if ({})", locator.slice(test.range()))
|
||||
} else {
|
||||
format!(" if {}", locator.slice(test.range()))
|
||||
}
|
||||
}
|
||||
None => String::new(),
|
||||
};
|
||||
|
||||
// if the loop target was an implicit tuple, add parentheses around it
|
||||
// ```python
|
||||
// for i in a, b:
|
||||
// ...
|
||||
// ```
|
||||
// becomes
|
||||
// {... for i in (a, b)}
|
||||
let iter_str = if let Expr::Tuple(ast::ExprTuple {
|
||||
parenthesized: false,
|
||||
..
|
||||
}) = &*for_stmt.iter
|
||||
{
|
||||
format!("({})", locator.slice(for_stmt.iter.range()))
|
||||
} else {
|
||||
locator.slice(for_stmt.iter.range()).to_string()
|
||||
};
|
||||
|
||||
let target_str = locator.slice(for_stmt.target.range());
|
||||
let for_type = if for_stmt.is_async {
|
||||
"async for"
|
||||
} else {
|
||||
"for"
|
||||
};
|
||||
let elt_str = format!(
|
||||
"{}: {}",
|
||||
locator.slice(key.range()),
|
||||
locator.slice(value.range())
|
||||
);
|
||||
|
||||
let comprehension_str = format!("{{{elt_str} {for_type} {target_str} in {iter_str}{if_str}}}");
|
||||
|
||||
let for_loop_inline_comments = comment_strings_in_range(
|
||||
checker,
|
||||
for_stmt.range,
|
||||
&[key.range(), value.range(), for_stmt.iter.range()],
|
||||
);
|
||||
|
||||
let newline = checker.stylist().line_ending().as_str();
|
||||
|
||||
let indent = locator.slice(TextRange::new(
|
||||
locator.line_start(for_stmt.range.start()),
|
||||
for_stmt.range.start(),
|
||||
));
|
||||
|
||||
let variable_name = locator.slice(binding);
|
||||
match fix_type {
|
||||
DictComprehensionType::Update => {
|
||||
let indentation = if for_loop_inline_comments.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
format!("{newline}{indent}")
|
||||
};
|
||||
|
||||
let comprehension_body = format!("{variable_name}.update({comprehension_str})");
|
||||
|
||||
let text_to_replace = format!(
|
||||
"{}{indentation}{comprehension_body}",
|
||||
for_loop_inline_comments.join(&indentation)
|
||||
);
|
||||
|
||||
Some(Fix::unsafe_edit(Edit::range_replacement(
|
||||
text_to_replace,
|
||||
for_stmt.range,
|
||||
)))
|
||||
}
|
||||
DictComprehensionType::Comprehension => {
|
||||
let binding_stmt = binding.statement(checker.semantic());
|
||||
debug_assert!(
|
||||
binding_stmt.is_some(),
|
||||
"must be passed a binding with a statement"
|
||||
);
|
||||
let binding_stmt = binding_stmt?;
|
||||
|
||||
let binding_stmt_range = binding_stmt.range();
|
||||
|
||||
let annotations = match binding_stmt.as_ann_assign_stmt() {
|
||||
Some(assign) => format!(": {}", locator.slice(assign.annotation.range())),
|
||||
None => String::new(),
|
||||
};
|
||||
|
||||
// If there are multiple binding statements in one line, we don't want to accidentally delete them
|
||||
// Instead, we just delete the binding statement and leave any comments where they are
|
||||
let (binding_stmt_deletion_range, binding_is_multiple_stmts) =
|
||||
statement_deletion_range(checker, binding_stmt_range);
|
||||
|
||||
let comments_to_move = if binding_is_multiple_stmts {
|
||||
for_loop_inline_comments
|
||||
} else {
|
||||
let mut new_comments =
|
||||
comment_strings_in_range(checker, binding_stmt_deletion_range, &[]);
|
||||
new_comments.extend(for_loop_inline_comments);
|
||||
new_comments
|
||||
};
|
||||
|
||||
let indentation = if comments_to_move.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
format!("{newline}{indent}")
|
||||
};
|
||||
let leading_comments = format!("{}{indentation}", comments_to_move.join(&indentation));
|
||||
|
||||
let comprehension_body =
|
||||
format!("{leading_comments}{variable_name}{annotations} = {comprehension_str}");
|
||||
Some(Fix::unsafe_edits(
|
||||
Edit::range_deletion(binding_stmt_deletion_range),
|
||||
[Edit::range_replacement(comprehension_body, for_stmt.range)],
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
enum DictComprehensionType {
|
||||
Update,
|
||||
Comprehension,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,16 +1,15 @@
|
|||
use ruff_python_ast::{self as ast, Arguments, Expr};
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::{checkers::ast::Checker, rules::perflint::helpers::statement_deletion_range};
|
||||
use anyhow::{anyhow, Result};
|
||||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
|
||||
|
||||
use crate::rules::perflint::helpers::comment_strings_in_range;
|
||||
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||
use ruff_python_ast::helpers::any_over_expr;
|
||||
use ruff_python_semantic::{analyze::typing::is_list, Binding};
|
||||
use ruff_python_trivia::{BackwardsTokenizer, PythonWhitespace, SimpleTokenKind, SimpleTokenizer};
|
||||
use ruff_source_file::LineRanges;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for `for` loops that can be replaced by a list comprehension.
|
||||
///
|
||||
|
|
@ -264,12 +263,14 @@ pub(crate) fn manual_list_comprehension(checker: &Checker, for_stmt: &ast::StmtF
|
|||
ast::Stmt::Assign(assign) => Some(&assign.value),
|
||||
_ => None,
|
||||
});
|
||||
|
||||
// If the variable is an empty list literal, then we might be able to replace it with a full list comprehension
|
||||
// otherwise, it has to be replaced with a `list.extend`
|
||||
// otherwise, it has to be replaced with a `list.extend`.
|
||||
let binding_is_empty_list =
|
||||
list_binding_value.is_some_and(|binding_value| match binding_value.as_list_expr() {
|
||||
Some(list_expr) => list_expr.elts.is_empty(),
|
||||
None => false,
|
||||
list_binding_value.is_some_and(|binding_value| match binding_value {
|
||||
// `value = []`
|
||||
Expr::List(list_expr) => list_expr.is_empty(),
|
||||
_ => false,
|
||||
});
|
||||
|
||||
// If the for loop does not have the same parent element as the binding, then it cannot always be
|
||||
|
|
@ -397,22 +398,12 @@ fn convert_to_list_extend(
|
|||
let elt_str = locator.slice(to_append);
|
||||
let generator_str = format!("{elt_str} {for_type} {target_str} in {for_iter_str}{if_str}");
|
||||
|
||||
let comment_strings_in_range = |range| {
|
||||
checker
|
||||
.comment_ranges()
|
||||
.comments_in_range(range)
|
||||
.iter()
|
||||
// Ignore comments inside of the append or iterator, since these are preserved
|
||||
.filter(|comment| {
|
||||
!to_append.range().contains_range(**comment)
|
||||
&& !for_stmt.iter.range().contains_range(**comment)
|
||||
})
|
||||
.map(|range| locator.slice(range).trim_whitespace_start())
|
||||
.collect()
|
||||
};
|
||||
|
||||
let variable_name = locator.slice(binding);
|
||||
let for_loop_inline_comments: Vec<&str> = comment_strings_in_range(for_stmt.range);
|
||||
let for_loop_inline_comments = comment_strings_in_range(
|
||||
checker,
|
||||
for_stmt.range,
|
||||
&[to_append.range(), for_stmt.iter.range()],
|
||||
);
|
||||
|
||||
let newline = checker.stylist().line_ending().as_str();
|
||||
|
||||
|
|
@ -457,74 +448,25 @@ fn convert_to_list_extend(
|
|||
.ok_or(anyhow!(
|
||||
"Binding must have a statement to convert into a list comprehension"
|
||||
))?;
|
||||
// If the binding has multiple statements on its line, the fix would be substantially more complicated
|
||||
let (semicolon_before, after_semicolon) = {
|
||||
// determine whether there's a semicolon either before or after the binding statement.
|
||||
// Since it's a binding statement, we can just check whether there's a semicolon immediately
|
||||
// after the whitespace in front of or behind it
|
||||
let mut after_tokenizer =
|
||||
SimpleTokenizer::starts_at(binding_stmt_range.end(), locator.contents())
|
||||
.skip_trivia();
|
||||
|
||||
let after_semicolon = if after_tokenizer
|
||||
.next()
|
||||
.is_some_and(|token| token.kind() == SimpleTokenKind::Semi)
|
||||
{
|
||||
after_tokenizer.next()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let semicolon_before = BackwardsTokenizer::up_to(
|
||||
binding_stmt_range.start(),
|
||||
locator.contents(),
|
||||
checker.comment_ranges(),
|
||||
)
|
||||
.skip_trivia()
|
||||
.next()
|
||||
.filter(|token| token.kind() == SimpleTokenKind::Semi);
|
||||
|
||||
(semicolon_before, after_semicolon)
|
||||
};
|
||||
// If there are multiple binding statements in one line, we don't want to accidentally delete them
|
||||
// Instead, we just delete the binding statement and leave any comments where they are
|
||||
let (binding_stmt_deletion_range, binding_is_multiple_stmts) =
|
||||
match (semicolon_before, after_semicolon) {
|
||||
// ```python
|
||||
// a = []
|
||||
// ```
|
||||
(None, None) => (locator.full_lines_range(binding_stmt_range), false),
|
||||
|
||||
// ```python
|
||||
// a = 1; b = []
|
||||
// ^^^^^^^^
|
||||
// a = 1; b = []; c = 3
|
||||
// ^^^^^^^^
|
||||
// ```
|
||||
(Some(semicolon_before), Some(_) | None) => (
|
||||
TextRange::new(semicolon_before.start(), binding_stmt_range.end()),
|
||||
true,
|
||||
),
|
||||
|
||||
// ```python
|
||||
// a = []; b = 3
|
||||
// ^^^^^^^
|
||||
// ```
|
||||
(None, Some(after_semicolon)) => (
|
||||
TextRange::new(binding_stmt_range.start(), after_semicolon.start()),
|
||||
true,
|
||||
),
|
||||
};
|
||||
statement_deletion_range(checker, binding_stmt_range);
|
||||
|
||||
let annotations = match binding_stmt.and_then(|stmt| stmt.as_ann_assign_stmt()) {
|
||||
Some(assign) => format!(": {}", locator.slice(assign.annotation.range())),
|
||||
None => String::new(),
|
||||
};
|
||||
|
||||
let mut comments_to_move = for_loop_inline_comments;
|
||||
if !binding_is_multiple_stmts {
|
||||
comments_to_move.extend(comment_strings_in_range(binding_stmt_deletion_range));
|
||||
}
|
||||
let comments_to_move = if binding_is_multiple_stmts {
|
||||
for_loop_inline_comments
|
||||
} else {
|
||||
let mut new_comments =
|
||||
comment_strings_in_range(checker, binding_stmt_deletion_range, &[]);
|
||||
new_comments.extend(for_loop_inline_comments);
|
||||
new_comments
|
||||
};
|
||||
|
||||
let indentation = if comments_to_move.is_empty() {
|
||||
String::new()
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/perflint/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
PERF403.py:5:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
|
|
@ -9,6 +8,7 @@ PERF403.py:5:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
|||
5 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:13:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
|
|
@ -17,43 +17,114 @@ PERF403.py:13:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
|||
13 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:31:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
PERF403.py:33:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
29 | for idx, name in enumerate(fruit):
|
||||
30 | if idx % 2:
|
||||
31 | result[idx] = name # PERF403
|
||||
31 | for idx, name in enumerate(fruit):
|
||||
32 | if idx % 2:
|
||||
33 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:61:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
PERF403.py:63:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
59 | for idx, name in enumerate(fruit):
|
||||
60 | if idx % 2:
|
||||
61 | result[idx] = name # PERF403
|
||||
61 | for idx, name in enumerate(fruit):
|
||||
62 | if idx % 2:
|
||||
63 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:76:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
PERF403.py:78:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
74 | result = {}
|
||||
75 | for name in fruit:
|
||||
76 | result[name] = name # PERF403
|
||||
76 | result = {}
|
||||
77 | for name in fruit:
|
||||
78 | result[name] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:83:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
PERF403.py:85:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
81 | result = {}
|
||||
82 | for idx, name in enumerate(fruit):
|
||||
83 | result[name] = idx # PERF403
|
||||
83 | result = {}
|
||||
84 | for idx, name in enumerate(fruit):
|
||||
85 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:91:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
PERF403.py:94:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
89 | result = SneakyDict()
|
||||
90 | for idx, name in enumerate(fruit):
|
||||
91 | result[name] = idx # PERF403
|
||||
92 | result = SneakyDict()
|
||||
93 | for idx, name in enumerate(fruit):
|
||||
94 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:106:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
104 | ):
|
||||
105 | # comment 3
|
||||
106 | / result[
|
||||
107 | | name # comment 4
|
||||
108 | | ] = idx # PERF403
|
||||
| |_______________^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:115:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
113 | a = 1; result = {}; b = 2
|
||||
114 | for idx, name in enumerate(fruit):
|
||||
115 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:122:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
120 | result = {"kiwi": 3}
|
||||
121 | for idx, name in enumerate(fruit):
|
||||
122 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:129:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
127 | (_, result) = (None, {"kiwi": 3})
|
||||
128 | for idx, name in enumerate(fruit):
|
||||
129 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:137:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
135 | print(len(result))
|
||||
136 | for idx, name in enumerate(fruit):
|
||||
137 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:145:13: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
143 | for idx, name in enumerate(fruit):
|
||||
144 | if last_idx := idx % 3:
|
||||
145 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403.py:153:9: PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
151 | result = {}
|
||||
152 | for idx, name in indices, fruit:
|
||||
153 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
|
|
|||
|
|
@ -169,10 +169,10 @@ PERF401.py:119:13: PERF401 [*] Use a list comprehension to create a transformed
|
|||
117 |- # single-line comment 2 should be protected
|
||||
118 |- if i % 2: # single-line comment 3 should be protected
|
||||
119 |- result.append(i) # PERF401
|
||||
115 |+ # single-line comment 1 should be protected
|
||||
116 |+ # single-line comment 2 should be protected
|
||||
117 |+ # single-line comment 3 should be protected
|
||||
118 |+ # comment after assignment should be protected
|
||||
115 |+ # comment after assignment should be protected
|
||||
116 |+ # single-line comment 1 should be protected
|
||||
117 |+ # single-line comment 2 should be protected
|
||||
118 |+ # single-line comment 3 should be protected
|
||||
119 |+ result = [i for i in range(10) if i % 2] # PERF401
|
||||
120 120 |
|
||||
121 121 |
|
||||
|
|
|
|||
|
|
@ -0,0 +1,307 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/perflint/mod.rs
|
||||
---
|
||||
PERF403.py:5:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
3 | result = {}
|
||||
4 | for idx, name in enumerate(fruit):
|
||||
5 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
1 1 | def foo():
|
||||
2 2 | fruit = ["apple", "pear", "orange"]
|
||||
3 |- result = {}
|
||||
4 |- for idx, name in enumerate(fruit):
|
||||
5 |- result[idx] = name # PERF403
|
||||
3 |+ result = {idx: name for idx, name in enumerate(fruit)} # PERF403
|
||||
6 4 |
|
||||
7 5 |
|
||||
8 6 | def foo():
|
||||
|
||||
PERF403.py:13:13: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
11 | for idx, name in enumerate(fruit):
|
||||
12 | if idx % 2:
|
||||
13 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
7 7 |
|
||||
8 8 | def foo():
|
||||
9 9 | fruit = ["apple", "pear", "orange"]
|
||||
10 |- result = {}
|
||||
11 |- for idx, name in enumerate(fruit):
|
||||
12 |- if idx % 2:
|
||||
13 |- result[idx] = name # PERF403
|
||||
10 |+ result = {idx: name for idx, name in enumerate(fruit) if idx % 2} # PERF403
|
||||
14 11 |
|
||||
15 12 |
|
||||
16 13 | def foo():
|
||||
|
||||
PERF403.py:33:13: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
31 | for idx, name in enumerate(fruit):
|
||||
32 | if idx % 2:
|
||||
33 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
26 26 |
|
||||
27 27 |
|
||||
28 28 | def foo():
|
||||
29 |- result = {}
|
||||
30 29 | fruit = ["apple", "pear", "orange"]
|
||||
31 |- for idx, name in enumerate(fruit):
|
||||
32 |- if idx % 2:
|
||||
33 |- result[idx] = name # PERF403
|
||||
30 |+ result = {idx: name for idx, name in enumerate(fruit) if idx % 2} # PERF403
|
||||
34 31 |
|
||||
35 32 |
|
||||
36 33 | def foo():
|
||||
|
||||
PERF403.py:63:13: PERF403 [*] Use `dict.update` instead of a for-loop
|
||||
|
|
||||
61 | for idx, name in enumerate(fruit):
|
||||
62 | if idx % 2:
|
||||
63 | result[idx] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with `dict.update`
|
||||
|
||||
ℹ Unsafe fix
|
||||
58 58 | def foo():
|
||||
59 59 | result = {1: "banana"}
|
||||
60 60 | fruit = ["apple", "pear", "orange"]
|
||||
61 |- for idx, name in enumerate(fruit):
|
||||
62 |- if idx % 2:
|
||||
63 |- result[idx] = name # PERF403
|
||||
61 |+ result.update({idx: name for idx, name in enumerate(fruit) if idx % 2}) # PERF403
|
||||
64 62 |
|
||||
65 63 |
|
||||
66 64 | def foo():
|
||||
|
||||
PERF403.py:78:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
76 | result = {}
|
||||
77 | for name in fruit:
|
||||
78 | result[name] = name # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
73 73 |
|
||||
74 74 | def foo():
|
||||
75 75 | fruit = ["apple", "pear", "orange"]
|
||||
76 |- result = {}
|
||||
77 |- for name in fruit:
|
||||
78 |- result[name] = name # PERF403
|
||||
76 |+ result = {name: name for name in fruit} # PERF403
|
||||
79 77 |
|
||||
80 78 |
|
||||
81 79 | def foo():
|
||||
|
||||
PERF403.py:85:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
83 | result = {}
|
||||
84 | for idx, name in enumerate(fruit):
|
||||
85 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
80 80 |
|
||||
81 81 | def foo():
|
||||
82 82 | fruit = ["apple", "pear", "orange"]
|
||||
83 |- result = {}
|
||||
84 |- for idx, name in enumerate(fruit):
|
||||
85 |- result[name] = idx # PERF403
|
||||
83 |+ result = {name: idx for idx, name in enumerate(fruit)} # PERF403
|
||||
86 84 |
|
||||
87 85 |
|
||||
88 86 | def foo():
|
||||
|
||||
PERF403.py:94:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
92 | result = SneakyDict()
|
||||
93 | for idx, name in enumerate(fruit):
|
||||
94 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
89 89 | from builtins import dict as SneakyDict
|
||||
90 90 |
|
||||
91 91 | fruit = ["apple", "pear", "orange"]
|
||||
92 |- result = SneakyDict()
|
||||
93 |- for idx, name in enumerate(fruit):
|
||||
94 |- result[name] = idx # PERF403
|
||||
92 |+ result = {name: idx for idx, name in enumerate(fruit)} # PERF403
|
||||
95 93 |
|
||||
96 94 |
|
||||
97 95 | def foo():
|
||||
|
||||
PERF403.py:106:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
104 | ):
|
||||
105 | # comment 3
|
||||
106 | / result[
|
||||
107 | | name # comment 4
|
||||
108 | | ] = idx # PERF403
|
||||
| |_______________^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
96 96 |
|
||||
97 97 | def foo():
|
||||
98 98 | fruit = ["apple", "pear", "orange"]
|
||||
99 |- result: dict[str, int] = {
|
||||
100 |- # comment 1
|
||||
101 |- }
|
||||
102 |- for idx, name in enumerate(
|
||||
99 |+ # comment 1
|
||||
100 |+ # comment 3
|
||||
101 |+ # comment 4
|
||||
102 |+ result: dict[str, int] = {name: idx for idx, name in enumerate(
|
||||
103 103 | fruit # comment 2
|
||||
104 |- ):
|
||||
105 |- # comment 3
|
||||
106 |- result[
|
||||
107 |- name # comment 4
|
||||
108 |- ] = idx # PERF403
|
||||
104 |+ )} # PERF403
|
||||
109 105 |
|
||||
110 106 |
|
||||
111 107 | def foo():
|
||||
|
||||
PERF403.py:115:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
113 | a = 1; result = {}; b = 2
|
||||
114 | for idx, name in enumerate(fruit):
|
||||
115 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
110 110 |
|
||||
111 111 | def foo():
|
||||
112 112 | fruit = ["apple", "pear", "orange"]
|
||||
113 |- a = 1; result = {}; b = 2
|
||||
114 |- for idx, name in enumerate(fruit):
|
||||
115 |- result[name] = idx # PERF403
|
||||
113 |+ a = 1; b = 2
|
||||
114 |+ result = {name: idx for idx, name in enumerate(fruit)} # PERF403
|
||||
116 115 |
|
||||
117 116 |
|
||||
118 117 | def foo():
|
||||
|
||||
PERF403.py:122:9: PERF403 [*] Use `dict.update` instead of a for-loop
|
||||
|
|
||||
120 | result = {"kiwi": 3}
|
||||
121 | for idx, name in enumerate(fruit):
|
||||
122 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with `dict.update`
|
||||
|
||||
ℹ Unsafe fix
|
||||
118 118 | def foo():
|
||||
119 119 | fruit = ["apple", "pear", "orange"]
|
||||
120 120 | result = {"kiwi": 3}
|
||||
121 |- for idx, name in enumerate(fruit):
|
||||
122 |- result[name] = idx # PERF403
|
||||
121 |+ result.update({name: idx for idx, name in enumerate(fruit)}) # PERF403
|
||||
123 122 |
|
||||
124 123 |
|
||||
125 124 | def foo():
|
||||
|
||||
PERF403.py:129:9: PERF403 [*] Use `dict.update` instead of a for-loop
|
||||
|
|
||||
127 | (_, result) = (None, {"kiwi": 3})
|
||||
128 | for idx, name in enumerate(fruit):
|
||||
129 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with `dict.update`
|
||||
|
||||
ℹ Unsafe fix
|
||||
125 125 | def foo():
|
||||
126 126 | fruit = ["apple", "pear", "orange"]
|
||||
127 127 | (_, result) = (None, {"kiwi": 3})
|
||||
128 |- for idx, name in enumerate(fruit):
|
||||
129 |- result[name] = idx # PERF403
|
||||
128 |+ result.update({name: idx for idx, name in enumerate(fruit)}) # PERF403
|
||||
130 129 |
|
||||
131 130 |
|
||||
132 131 | def foo():
|
||||
|
||||
PERF403.py:137:9: PERF403 [*] Use `dict.update` instead of a for-loop
|
||||
|
|
||||
135 | print(len(result))
|
||||
136 | for idx, name in enumerate(fruit):
|
||||
137 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with `dict.update`
|
||||
|
||||
ℹ Unsafe fix
|
||||
133 133 | fruit = ["apple", "pear", "orange"]
|
||||
134 134 | result = {}
|
||||
135 135 | print(len(result))
|
||||
136 |- for idx, name in enumerate(fruit):
|
||||
137 |- result[name] = idx # PERF403
|
||||
136 |+ result.update({name: idx for idx, name in enumerate(fruit)}) # PERF403
|
||||
138 137 |
|
||||
139 138 |
|
||||
140 139 | def foo():
|
||||
|
||||
PERF403.py:145:13: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
143 | for idx, name in enumerate(fruit):
|
||||
144 | if last_idx := idx % 3:
|
||||
145 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
139 139 |
|
||||
140 140 | def foo():
|
||||
141 141 | fruit = ["apple", "pear", "orange"]
|
||||
142 |- result = {}
|
||||
143 |- for idx, name in enumerate(fruit):
|
||||
144 |- if last_idx := idx % 3:
|
||||
145 |- result[name] = idx # PERF403
|
||||
142 |+ result = {name: idx for idx, name in enumerate(fruit) if (last_idx := idx % 3)} # PERF403
|
||||
146 143 |
|
||||
147 144 |
|
||||
148 145 | def foo():
|
||||
|
||||
PERF403.py:153:9: PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
|
|
||||
151 | result = {}
|
||||
152 | for idx, name in indices, fruit:
|
||||
153 | result[name] = idx # PERF403
|
||||
| ^^^^^^^^^^^^^^^^^^ PERF403
|
||||
|
|
||||
= help: Replace for loop with dict comprehension
|
||||
|
||||
ℹ Unsafe fix
|
||||
148 148 | def foo():
|
||||
149 149 | fruit = ["apple", "pear", "orange"]
|
||||
150 150 | indices = [0, 1, 2]
|
||||
151 |- result = {}
|
||||
152 |- for idx, name in indices, fruit:
|
||||
153 |- result[name] = idx # PERF403
|
||||
151 |+ result = {name: idx for idx, name in (indices, fruit)} # PERF403
|
||||
Loading…
Add table
Add a link
Reference in a new issue