Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ruff] Avoid treating named expressions as static keys (RUF011) #9494

Merged
merged 1 commit into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF011.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
{value.attribute: value.upper() for value in data for constant in data}
{constant[value]: value.upper() for value in data for constant in data}
{value[constant]: value.upper() for value in data for constant in data}
{local_id: token for token in tokens if (local_id := _extract_local_id(token)) is not None}
{key: kwargs.get(key) for key in kwargs.keys() if not params.get(key)}

# Errors
{"key": value.upper() for value in data}
Expand All @@ -20,3 +22,5 @@
{constant + constant: value.upper() for value in data}
{constant.attribute: value.upper() for value in data}
{constant[0]: value.upper() for value in data}
{tokens: token for token in tokens}

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::helpers;
use ruff_python_ast::helpers::NameFinder;
use ruff_python_ast::helpers::{NameFinder, StoredNameFinder};
use ruff_python_ast::visitor::Visitor;
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -80,7 +80,7 @@ impl Violation for UnusedLoopControlVariable {
/// B007
pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast::StmtFor) {
let control_names = {
let mut finder = NameFinder::default();
let mut finder = StoredNameFinder::default();
finder.visit_expr(stmt_for.target.as_ref());
finder.names
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::NameFinder;
use ruff_python_ast::helpers::StoredNameFinder;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, Expr};
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -51,9 +51,9 @@ impl Violation for StaticKeyDictComprehension {
pub(crate) fn static_key_dict_comprehension(checker: &mut Checker, dict_comp: &ast::ExprDictComp) {
// Collect the bound names in the comprehension's generators.
let names = {
let mut visitor = NameFinder::default();
let mut visitor = StoredNameFinder::default();
for generator in &dict_comp.generators {
visitor.visit_expr(&generator.target);
visitor.visit_comprehension(generator);
}
visitor.names
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,80 +1,90 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF011.py:15:2: RUF011 Dictionary comprehension uses static key: `"key"`
RUF011.py:17:2: RUF011 Dictionary comprehension uses static key: `"key"`
|
14 | # Errors
15 | {"key": value.upper() for value in data}
16 | # Errors
17 | {"key": value.upper() for value in data}
| ^^^^^ RUF011
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
18 | {True: value.upper() for value in data}
19 | {0: value.upper() for value in data}
|

RUF011.py:16:2: RUF011 Dictionary comprehension uses static key: `True`
RUF011.py:18:2: RUF011 Dictionary comprehension uses static key: `True`
|
14 | # Errors
15 | {"key": value.upper() for value in data}
16 | {True: value.upper() for value in data}
16 | # Errors
17 | {"key": value.upper() for value in data}
18 | {True: value.upper() for value in data}
| ^^^^ RUF011
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {0: value.upper() for value in data}
20 | {(1, "a"): value.upper() for value in data} # Constant tuple
|

RUF011.py:17:2: RUF011 Dictionary comprehension uses static key: `0`
RUF011.py:19:2: RUF011 Dictionary comprehension uses static key: `0`
|
15 | {"key": value.upper() for value in data}
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
17 | {"key": value.upper() for value in data}
18 | {True: value.upper() for value in data}
19 | {0: value.upper() for value in data}
| ^ RUF011
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
20 | {(1, "a"): value.upper() for value in data} # Constant tuple
21 | {constant: value.upper() for value in data}
|

RUF011.py:18:2: RUF011 Dictionary comprehension uses static key: `(1, "a")`
RUF011.py:20:2: RUF011 Dictionary comprehension uses static key: `(1, "a")`
|
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
18 | {True: value.upper() for value in data}
19 | {0: value.upper() for value in data}
20 | {(1, "a"): value.upper() for value in data} # Constant tuple
| ^^^^^^^^ RUF011
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
21 | {constant: value.upper() for value in data}
22 | {constant + constant: value.upper() for value in data}
|

RUF011.py:19:2: RUF011 Dictionary comprehension uses static key: `constant`
RUF011.py:21:2: RUF011 Dictionary comprehension uses static key: `constant`
|
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
19 | {0: value.upper() for value in data}
20 | {(1, "a"): value.upper() for value in data} # Constant tuple
21 | {constant: value.upper() for value in data}
| ^^^^^^^^ RUF011
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
22 | {constant + constant: value.upper() for value in data}
23 | {constant.attribute: value.upper() for value in data}
|

RUF011.py:20:2: RUF011 Dictionary comprehension uses static key: `constant + constant`
RUF011.py:22:2: RUF011 Dictionary comprehension uses static key: `constant + constant`
|
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
20 | {(1, "a"): value.upper() for value in data} # Constant tuple
21 | {constant: value.upper() for value in data}
22 | {constant + constant: value.upper() for value in data}
| ^^^^^^^^^^^^^^^^^^^ RUF011
21 | {constant.attribute: value.upper() for value in data}
22 | {constant[0]: value.upper() for value in data}
23 | {constant.attribute: value.upper() for value in data}
24 | {constant[0]: value.upper() for value in data}
|

RUF011.py:21:2: RUF011 Dictionary comprehension uses static key: `constant.attribute`
RUF011.py:23:2: RUF011 Dictionary comprehension uses static key: `constant.attribute`
|
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
21 | {constant: value.upper() for value in data}
22 | {constant + constant: value.upper() for value in data}
23 | {constant.attribute: value.upper() for value in data}
| ^^^^^^^^^^^^^^^^^^ RUF011
22 | {constant[0]: value.upper() for value in data}
24 | {constant[0]: value.upper() for value in data}
25 | {tokens: token for token in tokens}
|

RUF011.py:22:2: RUF011 Dictionary comprehension uses static key: `constant[0]`
RUF011.py:24:2: RUF011 Dictionary comprehension uses static key: `constant[0]`
|
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
22 | {constant[0]: value.upper() for value in data}
22 | {constant + constant: value.upper() for value in data}
23 | {constant.attribute: value.upper() for value in data}
24 | {constant[0]: value.upper() for value in data}
| ^^^^^^^^^^^ RUF011
25 | {tokens: token for token in tokens}
|

RUF011.py:25:2: RUF011 Dictionary comprehension uses static key: `tokens`
|
23 | {constant.attribute: value.upper() for value in data}
24 | {constant[0]: value.upper() for value in data}
25 | {tokens: token for token in tokens}
| ^^^^^^ RUF011
|


21 changes: 21 additions & 0 deletions crates/ruff_python_ast/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,27 @@ where
}
}

/// A [`Visitor`] to collect all stored [`Expr::Name`] nodes in an AST.
#[derive(Debug, Default)]
pub struct StoredNameFinder<'a> {
/// A map from identifier to defining expression.
pub names: FxHashMap<&'a str, &'a ast::ExprName>,
}

impl<'a, 'b> Visitor<'b> for StoredNameFinder<'a>
where
'b: 'a,
{
fn visit_expr(&mut self, expr: &'a Expr) {
if let Expr::Name(name) = expr {
if name.ctx.is_store() {
self.names.insert(&name.id, name);
}
}
crate::visitor::walk_expr(self, expr);
}
}

/// A [`StatementVisitor`] that collects all `return` statements in a function or method.
#[derive(Default)]
pub struct ReturnStatementVisitor<'a> {
Expand Down
Loading