Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove unnecessary `check_deferred_assignments` (#5963) 

## Summary

These rules can just be included in the `check_deferred_scopes`.
This commit is contained in:
Charlie Marsh 2023-07-21 22:08:44 -04:00 committed by GitHub
parent 40e9884353
commit 2dcd9e2e9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 198 additions and 199 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
crates/ruff/src/checkers/ast/deferred.rs
View File

@ -14,5 +14,4 @@ pub(crate) struct Deferred<'a> {
pub(crate) functions: Vec<Snapshot>,
pub(crate) lambdas: Vec<(&'a Expr, Snapshot)>,
pub(crate) for_loops: Vec<Snapshot>,
pub(crate) assignments: Vec<Snapshot>,
}

156
crates/ruff/src/checkers/ast/mod.rs
View File

@ -4623,8 +4623,6 @@ impl<'a> Checker<'a> {
unreachable!("Expected Stmt::FunctionDef | Stmt::AsyncFunctionDef")
}
}
self.deferred.assignments.push(snapshot);
}
}
}
@ -4646,40 +4644,6 @@ impl<'a> Checker<'a> {
} else {
unreachable!("Expected Expr::Lambda");
}
self.deferred.assignments.push(snapshot);
}
}
}
fn check_deferred_assignments(&mut self) {
while !self.deferred.assignments.is_empty() {
let assignments = std::mem::take(&mut self.deferred.assignments);
for snapshot in assignments {
self.semantic.restore(snapshot);
if self.enabled(Rule::UnusedVariable) {
pyflakes::rules::unused_variable(self, self.semantic.scope_id);
}
if self.enabled(Rule::UnusedAnnotation) {
pyflakes::rules::unused_annotation(self, self.semantic.scope_id);
}
if !self.is_stub {
if self.any_enabled(&[
Rule::UnusedFunctionArgument,
Rule::UnusedMethodArgument,
Rule::UnusedClassMethodArgument,
Rule::UnusedStaticMethodArgument,
Rule::UnusedLambdaArgument,
]) {
let scope = &self.semantic.scopes[self.semantic.scope_id];
let parent = &self.semantic.scopes[scope.parent.unwrap()];
self.diagnostics
.extend(flake8_unused_arguments::rules::unused_arguments(
self, parent, scope,
));
}
}
}
}
}
@ -4687,7 +4651,6 @@ impl<'a> Checker<'a> {
fn check_deferred_for_loops(&mut self) {
while !self.deferred.for_loops.is_empty() {
let for_loops = std::mem::take(&mut self.deferred.for_loops);
for snapshot in for_loops {
self.semantic.restore(snapshot);
@ -4765,7 +4728,6 @@ impl<'a> Checker<'a> {
}
for binding in self.semantic.bindings.iter() {
// F841
if self.enabled(Rule::UnusedVariable) {
if binding.kind.is_bound_exception() && !binding.is_used() {
let mut diagnostic = Diagnostic::new(
@ -4842,7 +4804,6 @@ impl<'a> Checker<'a> {
.add_global_reference(binding_id, range, ExecutionContext::Runtime);
} else {
if self.semantic.global_scope().uses_star_imports() {
// F405
if self.enabled(Rule::UndefinedLocalWithImportStarUsage) {
self.diagnostics.push(Diagnostic::new(
pyflakes::rules::UndefinedLocalWithImportStarUsage {
@ -4852,7 +4813,6 @@ impl<'a> Checker<'a> {
));
}
} else {
// F822
if self.enabled(Rule::UndefinedExport) {
if !self.path.ends_with("__init__.py") {
self.diagnostics.push(Diagnostic::new(
@ -4877,8 +4837,15 @@ impl<'a> Checker<'a> {
Rule::TypingOnlyFirstPartyImport,
Rule::TypingOnlyStandardLibraryImport,
Rule::TypingOnlyThirdPartyImport,
Rule::UnusedImport,
Rule::UndefinedLocal,
Rule::UnusedAnnotation,
Rule::UnusedClassMethodArgument,
Rule::UnusedFunctionArgument,
Rule::UnusedImport,
Rule::UnusedLambdaArgument,
Rule::UnusedMethodArgument,
Rule::UnusedStaticMethodArgument,
Rule::UnusedVariable,
]) {
return;
}
@ -4925,12 +4892,10 @@ impl<'a> Checker<'a> {
for scope_id in self.deferred.scopes.iter().rev().copied() {
let scope = &self.semantic.scopes[scope_id];
// F823
if self.enabled(Rule::UndefinedLocal) {
pyflakes::rules::undefined_local(self, scope_id, scope, &mut diagnostics);
}
// PLW0602
if self.enabled(Rule::GlobalVariableNotAssigned) {
for (name, binding_id) in scope.bindings() {
let binding = self.semantic.binding(binding_id);
@ -4945,7 +4910,6 @@ impl<'a> Checker<'a> {
}
}
// F402
if self.enabled(Rule::ImportShadowedByLoopVar) {
for (name, binding_id) in scope.bindings() {
for shadow in self.semantic.shadowed_bindings(scope_id, binding_id) {
@ -4990,7 +4954,6 @@ impl<'a> Checker<'a> {
}
}
// F811
if self.enabled(Rule::RedefinedWhileUnused) {
for (name, binding_id) in scope.bindings() {
for shadow in self.semantic.shadowed_bindings(scope_id, binding_id) {
@ -5077,47 +5040,77 @@ impl<'a> Checker<'a> {
}
}
// Imports in classes are public members.
if scope.kind.is_class() {
continue;
}
if enforce_typing_imports {
let runtime_imports: Vec<&Binding> = if self.settings.flake8_type_checking.strict {
vec![]
} else {
self.semantic
.scopes
.ancestor_ids(scope_id)
.flat_map(|scope_id| runtime_imports[scope_id.as_usize()].iter())
.copied()
.collect()
};
if self.enabled(Rule::RuntimeImportInTypeCheckingBlock) {
flake8_type_checking::rules::runtime_import_in_type_checking_block(
self,
scope,
&mut diagnostics,
);
if matches!(
scope.kind,
ScopeKind::Function(_) | ScopeKind::AsyncFunction(_) | ScopeKind::Lambda(_)
) {
if self.enabled(Rule::UnusedVariable) {
pyflakes::rules::unused_variable(self, scope, &mut diagnostics);
}
if self.any_enabled(&[
Rule::TypingOnlyFirstPartyImport,
Rule::TypingOnlyThirdPartyImport,
Rule::TypingOnlyStandardLibraryImport,
]) {
flake8_type_checking::rules::typing_only_runtime_import(
self,
scope,
&runtime_imports,
&mut diagnostics,
);
if self.enabled(Rule::UnusedAnnotation) {
pyflakes::rules::unused_annotation(self, scope, &mut diagnostics);
}
if !self.is_stub {
if self.any_enabled(&[
Rule::UnusedFunctionArgument,
Rule::UnusedMethodArgument,
Rule::UnusedClassMethodArgument,
Rule::UnusedStaticMethodArgument,
Rule::UnusedLambdaArgument,
]) {
flake8_unused_arguments::rules::unused_arguments(
self,
scope,
&mut diagnostics,
);
}
}
}
if self.enabled(Rule::UnusedImport) {
pyflakes::rules::unused_import(self, scope, &mut diagnostics);
if matches!(
scope.kind,
ScopeKind::Function(_) | ScopeKind::AsyncFunction(_) | ScopeKind::Module
) {
if enforce_typing_imports {
let runtime_imports: Vec<&Binding> =
if self.settings.flake8_type_checking.strict {
vec![]
} else {
self.semantic
.scopes
.ancestor_ids(scope_id)
.flat_map(|scope_id| runtime_imports[scope_id.as_usize()].iter())
.copied()
.collect()
};
if self.enabled(Rule::RuntimeImportInTypeCheckingBlock) {
flake8_type_checking::rules::runtime_import_in_type_checking_block(
self,
scope,
&mut diagnostics,
);
}
if self.any_enabled(&[
Rule::TypingOnlyFirstPartyImport,
Rule::TypingOnlyThirdPartyImport,
Rule::TypingOnlyStandardLibraryImport,
]) {
flake8_type_checking::rules::typing_only_runtime_import(
self,
scope,
&runtime_imports,
&mut diagnostics,
);
}
}
if self.enabled(Rule::UnusedImport) {
pyflakes::rules::unused_import(self, scope, &mut diagnostics);
}
}
}
self.diagnostics.extend(diagnostics);
@ -5460,7 +5453,6 @@ pub(crate) fn check_ast(
checker.check_deferred_future_type_definitions();
let allocator = typed_arena::Arena::new();
checker.check_deferred_string_type_definitions(&allocator);
checker.check_deferred_assignments();
checker.check_deferred_for_loops();
// Check docstrings, exports, bindings, and unresolved references.

168
crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs
View File

@ -12,40 +12,7 @@ use ruff_python_semantic::{Scope, ScopeKind, SemanticModel};
use crate::checkers::ast::Checker;
use crate::registry::Rule;
use super::super::helpers;
/// An AST node that can contain arguments.
#[derive(Debug, Copy, Clone)]
enum Argumentable {
Function,
Method,
ClassMethod,
StaticMethod,
Lambda,
}
impl Argumentable {
fn check_for(self, name: String) -> DiagnosticKind {
match self {
Self::Function => UnusedFunctionArgument { name }.into(),
Self::Method => UnusedMethodArgument { name }.into(),
Self::ClassMethod => UnusedClassMethodArgument { name }.into(),
Self::StaticMethod => UnusedStaticMethodArgument { name }.into(),
Self::Lambda => UnusedLambdaArgument { name }.into(),
}
}
const fn rule_code(self) -> Rule {
match self {
Self::Function => Rule::UnusedFunctionArgument,
Self::Method => Rule::UnusedMethodArgument,
Self::ClassMethod => Rule::UnusedClassMethodArgument,
Self::StaticMethod => Rule::UnusedStaticMethodArgument,
Self::Lambda => Rule::UnusedLambdaArgument,
}
}
}
use crate::rules::flake8_unused_arguments::helpers;
/// ## What it does
/// Checks for the presence of unused arguments in function definitions.
@ -67,7 +34,7 @@ impl Argumentable {
/// ```
#[violation]
pub struct UnusedFunctionArgument {
pub(super) name: String,
name: String,
}
impl Violation for UnusedFunctionArgument {
@ -100,7 +67,7 @@ impl Violation for UnusedFunctionArgument {
/// ```
#[violation]
pub struct UnusedMethodArgument {
pub(super) name: String,
name: String,
}
impl Violation for UnusedMethodArgument {
@ -135,7 +102,7 @@ impl Violation for UnusedMethodArgument {
/// ```
#[violation]
pub struct UnusedClassMethodArgument {
pub(super) name: String,
name: String,
}
impl Violation for UnusedClassMethodArgument {
@ -170,7 +137,7 @@ impl Violation for UnusedClassMethodArgument {
/// ```
#[violation]
pub struct UnusedStaticMethodArgument {
pub(super) name: String,
name: String,
}
impl Violation for UnusedStaticMethodArgument {
@ -202,7 +169,7 @@ impl Violation for UnusedStaticMethodArgument {
/// ```
#[violation]
pub struct UnusedLambdaArgument {
pub(super) name: String,
name: String,
}
impl Violation for UnusedLambdaArgument {
@ -213,6 +180,38 @@ impl Violation for UnusedLambdaArgument {
}
}
/// An AST node that can contain arguments.
#[derive(Debug, Copy, Clone)]
enum Argumentable {
Function,
Method,
ClassMethod,
StaticMethod,
Lambda,
}
impl Argumentable {
fn check_for(self, name: String) -> DiagnosticKind {
match self {
Self::Function => UnusedFunctionArgument { name }.into(),
Self::Method => UnusedMethodArgument { name }.into(),
Self::ClassMethod => UnusedClassMethodArgument { name }.into(),
Self::StaticMethod => UnusedStaticMethodArgument { name }.into(),
Self::Lambda => UnusedLambdaArgument { name }.into(),
}
}
const fn rule_code(self) -> Rule {
match self {
Self::Function => Rule::UnusedFunctionArgument,
Self::Method => Rule::UnusedMethodArgument,
Self::ClassMethod => Rule::UnusedClassMethodArgument,
Self::StaticMethod => Rule::UnusedStaticMethodArgument,
Self::Lambda => Rule::UnusedLambdaArgument,
}
}
}
/// Check a plain function for unused arguments.
fn function(
argumentable: Argumentable,
@ -221,7 +220,8 @@ fn function(
semantic: &SemanticModel,
dummy_variable_rgx: &Regex,
ignore_variadic_names: bool,
) -> Vec<Diagnostic> {
diagnostics: &mut Vec<Diagnostic>,
) {
let args = args
.posonlyargs
.iter()
@ -238,7 +238,14 @@ fn function(
.flatten()
.skip(usize::from(ignore_variadic_names)),
);
call(argumentable, args, values, semantic, dummy_variable_rgx)
call(
argumentable,
args,
values,
semantic,
dummy_variable_rgx,
diagnostics,
);
}
/// Check a method for unused arguments.
@ -249,7 +256,8 @@ fn method(
semantic: &SemanticModel,
dummy_variable_rgx: &Regex,
ignore_variadic_names: bool,
) -> Vec<Diagnostic> {
diagnostics: &mut Vec<Diagnostic>,
) {
let args = args
.posonlyargs
.iter()
@ -267,7 +275,14 @@ fn method(
.flatten()
.skip(usize::from(ignore_variadic_names)),
);
call(argumentable, args, values, semantic, dummy_variable_rgx)
call(
argumentable,
args,
values,
semantic,
dummy_variable_rgx,
diagnostics,
);
}
fn call<'a>(
@ -276,33 +291,39 @@ fn call<'a>(
values: &Scope,
semantic: &SemanticModel,
dummy_variable_rgx: &Regex,
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];
for arg in args {
if let Some(binding) = values
diagnostics: &mut Vec<Diagnostic>,
) {
diagnostics.extend(args.filter_map(|arg| {
let binding = values
.get(arg.arg.as_str())
.map(|binding_id| semantic.binding(binding_id))
.map(|binding_id| semantic.binding(binding_id))?;
if binding.kind.is_argument()
&& !binding.is_used()
&& !dummy_variable_rgx.is_match(arg.arg.as_str())
{
if binding.kind.is_argument()
&& !binding.is_used()
&& !dummy_variable_rgx.is_match(arg.arg.as_str())
{
diagnostics.push(Diagnostic::new(
argumentable.check_for(arg.arg.to_string()),
binding.range,
));
}
Some(Diagnostic::new(
argumentable.check_for(arg.arg.to_string()),
binding.range,
))
} else {
None
}
}
diagnostics
}));
}
/// ARG001, ARG002, ARG003, ARG004, ARG005
pub(crate) fn unused_arguments(
checker: &Checker,
parent: &Scope,
scope: &Scope,
) -> Vec<Diagnostic> {
diagnostics: &mut Vec<Diagnostic>,
) {
let Some(parent) = scope
.parent
.map(|scope_id| &checker.semantic().scopes[scope_id])
else {
return;
};
match &scope.kind {
ScopeKind::Function(ast::StmtFunctionDef {
name,
@ -340,9 +361,8 @@ pub(crate) fn unused_arguments(
.settings
.flake8_unused_arguments
.ignore_variadic_names,
)
} else {
vec![]
diagnostics,
);
}
}
function_type::FunctionType::Method => {
@ -366,9 +386,8 @@ pub(crate) fn unused_arguments(
.settings
.flake8_unused_arguments
.ignore_variadic_names,
)
} else {
vec![]
diagnostics,
);
}
}
function_type::FunctionType::ClassMethod => {
@ -392,9 +411,8 @@ pub(crate) fn unused_arguments(
.settings
.flake8_unused_arguments
.ignore_variadic_names,
)
} else {
vec![]
diagnostics,
);
}
}
function_type::FunctionType::StaticMethod => {
@ -418,9 +436,8 @@ pub(crate) fn unused_arguments(
.settings
.flake8_unused_arguments
.ignore_variadic_names,
)
} else {
vec![]
diagnostics,
);
}
}
}
@ -437,9 +454,8 @@ pub(crate) fn unused_arguments(
.settings
.flake8_unused_arguments
.ignore_variadic_names,
)
} else {
vec![]
diagnostics,
);
}
}
_ => panic!("Expected ScopeKind::Function | ScopeKind::Lambda"),

2
crates/ruff/src/rules/pyflakes/mod.rs
View File

@ -1027,8 +1027,8 @@ mod tests {
&[
Rule::UndefinedName,
Rule::UndefinedName,
Rule::UnusedVariable,
Rule::UndefinedName,
Rule::UnusedVariable,
Rule::UndefinedName,
],
);

41
crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs
View File

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::ScopeId;
use ruff_python_semantic::Scope;
use crate::checkers::ast::Checker;
@ -33,27 +33,22 @@ impl Violation for UnusedAnnotation {
}
/// F842
pub(crate) fn unused_annotation(checker: &mut Checker, scope: ScopeId) {
let scope = &checker.semantic().scopes[scope];
let bindings: Vec<_> = scope
.bindings()
.filter_map(|(name, binding_id)| {
let binding = checker.semantic().binding(binding_id);
if binding.kind.is_annotation()
&& !binding.is_used()
&& !checker.settings.dummy_variable_rgx.is_match(name)
{
Some((name.to_string(), binding.range))
} else {
None
}
})
.collect();
for (name, range) in bindings {
checker
.diagnostics
.push(Diagnostic::new(UnusedAnnotation { name }, range));
pub(crate) fn unused_annotation(
checker: &Checker,
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
for (name, range) in scope.bindings().filter_map(|(name, binding_id)| {
let binding = checker.semantic().binding(binding_id);
if binding.kind.is_annotation()
&& !binding.is_used()
&& !checker.settings.dummy_variable_rgx.is_match(name)
{
Some((name.to_string(), binding.range))
} else {
None
}
}) {
diagnostics.push(Diagnostic::new(UnusedAnnotation { name }, range));
}
}

13
crates/ruff/src/rules/pyflakes/rules/unused_variable.rs
View File

@ -7,7 +7,7 @@ use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::contains_effect;
use ruff_python_ast::source_code::Locator;
use ruff_python_semantic::ScopeId;
use ruff_python_semantic::Scope;
use crate::autofix::edits::delete_stmt;
use crate::checkers::ast::Checker;
@ -272,13 +272,12 @@ fn remove_unused_variable(
}
/// F841
pub(crate) fn unused_variable(checker: &mut Checker, scope: ScopeId) {
let scope = &checker.semantic().scopes[scope];
pub(crate) fn unused_variable(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
if scope.uses_locals() && scope.kind.is_any_function() {
return;
}
let bindings: Vec<_> = scope
for (name, range, source) in scope
.bindings()
.map(|(name, binding_id)| (name, checker.semantic().binding(binding_id)))
.filter_map(|(name, binding)| {
@ -297,9 +296,7 @@ pub(crate) fn unused_variable(checker: &mut Checker, scope: ScopeId) {
None
})
.collect();
for (name, range, source) in bindings {
{
let mut diagnostic = Diagnostic::new(UnusedVariable { name }, range);
if checker.patch(diagnostic.kind.rule()) {
if let Some(source) = source {
@ -310,6 +307,6 @@ pub(crate) fn unused_variable(checker: &mut Checker, scope: ScopeId) {
}
}
}
checker.diagnostics.push(diagnostic);
diagnostics.push(diagnostic);
}
}

16
crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__augmented_assignment_after_del.snap
View File

@ -1,6 +1,14 @@
---
source: crates/ruff/src/rules/pyflakes/mod.rs
---
<filename>:10:5: F821 Undefined name `x`
|
8 | # entirely after the `del` statement. However, it should be an F821
9 | # error, because the name is defined in the scope, but unbound.
10 | x += 1
| ^ F821
|
<filename>:10:5: F841 Local variable `x` is assigned to but never used
|
8 | # entirely after the `del` statement. However, it should be an F821
@ -10,12 +18,4 @@ source: crates/ruff/src/rules/pyflakes/mod.rs
|
= help: Remove assignment to unused variable `x`
<filename>:10:5: F821 Undefined name `x`
|
8 | # entirely after the `del` statement. However, it should be an F821
9 | # error, because the name is defined in the scope, but unbound.
10 | x += 1
| ^ F821
|