Skip to content

Commit f6def1c

Browse files
authored
Move big rule implementations (#18931)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> Here's the part that was split out of #18906. I wanted to move these into the rule files since the rest of the rules in `deferred_scope`/`statement` have that same structure of implementations being in the rule definition file. It also resolves the dilemma of where to put the comment, at least for these rules. ## Test Plan <!-- How was it tested? --> N/A, no test/functionality affected
1 parent 5aab498 commit f6def1c

File tree

7 files changed

+299
-269
lines changed

7 files changed

+299
-269
lines changed

crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs

Lines changed: 5 additions & 255 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
1-
use ruff_python_semantic::analyze::visibility;
2-
use ruff_python_semantic::{Binding, BindingKind, Imported, ResolvedReference, ScopeKind};
3-
use ruff_text_size::Ranged;
4-
use rustc_hash::FxHashMap;
1+
use ruff_python_semantic::{Binding, ScopeKind};
52

6-
use crate::Fix;
73
use crate::checkers::ast::Checker;
84
use crate::codes::Rule;
9-
use crate::fix;
105
use crate::rules::{
116
flake8_builtins, flake8_pyi, flake8_type_checking, flake8_unused_arguments, pep8_naming,
127
pyflakes, pylint, ruff,
@@ -93,265 +88,20 @@ pub(crate) fn deferred_scopes(checker: &Checker) {
9388
pyflakes::rules::undefined_local(checker, scope_id, scope);
9489
}
9590

96-
// PLW0602
9791
if checker.is_rule_enabled(Rule::GlobalVariableNotAssigned) {
98-
for (name, binding_id) in scope.bindings() {
99-
let binding = checker.semantic.binding(binding_id);
100-
// If the binding is a `global`, then it's a top-level `global` that was never
101-
// assigned in the current scope. If it were assigned, the `global` would be
102-
// shadowed by the assignment.
103-
if binding.kind.is_global() {
104-
// If the binding was conditionally deleted, it will include a reference within
105-
// a `Del` context, but won't be shadowed by a `BindingKind::Deletion`, as in:
106-
// ```python
107-
// if condition:
108-
// del var
109-
// ```
110-
if binding
111-
.references
112-
.iter()
113-
.map(|id| checker.semantic.reference(*id))
114-
.all(ResolvedReference::is_load)
115-
{
116-
checker.report_diagnostic(
117-
pylint::rules::GlobalVariableNotAssigned {
118-
name: (*name).to_string(),
119-
},
120-
binding.range(),
121-
);
122-
}
123-
}
124-
}
92+
pylint::rules::global_variable_not_assigned(checker, scope);
12593
}
12694

127-
// PLR1704
12895
if checker.is_rule_enabled(Rule::RedefinedArgumentFromLocal) {
129-
for (name, binding_id) in scope.bindings() {
130-
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
131-
let binding = &checker.semantic.bindings[shadow.binding_id()];
132-
if !matches!(
133-
binding.kind,
134-
BindingKind::LoopVar
135-
| BindingKind::BoundException
136-
| BindingKind::WithItemVar
137-
) {
138-
continue;
139-
}
140-
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
141-
if !shadowed.kind.is_argument() {
142-
continue;
143-
}
144-
if checker.settings().dummy_variable_rgx.is_match(name) {
145-
continue;
146-
}
147-
let scope = &checker.semantic.scopes[binding.scope];
148-
if scope.kind.is_generator() {
149-
continue;
150-
}
151-
checker.report_diagnostic(
152-
pylint::rules::RedefinedArgumentFromLocal {
153-
name: name.to_string(),
154-
},
155-
binding.range(),
156-
);
157-
}
158-
}
96+
pylint::rules::redefined_argument_from_local(checker, scope_id, scope);
15997
}
16098

161-
// F402
16299
if checker.is_rule_enabled(Rule::ImportShadowedByLoopVar) {
163-
for (name, binding_id) in scope.bindings() {
164-
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
165-
// If the shadowing binding isn't a loop variable, abort.
166-
let binding = &checker.semantic.bindings[shadow.binding_id()];
167-
if !binding.kind.is_loop_var() {
168-
continue;
169-
}
170-
171-
// If the shadowed binding isn't an import, abort.
172-
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
173-
if !matches!(
174-
shadowed.kind,
175-
BindingKind::Import(..)
176-
| BindingKind::FromImport(..)
177-
| BindingKind::SubmoduleImport(..)
178-
| BindingKind::FutureImport
179-
) {
180-
continue;
181-
}
182-
183-
// If the bindings are in different forks, abort.
184-
if shadowed.source.is_none_or(|left| {
185-
binding
186-
.source
187-
.is_none_or(|right| !checker.semantic.same_branch(left, right))
188-
}) {
189-
continue;
190-
}
191-
192-
checker.report_diagnostic(
193-
pyflakes::rules::ImportShadowedByLoopVar {
194-
name: name.to_string(),
195-
row: checker.compute_source_row(shadowed.start()),
196-
},
197-
binding.range(),
198-
);
199-
}
200-
}
100+
pyflakes::rules::import_shadowed_by_loop_var(checker, scope_id, scope);
201101
}
202102

203-
// F811
204103
if checker.is_rule_enabled(Rule::RedefinedWhileUnused) {
205-
// Index the redefined bindings by statement.
206-
let mut redefinitions = FxHashMap::default();
207-
208-
for (name, binding_id) in scope.bindings() {
209-
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
210-
// If the shadowing binding is a loop variable, abort, to avoid overlap
211-
// with F402.
212-
let binding = &checker.semantic.bindings[shadow.binding_id()];
213-
if binding.kind.is_loop_var() {
214-
continue;
215-
}
216-
217-
// If the shadowed binding is used, abort.
218-
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
219-
if shadowed.is_used() {
220-
continue;
221-
}
222-
223-
// If the shadowing binding isn't considered a "redefinition" of the
224-
// shadowed binding, abort.
225-
if !binding.redefines(shadowed) {
226-
continue;
227-
}
228-
229-
if shadow.same_scope() {
230-
// If the symbol is a dummy variable, abort, unless the shadowed
231-
// binding is an import.
232-
if !matches!(
233-
shadowed.kind,
234-
BindingKind::Import(..)
235-
| BindingKind::FromImport(..)
236-
| BindingKind::SubmoduleImport(..)
237-
| BindingKind::FutureImport
238-
) && checker.settings().dummy_variable_rgx.is_match(name)
239-
{
240-
continue;
241-
}
242-
243-
let Some(node_id) = shadowed.source else {
244-
continue;
245-
};
246-
247-
// If this is an overloaded function, abort.
248-
if shadowed.kind.is_function_definition() {
249-
if checker
250-
.semantic
251-
.statement(node_id)
252-
.as_function_def_stmt()
253-
.is_some_and(|function| {
254-
visibility::is_overload(
255-
&function.decorator_list,
256-
&checker.semantic,
257-
)
258-
})
259-
{
260-
continue;
261-
}
262-
}
263-
} else {
264-
// Only enforce cross-scope shadowing for imports.
265-
if !matches!(
266-
shadowed.kind,
267-
BindingKind::Import(..)
268-
| BindingKind::FromImport(..)
269-
| BindingKind::SubmoduleImport(..)
270-
| BindingKind::FutureImport
271-
) {
272-
continue;
273-
}
274-
}
275-
276-
// If the bindings are in different forks, abort.
277-
if shadowed.source.is_none_or(|left| {
278-
binding
279-
.source
280-
.is_none_or(|right| !checker.semantic.same_branch(left, right))
281-
}) {
282-
continue;
283-
}
284-
285-
redefinitions
286-
.entry(binding.source)
287-
.or_insert_with(Vec::new)
288-
.push((shadowed, binding));
289-
}
290-
}
291-
292-
// Create a fix for each source statement.
293-
let mut fixes = FxHashMap::default();
294-
for (source, entries) in &redefinitions {
295-
let Some(source) = source else {
296-
continue;
297-
};
298-
299-
let member_names = entries
300-
.iter()
301-
.filter_map(|(shadowed, binding)| {
302-
if let Some(shadowed_import) = shadowed.as_any_import() {
303-
if let Some(import) = binding.as_any_import() {
304-
if shadowed_import.qualified_name() == import.qualified_name() {
305-
return Some(import.member_name());
306-
}
307-
}
308-
}
309-
None
310-
})
311-
.collect::<Vec<_>>();
312-
313-
if !member_names.is_empty() {
314-
let statement = checker.semantic.statement(*source);
315-
let parent = checker.semantic.parent_statement(*source);
316-
let Ok(edit) = fix::edits::remove_unused_imports(
317-
member_names.iter().map(std::convert::AsRef::as_ref),
318-
statement,
319-
parent,
320-
checker.locator(),
321-
checker.stylist(),
322-
checker.indexer(),
323-
) else {
324-
continue;
325-
};
326-
fixes.insert(
327-
*source,
328-
Fix::safe_edit(edit).isolate(Checker::isolation(
329-
checker.semantic().parent_statement_id(*source),
330-
)),
331-
);
332-
}
333-
}
334-
335-
// Create diagnostics for each statement.
336-
for (source, entries) in &redefinitions {
337-
for (shadowed, binding) in entries {
338-
let mut diagnostic = checker.report_diagnostic(
339-
pyflakes::rules::RedefinedWhileUnused {
340-
name: binding.name(checker.source()).to_string(),
341-
row: checker.compute_source_row(shadowed.start()),
342-
},
343-
binding.range(),
344-
);
345-
346-
if let Some(range) = binding.parent_range(&checker.semantic) {
347-
diagnostic.set_parent(range.start());
348-
}
349-
350-
if let Some(fix) = source.as_ref().and_then(|source| fixes.get(source)) {
351-
diagnostic.set_fix(fix.clone());
352-
}
353-
}
354-
}
104+
pyflakes::rules::redefined_while_unused(checker, scope_id, scope);
355105
}
356106

357107
if checker.source_type.is_stub()

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
4545
}
4646
}
4747
if checker.is_rule_enabled(Rule::NonlocalWithoutBinding) {
48-
if !checker.semantic.scope_id.is_global() {
49-
for name in names {
50-
if checker.semantic.nonlocal(name).is_none() {
51-
checker.report_diagnostic(
52-
pylint::rules::NonlocalWithoutBinding {
53-
name: name.to_string(),
54-
},
55-
name.range(),
56-
);
57-
}
58-
}
59-
}
48+
pylint::rules::nonlocal_without_binding(checker, nonlocal);
6049
}
6150
if checker.is_rule_enabled(Rule::NonlocalAndGlobal) {
6251
pylint::rules::nonlocal_and_global(checker, nonlocal);

crates/ruff_linter/src/rules/pyflakes/rules/imports.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
use ruff_python_semantic::{BindingKind, Scope, ScopeId};
23
use ruff_source_file::SourceRow;
4+
use ruff_text_size::Ranged;
35

46
use crate::Violation;
7+
use crate::checkers::ast::Checker;
58

69
/// ## What it does
710
/// Checks for import bindings that are shadowed by loop variables.
@@ -44,6 +47,48 @@ impl Violation for ImportShadowedByLoopVar {
4447
}
4548
}
4649

50+
/// F402
51+
pub(crate) fn import_shadowed_by_loop_var(checker: &Checker, scope_id: ScopeId, scope: &Scope) {
52+
for (name, binding_id) in scope.bindings() {
53+
for shadow in checker.semantic().shadowed_bindings(scope_id, binding_id) {
54+
// If the shadowing binding isn't a loop variable, abort.
55+
let binding = &checker.semantic().bindings[shadow.binding_id()];
56+
if !binding.kind.is_loop_var() {
57+
continue;
58+
}
59+
60+
// If the shadowed binding isn't an import, abort.
61+
let shadowed = &checker.semantic().bindings[shadow.shadowed_id()];
62+
if !matches!(
63+
shadowed.kind,
64+
BindingKind::Import(..)
65+
| BindingKind::FromImport(..)
66+
| BindingKind::SubmoduleImport(..)
67+
| BindingKind::FutureImport
68+
) {
69+
continue;
70+
}
71+
72+
// If the bindings are in different forks, abort.
73+
if shadowed.source.is_none_or(|left| {
74+
binding
75+
.source
76+
.is_none_or(|right| !checker.semantic().same_branch(left, right))
77+
}) {
78+
continue;
79+
}
80+
81+
checker.report_diagnostic(
82+
ImportShadowedByLoopVar {
83+
name: name.to_string(),
84+
row: checker.compute_source_row(shadowed.start()),
85+
},
86+
binding.range(),
87+
);
88+
}
89+
}
90+
}
91+
4792
/// ## What it does
4893
/// Checks for the use of wildcard imports.
4994
///

0 commit comments

Comments
 (0)