Skip to content

Move big rule implementations #18931

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jun 25, 2025
Merged

Move big rule implementations #18931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
23a7afc
Movement comment adds
MeGaGiGaGon Jun 24, 2025
30400bf
Resolve compilation errors
MeGaGiGaGon Jun 23, 2025
bc951ee
Resolve compilation errors 2
MeGaGiGaGon Jun 23, 2025
2c84e30
Resolve compilation errors 3
MeGaGiGaGon Jun 24, 2025
c9f295d
Fix clippy errors
MeGaGiGaGon Jun 24, 2025
48bf91a
Movement tab to space
MeGaGiGaGon Jun 24, 2025
f34cc04
Precommit fixes
MeGaGiGaGon Jun 24, 2025
855f13c
Merge branch 'main' of https://github.com/astral-sh/ruff into move-bi…
MeGaGiGaGon Jun 25, 2025
e0c6d0f
Update deferred_scopes.rs
MeGaGiGaGon Jun 25, 2025
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
260 changes: 5 additions & 255 deletions crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Binding, BindingKind, Imported, ResolvedReference, ScopeKind};
use ruff_text_size::Ranged;
use rustc_hash::FxHashMap;
use ruff_python_semantic::{Binding, ScopeKind};

use crate::Fix;
use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::fix;
use crate::rules::{
flake8_builtins, flake8_pyi, flake8_type_checking, flake8_unused_arguments, pep8_naming,
pyflakes, pylint, ruff,
Expand Down Expand Up @@ -93,265 +88,20 @@ pub(crate) fn deferred_scopes(checker: &Checker) {
pyflakes::rules::undefined_local(checker, scope_id, scope);
}

// PLW0602
if checker.is_rule_enabled(Rule::GlobalVariableNotAssigned) {
for (name, binding_id) in scope.bindings() {
let binding = checker.semantic.binding(binding_id);
// If the binding is a `global`, then it's a top-level `global` that was never
// assigned in the current scope. If it were assigned, the `global` would be
// shadowed by the assignment.
if binding.kind.is_global() {
// If the binding was conditionally deleted, it will include a reference within
// a `Del` context, but won't be shadowed by a `BindingKind::Deletion`, as in:
// ```python
// if condition:
// del var
// ```
if binding
.references
.iter()
.map(|id| checker.semantic.reference(*id))
.all(ResolvedReference::is_load)
{
checker.report_diagnostic(
pylint::rules::GlobalVariableNotAssigned {
name: (*name).to_string(),
},
binding.range(),
);
}
}
}
pylint::rules::global_variable_not_assigned(checker, scope);
}

// PLR1704
if checker.is_rule_enabled(Rule::RedefinedArgumentFromLocal) {
for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
let binding = &checker.semantic.bindings[shadow.binding_id()];
if !matches!(
binding.kind,
BindingKind::LoopVar
| BindingKind::BoundException
| BindingKind::WithItemVar
) {
continue;
}
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
if !shadowed.kind.is_argument() {
continue;
}
if checker.settings().dummy_variable_rgx.is_match(name) {
continue;
}
let scope = &checker.semantic.scopes[binding.scope];
if scope.kind.is_generator() {
continue;
}
checker.report_diagnostic(
pylint::rules::RedefinedArgumentFromLocal {
name: name.to_string(),
},
binding.range(),
);
}
}
pylint::rules::redefined_argument_from_local(checker, scope_id, scope);
}

// F402
if checker.is_rule_enabled(Rule::ImportShadowedByLoopVar) {
for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
// If the shadowing binding isn't a loop variable, abort.
let binding = &checker.semantic.bindings[shadow.binding_id()];
if !binding.kind.is_loop_var() {
continue;
}

// If the shadowed binding isn't an import, abort.
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
if !matches!(
shadowed.kind,
BindingKind::Import(..)
| BindingKind::FromImport(..)
| BindingKind::SubmoduleImport(..)
| BindingKind::FutureImport
) {
continue;
}

// If the bindings are in different forks, abort.
if shadowed.source.is_none_or(|left| {
binding
.source
.is_none_or(|right| !checker.semantic.same_branch(left, right))
}) {
continue;
}

checker.report_diagnostic(
pyflakes::rules::ImportShadowedByLoopVar {
name: name.to_string(),
row: checker.compute_source_row(shadowed.start()),
},
binding.range(),
);
}
}
pyflakes::rules::import_shadowed_by_loop_var(checker, scope_id, scope);
}

// F811
if checker.is_rule_enabled(Rule::RedefinedWhileUnused) {
// Index the redefined bindings by statement.
let mut redefinitions = FxHashMap::default();

for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
// If the shadowing binding is a loop variable, abort, to avoid overlap
// with F402.
let binding = &checker.semantic.bindings[shadow.binding_id()];
if binding.kind.is_loop_var() {
continue;
}

// If the shadowed binding is used, abort.
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
if shadowed.is_used() {
continue;
}

// If the shadowing binding isn't considered a "redefinition" of the
// shadowed binding, abort.
if !binding.redefines(shadowed) {
continue;
}

if shadow.same_scope() {
// If the symbol is a dummy variable, abort, unless the shadowed
// binding is an import.
if !matches!(
shadowed.kind,
BindingKind::Import(..)
| BindingKind::FromImport(..)
| BindingKind::SubmoduleImport(..)
| BindingKind::FutureImport
) && checker.settings().dummy_variable_rgx.is_match(name)
{
continue;
}

let Some(node_id) = shadowed.source else {
continue;
};

// If this is an overloaded function, abort.
if shadowed.kind.is_function_definition() {
if checker
.semantic
.statement(node_id)
.as_function_def_stmt()
.is_some_and(|function| {
visibility::is_overload(
&function.decorator_list,
&checker.semantic,
)
})
{
continue;
}
}
} else {
// Only enforce cross-scope shadowing for imports.
if !matches!(
shadowed.kind,
BindingKind::Import(..)
| BindingKind::FromImport(..)
| BindingKind::SubmoduleImport(..)
| BindingKind::FutureImport
) {
continue;
}
}

// If the bindings are in different forks, abort.
if shadowed.source.is_none_or(|left| {
binding
.source
.is_none_or(|right| !checker.semantic.same_branch(left, right))
}) {
continue;
}

redefinitions
.entry(binding.source)
.or_insert_with(Vec::new)
.push((shadowed, binding));
}
}

// Create a fix for each source statement.
let mut fixes = FxHashMap::default();
for (source, entries) in &redefinitions {
let Some(source) = source else {
continue;
};

let member_names = entries
.iter()
.filter_map(|(shadowed, binding)| {
if let Some(shadowed_import) = shadowed.as_any_import() {
if let Some(import) = binding.as_any_import() {
if shadowed_import.qualified_name() == import.qualified_name() {
return Some(import.member_name());
}
}
}
None
})
.collect::<Vec<_>>();

if !member_names.is_empty() {
let statement = checker.semantic.statement(*source);
let parent = checker.semantic.parent_statement(*source);
let Ok(edit) = fix::edits::remove_unused_imports(
member_names.iter().map(std::convert::AsRef::as_ref),
statement,
parent,
checker.locator(),
checker.stylist(),
checker.indexer(),
) else {
continue;
};
fixes.insert(
*source,
Fix::safe_edit(edit).isolate(Checker::isolation(
checker.semantic().parent_statement_id(*source),
)),
);
}
}

// Create diagnostics for each statement.
for (source, entries) in &redefinitions {
for (shadowed, binding) in entries {
let mut diagnostic = checker.report_diagnostic(
pyflakes::rules::RedefinedWhileUnused {
name: binding.name(checker.source()).to_string(),
row: checker.compute_source_row(shadowed.start()),
},
binding.range(),
);

if let Some(range) = binding.parent_range(&checker.semantic) {
diagnostic.set_parent(range.start());
}

if let Some(fix) = source.as_ref().and_then(|source| fixes.get(source)) {
diagnostic.set_fix(fix.clone());
}
}
}
pyflakes::rules::redefined_while_unused(checker, scope_id, scope);
}

if checker.source_type.is_stub()
Expand Down
13 changes: 1 addition & 12 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
if checker.is_rule_enabled(Rule::NonlocalWithoutBinding) {
if !checker.semantic.scope_id.is_global() {
for name in names {
if checker.semantic.nonlocal(name).is_none() {
checker.report_diagnostic(
pylint::rules::NonlocalWithoutBinding {
name: name.to_string(),
},
name.range(),
);
}
}
}
pylint::rules::nonlocal_without_binding(checker, nonlocal);
}
if checker.is_rule_enabled(Rule::NonlocalAndGlobal) {
pylint::rules::nonlocal_and_global(checker, nonlocal);
Expand Down
45 changes: 45 additions & 0 deletions crates/ruff_linter/src/rules/pyflakes/rules/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_semantic::{BindingKind, Scope, ScopeId};
use ruff_source_file::SourceRow;
use ruff_text_size::Ranged;

use crate::Violation;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for import bindings that are shadowed by loop variables.
Expand Down Expand Up @@ -44,6 +47,48 @@ impl Violation for ImportShadowedByLoopVar {
}
}

/// F402
pub(crate) fn import_shadowed_by_loop_var(checker: &Checker, scope_id: ScopeId, scope: &Scope) {
for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic().shadowed_bindings(scope_id, binding_id) {
// If the shadowing binding isn't a loop variable, abort.
let binding = &checker.semantic().bindings[shadow.binding_id()];
if !binding.kind.is_loop_var() {
continue;
}

// If the shadowed binding isn't an import, abort.
let shadowed = &checker.semantic().bindings[shadow.shadowed_id()];
if !matches!(
shadowed.kind,
BindingKind::Import(..)
| BindingKind::FromImport(..)
| BindingKind::SubmoduleImport(..)
| BindingKind::FutureImport
) {
continue;
}

// If the bindings are in different forks, abort.
if shadowed.source.is_none_or(|left| {
binding
.source
.is_none_or(|right| !checker.semantic().same_branch(left, right))
}) {
continue;
}

checker.report_diagnostic(
ImportShadowedByLoopVar {
name: name.to_string(),
row: checker.compute_source_row(shadowed.start()),
},
binding.range(),
);
}
}
}

/// ## What it does
/// Checks for the use of wildcard imports.
///
Expand Down
Loading
Loading