diff --git a/LICENSE b/LICENSE index 717c7af4ca..148d7ea172 100644 --- a/LICENSE +++ b/LICENSE @@ -388,6 +388,56 @@ are: SOFTWARE. """ +- flake8-import-conventions, licensed as follows: + """ + MIT License + + Copyright (c) 2021 João Palmeiro + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + """ + +- flake8-unused-arguments, licensed as follows: + """ + MIT License + + Copyright (c) 2019 Nathan Hoad + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + """ + - isort, licensed as follows: """ The MIT License (MIT) diff --git a/README.md b/README.md index 273d285a14..cf031c368f 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,7 @@ of [Conda](https://docs.conda.io/en/latest/): 1. [flake8-quotes (Q)](#flake8-quotes-q) 1. [flake8-return (RET)](#flake8-return-ret) 1. [flake8-tidy-imports (I25)](#flake8-tidy-imports-i25) + 1. [flake8-unused-arguments (ARG)](#flake8-unused-arguments-arg) 1. [eradicate (ERA)](#eradicate-era) 1. [pygrep-hooks (PGH)](#pygrep-hooks-pgh) 1. [Pylint (PLC, PLE, PLR, PLW)](#pylint-plc-ple-plr-plw) @@ -768,6 +769,18 @@ For more, see [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports | ---- | ---- | ------- | --- | | I252 | BannedRelativeImport | Relative imports are banned | | +### flake8-unused-arguments (ARG) + +For more, see [flake8-unused-arguments](https://pypi.org/project/flake8-unused-arguments/0.0.12/) on PyPI. + +| Code | Name | Message | Fix | +| ---- | ---- | ------- | --- | +| ARG001 | UnusedFunctionArgument | Unused function argument: `...` | | +| ARG002 | UnusedMethodArgument | Unused method argument: `...` | | +| ARG003 | UnusedClassMethodArgument | Unused class method argument: `...` | | +| ARG004 | UnusedStaticMethodArgument | Unused static method argument: `...` | | +| ARG005 | UnusedLambdaArgument | Unused lambda argument: `...` | | + ### eradicate (ERA) For more, see [eradicate](https://pypi.org/project/eradicate/2.1.0/) on PyPI. diff --git a/resources/test/fixtures/flake8_unused_arguments/ARG.py b/resources/test/fixtures/flake8_unused_arguments/ARG.py new file mode 100644 index 0000000000..e28916ebdd --- /dev/null +++ b/resources/test/fixtures/flake8_unused_arguments/ARG.py @@ -0,0 +1,137 @@ +from abc import abstractmethod +from typing_extensions import override + + +### +# Unused arguments on functions. +### +def f(self, x): + print("Hello, world!") + + +def f(cls, x): + print("Hello, world!") + + +def f(self, x): + ... + + +def f(cls, x): + ... + + +### +# Unused arguments on lambdas. +### +lambda x: print("Hello, world!") + + +class X: + ### + # Unused arguments. + ### + def f(self, x): + print("Hello, world!") + + def f(self, /, x): + print("Hello, world!") + + def f(cls, x): + print("Hello, world!") + + @classmethod + def f(cls, x): + print("Hello, world!") + + @staticmethod + def f(cls, x): + print("Hello, world!") + + @staticmethod + def f(x): + print("Hello, world!") + + ### + # Unused arguments attached to empty functions (OK). + ### + def f(self, x): + ... + + def f(self, /, x): + ... + + def f(cls, x): + ... + + @classmethod + def f(cls, x): + ... + + @staticmethod + def f(cls, x): + ... + + @staticmethod + def f(x): + ... + + ### + # Unused functions attached to abstract methods (OK). + ### + @abstractmethod + def f(self, x): + print("Hello, world!") + + @abstractmethod + def f(self, /, x): + print("Hello, world!") + + @abstractmethod + def f(cls, x): + print("Hello, world!") + + @classmethod + @abstractmethod + def f(cls, x): + print("Hello, world!") + + @staticmethod + @abstractmethod + def f(cls, x): + print("Hello, world!") + + @staticmethod + @abstractmethod + def f(x): + print("Hello, world!") + + ### + # Unused functions attached to overrides (OK). + ### + @override + def f(self, x): + print("Hello, world!") + + @override + def f(self, /, x): + print("Hello, world!") + + @override + def f(cls, x): + print("Hello, world!") + + @classmethod + @override + def f(cls, x): + print("Hello, world!") + + @staticmethod + @override + def f(cls, x): + print("Hello, world!") + + @staticmethod + @override + def f(x): + print("Hello, world!") diff --git a/src/ast/cast.rs b/src/ast/cast.rs new file mode 100644 index 0000000000..d4bca521dc --- /dev/null +++ b/src/ast/cast.rs @@ -0,0 +1,9 @@ +use rustpython_ast::{Expr, Stmt, StmtKind}; + +pub fn decorator_list(stmt: &Stmt) -> &Vec { + match &stmt.node { + StmtKind::FunctionDef { decorator_list, .. } + | StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list, + _ => panic!("Expected StmtKind::FunctionDef | StmtKind::AsyncFunctionDef"), + } +} diff --git a/src/ast/function_type.rs b/src/ast/function_type.rs new file mode 100644 index 0000000000..07cb9d5f5b --- /dev/null +++ b/src/ast/function_type.rs @@ -0,0 +1,65 @@ +use rustc_hash::{FxHashMap, FxHashSet}; +use rustpython_ast::Expr; + +use crate::ast::helpers::{ + collect_call_paths, dealias_call_path, match_call_path, to_module_and_member, +}; +use crate::ast::types::{Scope, ScopeKind}; + +const CLASS_METHODS: [&str; 3] = ["__new__", "__init_subclass__", "__class_getitem__"]; +const METACLASS_BASES: [(&str, &str); 2] = [("", "type"), ("abc", "ABCMeta")]; + +pub enum FunctionType { + Function, + Method, + ClassMethod, + StaticMethod, +} + +/// Classify a function based on its scope, name, and decorators. +pub fn classify( + scope: &Scope, + name: &str, + decorator_list: &[Expr], + from_imports: &FxHashMap<&str, FxHashSet<&str>>, + import_aliases: &FxHashMap<&str, &str>, + classmethod_decorators: &[String], + staticmethod_decorators: &[String], +) -> FunctionType { + let ScopeKind::Class(scope) = &scope.kind else { + return FunctionType::Function; + }; + // Special-case class method, like `__new__`. + if CLASS_METHODS.contains(&name) + || scope.bases.iter().any(|expr| { + // The class itself extends a known metaclass, so all methods are class methods. + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); + METACLASS_BASES + .iter() + .any(|(module, member)| match_call_path(&call_path, module, member, from_imports)) + }) + || decorator_list.iter().any(|expr| { + // The method is decorated with a class method decorator (like `@classmethod`). + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); + classmethod_decorators.iter().any(|decorator| { + let (module, member) = to_module_and_member(decorator); + match_call_path(&call_path, module, member, from_imports) + }) + }) + { + FunctionType::ClassMethod + } else if decorator_list.iter().any(|expr| { + // The method is decorated with a static method decorator (like + // `@staticmethod`). + let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); + staticmethod_decorators.iter().any(|decorator| { + let (module, member) = to_module_and_member(decorator); + match_call_path(&call_path, module, member, from_imports) + }) + }) { + FunctionType::StaticMethod + } else { + // It's an instance method. + FunctionType::Method + } +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 9bdb26a1c7..9e807a28df 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -1,3 +1,5 @@ +pub mod cast; +pub mod function_type; pub mod helpers; pub mod operations; pub mod relocate; diff --git a/src/ast/types.rs b/src/ast/types.rs index d8552a320a..2de88c5ce8 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -1,7 +1,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use rustc_hash::FxHashMap; -use rustpython_ast::{Expr, Keyword, Stmt}; +use rustpython_ast::{Arguments, Expr, Keyword, Stmt}; use rustpython_parser::ast::{Located, Location}; fn id() -> usize { @@ -30,35 +30,49 @@ impl Range { } } -#[derive(Clone, Debug)] -pub struct FunctionScope { +#[derive(Debug)] +pub struct FunctionDef<'a> { + pub name: &'a str, + pub args: &'a Arguments, + pub body: &'a [Stmt], + pub decorator_list: &'a [Expr], + // pub returns: Option<&'a Expr>, + // pub type_comment: Option<&'a str>, + // TODO(charlie): Create AsyncFunctionDef to mirror the AST. pub async_: bool, - pub uses_locals: bool, } -#[derive(Clone, Debug)] -pub struct ClassScope<'a> { +#[derive(Debug)] +pub struct ClassDef<'a> { pub name: &'a str, pub bases: &'a [Expr], pub keywords: &'a [Keyword], + // pub body: &'a [Stmt], pub decorator_list: &'a [Expr], } -#[derive(Clone, Debug)] +#[derive(Debug)] +pub struct Lambda<'a> { + pub args: &'a Arguments, + pub body: &'a Expr, +} + +#[derive(Debug)] pub enum ScopeKind<'a> { - Class(ClassScope<'a>), - Function(FunctionScope), + Class(ClassDef<'a>), + Function(FunctionDef<'a>), Generator, Module, Arg, - Lambda, + Lambda(Lambda<'a>), } -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct Scope<'a> { pub id: usize, pub kind: ScopeKind<'a>, pub import_starred: bool, + pub uses_locals: bool, pub values: FxHashMap<&'a str, Binding>, } @@ -68,6 +82,7 @@ impl<'a> Scope<'a> { id: id(), kind, import_starred: false, + uses_locals: false, values: FxHashMap::default(), } } diff --git a/src/check_ast.rs b/src/check_ast.rs index a582a2f8f0..5f0f8b1547 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -17,7 +17,8 @@ use crate::ast::helpers::{ use crate::ast::operations::extract_all_names; use crate::ast::relocate::relocate_expr; use crate::ast::types::{ - Binding, BindingContext, BindingKind, ClassScope, FunctionScope, Node, Range, Scope, ScopeKind, + Binding, BindingContext, BindingKind, ClassDef, FunctionDef, Lambda, Node, Range, Scope, + ScopeKind, }; use crate::ast::visitor::{walk_excepthandler, Visitor}; use crate::ast::{helpers, operations, visitor}; @@ -35,8 +36,9 @@ use crate::visibility::{module_visibility, transition_scope, Modifier, Visibilit use crate::{ docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_debugger, - flake8_import_conventions, flake8_print, flake8_return, flake8_tidy_imports, mccabe, - pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, + flake8_import_conventions, flake8_print, flake8_return, flake8_tidy_imports, + flake8_unused_arguments, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, + pylint, pyupgrade, }; const GLOBAL_SCOPE_INDEX: usize = 0; @@ -71,7 +73,7 @@ pub struct Checker<'a> { deferred_type_definitions: Vec<(&'a Expr, bool, DeferralContext)>, deferred_functions: Vec<(&'a Stmt, DeferralContext, VisibleScope)>, deferred_lambdas: Vec<(&'a Expr, DeferralContext)>, - deferred_assignments: Vec, + deferred_assignments: Vec<(usize, DeferralContext)>, // Internal, derivative state. visible_scope: VisibleScope, in_f_string: Option, @@ -580,7 +582,7 @@ where for expr in decorator_list { self.visit_expr(expr); } - self.push_scope(Scope::new(ScopeKind::Class(ClassScope { + self.push_scope(Scope::new(ScopeKind::Class(ClassDef { name, bases, keywords, @@ -1406,20 +1408,18 @@ where } Ok(summary) => { if self.settings.enabled.contains(&CheckCode::F522) { - if let Some(check) = - pyflakes::checks::string_dot_format_extra_named_arguments( - &summary, keywords, location, - ) + if let Some(check) = pyflakes::checks::string_dot_format_extra_named_arguments( + &summary, keywords, location, + ) { self.add_check(check); } } if self.settings.enabled.contains(&CheckCode::F523) { - if let Some(check) = - pyflakes::checks::string_dot_format_extra_positional_arguments( - &summary, args, location, - ) + if let Some(check) = pyflakes::checks::string_dot_format_extra_positional_arguments( + &summary, args, location, + ) { self.add_check(check); } @@ -1486,7 +1486,7 @@ where .scope_stack .iter() .rev() - .any(|index| matches!(self.scopes[*index].kind, ScopeKind::Lambda)) + .any(|index| matches!(self.scopes[*index].kind, ScopeKind::Lambda(..))) { flake8_bugbear::plugins::setattr_with_constant(self, expr, func, args); } @@ -1747,9 +1747,7 @@ where if id == "locals" && matches!(ctx, ExprContext::Load) { let scope = &mut self.scopes [*(self.scope_stack.last().expect("No current scope found"))]; - if let ScopeKind::Function(inner) = &mut scope.kind { - inner.uses_locals = true; - } + scope.uses_locals = true; } } @@ -2070,7 +2068,7 @@ where } } } - ExprKind::Lambda { args, .. } => { + ExprKind::Lambda { args, body, .. } => { // Visit the arguments, but avoid the body, which will be deferred. for arg in &args.posonlyargs { if let Some(expr) = &arg.node.annotation { @@ -2103,7 +2101,7 @@ where for expr in &args.defaults { self.visit_expr(expr); } - self.push_scope(Scope::new(ScopeKind::Lambda)); + self.push_scope(Scope::new(ScopeKind::Lambda(Lambda { args, body }))); } ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { if self.settings.enabled.contains(&CheckCode::C416) { @@ -2958,27 +2956,44 @@ impl<'a> Checker<'a> { fn check_deferred_functions(&mut self) { while let Some((stmt, (scopes, parents), visibility)) = self.deferred_functions.pop() { - self.scope_stack = scopes; - self.parent_stack = parents; + self.scope_stack = scopes.clone(); + self.parent_stack = parents.clone(); self.visible_scope = visibility; - self.push_scope(Scope::new(ScopeKind::Function(FunctionScope { - async_: matches!(stmt.node, StmtKind::AsyncFunctionDef { .. }), - uses_locals: false, - }))); match &stmt.node { - StmtKind::FunctionDef { body, args, .. } - | StmtKind::AsyncFunctionDef { body, args, .. } => { + StmtKind::FunctionDef { + name, + body, + args, + decorator_list, + .. + } + | StmtKind::AsyncFunctionDef { + name, + body, + args, + decorator_list, + .. + } => { + self.push_scope(Scope::new(ScopeKind::Function(FunctionDef { + name, + body, + args, + decorator_list, + async_: matches!(stmt.node, StmtKind::AsyncFunctionDef { .. }), + }))); self.visit_arguments(args); for stmt in body { self.visit_stmt(stmt); } } - _ => {} + _ => unreachable!("Expected StmtKind::FunctionDef | StmtKind::AsyncFunctionDef"), } - self.deferred_assignments - .push(*self.scope_stack.last().expect("No current scope found")); + self.deferred_assignments.push(( + *self.scope_stack.last().expect("No current scope found"), + (scopes, parents), + )); self.pop_scope(); } @@ -2986,25 +3001,29 @@ impl<'a> Checker<'a> { fn check_deferred_lambdas(&mut self) { while let Some((expr, (scopes, parents))) = self.deferred_lambdas.pop() { - self.scope_stack = scopes; - self.parent_stack = parents; - self.push_scope(Scope::new(ScopeKind::Lambda)); + self.scope_stack = scopes.clone(); + self.parent_stack = parents.clone(); if let ExprKind::Lambda { args, body } = &expr.node { + self.push_scope(Scope::new(ScopeKind::Lambda(Lambda { args, body }))); self.visit_arguments(args); self.visit_expr(body); + } else { + unreachable!("Expected ExprKind::Lambda"); } - self.deferred_assignments - .push(*self.scope_stack.last().expect("No current scope found")); + self.deferred_assignments.push(( + *self.scope_stack.last().expect("No current scope found"), + (scopes, parents), + )); self.pop_scope(); } } fn check_deferred_assignments(&mut self) { - if self.settings.enabled.contains(&CheckCode::F841) { - while let Some(index) = self.deferred_assignments.pop() { + while let Some((index, (scopes, _parents))) = self.deferred_assignments.pop() { + if self.settings.enabled.contains(&CheckCode::F841) { self.add_checks( pyflakes::checks::unused_variables( &self.scopes[index], @@ -3013,6 +3032,23 @@ impl<'a> Checker<'a> { .into_iter(), ); } + if self.settings.enabled.contains(&CheckCode::ARG001) + || self.settings.enabled.contains(&CheckCode::ARG002) + || self.settings.enabled.contains(&CheckCode::ARG003) + || self.settings.enabled.contains(&CheckCode::ARG004) + || self.settings.enabled.contains(&CheckCode::ARG005) + { + self.add_checks( + flake8_unused_arguments::plugins::unused_arguments( + self, + &self.scopes[*scopes + .last() + .expect("Expected parent scope above function scope")], + &self.scopes[index], + ) + .into_iter(), + ); + } } } @@ -3092,7 +3128,7 @@ impl<'a> Checker<'a> { for (name, binding) in &scope.values { let (BindingKind::Importation(_, full_name, context) | BindingKind::SubmoduleImportation(_, full_name, context) - | BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue }; + | BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue; }; // Skip used exports from `__all__` if binding.used.is_some() diff --git a/src/checks.rs b/src/checks.rs index a5ab1282e7..81901366a4 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -292,6 +292,12 @@ pub enum CheckCode { FBT001, FBT002, FBT003, + // flake8-unused-arguments + ARG001, + ARG002, + ARG003, + ARG004, + ARG005, // flake8-import-conventions ICN001, // Ruff @@ -326,6 +332,7 @@ pub enum CheckCategory { Flake8Quotes, Flake8Return, Flake8TidyImports, + Flake8UnusedArguments, Eradicate, PygrepHooks, Pylint, @@ -364,6 +371,7 @@ impl CheckCategory { CheckCategory::Flake8Quotes => "flake8-quotes", CheckCategory::Flake8Return => "flake8-return", CheckCategory::Flake8TidyImports => "flake8-tidy-imports", + CheckCategory::Flake8UnusedArguments => "flake8-unused-arguments", CheckCategory::Isort => "isort", CheckCategory::McCabe => "mccabe", CheckCategory::PEP8Naming => "pep8-naming", @@ -393,6 +401,7 @@ impl CheckCategory { CheckCategory::Flake8Quotes => vec![CheckCodePrefix::Q], CheckCategory::Flake8Return => vec![CheckCodePrefix::RET], CheckCategory::Flake8TidyImports => vec![CheckCodePrefix::I25], + CheckCategory::Flake8UnusedArguments => vec![CheckCodePrefix::ARG], CheckCategory::Isort => vec![CheckCodePrefix::I00], CheckCategory::McCabe => vec![CheckCodePrefix::C90], CheckCategory::PEP8Naming => vec![CheckCodePrefix::N], @@ -470,6 +479,10 @@ impl CheckCategory { "https://pypi.org/project/flake8-tidy-imports/4.8.0/", &Platform::PyPI, )), + CheckCategory::Flake8UnusedArguments => Some(( + "https://pypi.org/project/flake8-unused-arguments/0.0.12/", + &Platform::PyPI, + )), CheckCategory::Isort => { Some(("https://pypi.org/project/isort/5.10.1/", &Platform::PyPI)) } @@ -817,6 +830,12 @@ pub enum CheckKind { BooleanPositionalValueInFunctionCall, // pygrep-hooks NoEval, + // flake8-unused-arguments + UnusedFunctionArgument(String), + UnusedMethodArgument(String), + UnusedClassMethodArgument(String), + UnusedStaticMethodArgument(String), + UnusedLambdaArgument(String), // flake8-import-conventions ImportAliasIsNotConventional(String, String), // Ruff @@ -1159,6 +1178,12 @@ impl CheckCode { CheckCode::FBT003 => CheckKind::BooleanPositionalValueInFunctionCall, // pygrep-hooks CheckCode::PGH001 => CheckKind::NoEval, + // flake8-unused-arguments + CheckCode::ARG001 => CheckKind::UnusedFunctionArgument("...".to_string()), + CheckCode::ARG002 => CheckKind::UnusedMethodArgument("...".to_string()), + CheckCode::ARG003 => CheckKind::UnusedClassMethodArgument("...".to_string()), + CheckCode::ARG004 => CheckKind::UnusedStaticMethodArgument("...".to_string()), + CheckCode::ARG005 => CheckKind::UnusedLambdaArgument("...".to_string()), // flake8-import-conventions CheckCode::ICN001 => { CheckKind::ImportAliasIsNotConventional("...".to_string(), "...".to_string()) @@ -1188,6 +1213,11 @@ impl CheckCode { CheckCode::ANN205 => CheckCategory::Flake8Annotations, CheckCode::ANN206 => CheckCategory::Flake8Annotations, CheckCode::ANN401 => CheckCategory::Flake8Annotations, + CheckCode::ARG001 => CheckCategory::Flake8UnusedArguments, + CheckCode::ARG002 => CheckCategory::Flake8UnusedArguments, + CheckCode::ARG003 => CheckCategory::Flake8UnusedArguments, + CheckCode::ARG004 => CheckCategory::Flake8UnusedArguments, + CheckCode::ARG005 => CheckCategory::Flake8UnusedArguments, CheckCode::B002 => CheckCategory::Flake8Bugbear, CheckCode::B003 => CheckCategory::Flake8Bugbear, CheckCode::B004 => CheckCategory::Flake8Bugbear, @@ -1339,8 +1369,8 @@ impl CheckCode { CheckCode::FBT002 => CheckCategory::Flake8BooleanTrap, CheckCode::FBT003 => CheckCategory::Flake8BooleanTrap, CheckCode::I001 => CheckCategory::Isort, - CheckCode::ICN001 => CheckCategory::Flake8ImportConventions, CheckCode::I252 => CheckCategory::Flake8TidyImports, + CheckCode::ICN001 => CheckCategory::Flake8ImportConventions, CheckCode::N801 => CheckCategory::PEP8Naming, CheckCode::N802 => CheckCategory::PEP8Naming, CheckCode::N803 => CheckCategory::PEP8Naming, @@ -1686,6 +1716,12 @@ impl CheckKind { CheckKind::BooleanPositionalValueInFunctionCall => &CheckCode::FBT003, // pygrep-hooks CheckKind::NoEval => &CheckCode::PGH001, + // flake8-unused-arguments + CheckKind::UnusedFunctionArgument(..) => &CheckCode::ARG001, + CheckKind::UnusedMethodArgument(..) => &CheckCode::ARG002, + CheckKind::UnusedClassMethodArgument(..) => &CheckCode::ARG003, + CheckKind::UnusedStaticMethodArgument(..) => &CheckCode::ARG004, + CheckKind::UnusedLambdaArgument(..) => &CheckCode::ARG005, // flake8-import-conventions CheckKind::ImportAliasIsNotConventional(..) => &CheckCode::ICN001, // Ruff @@ -2477,6 +2513,18 @@ impl CheckKind { } // pygrep-hooks CheckKind::NoEval => "No builtin `eval()` allowed".to_string(), + // flake8-unused-arguments + CheckKind::UnusedFunctionArgument(name) => { + format!("Unused function argument: `{name}`") + } + CheckKind::UnusedMethodArgument(name) => format!("Unused method argument: `{name}`"), + CheckKind::UnusedClassMethodArgument(name) => { + format!("Unused class method argument: `{name}`") + } + CheckKind::UnusedStaticMethodArgument(name) => { + format!("Unused static method argument: `{name}`") + } + CheckKind::UnusedLambdaArgument(name) => format!("Unused lambda argument: `{name}`"), // flake8-import-conventions CheckKind::ImportAliasIsNotConventional(name, asname) => { format!("`{name}` should be imported as `{asname}`") diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 0f5b96e35c..75b3a762ee 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -36,6 +36,14 @@ pub enum CheckCodePrefix { ANN4, ANN40, ANN401, + ARG, + ARG0, + ARG00, + ARG001, + ARG002, + ARG003, + ARG004, + ARG005, B, B0, B00, @@ -492,6 +500,32 @@ impl CheckCodePrefix { CheckCodePrefix::ANN4 => vec![CheckCode::ANN401], CheckCodePrefix::ANN40 => vec![CheckCode::ANN401], CheckCodePrefix::ANN401 => vec![CheckCode::ANN401], + CheckCodePrefix::ARG => vec![ + CheckCode::ARG001, + CheckCode::ARG002, + CheckCode::ARG003, + CheckCode::ARG004, + CheckCode::ARG005, + ], + CheckCodePrefix::ARG0 => vec![ + CheckCode::ARG001, + CheckCode::ARG002, + CheckCode::ARG003, + CheckCode::ARG004, + CheckCode::ARG005, + ], + CheckCodePrefix::ARG00 => vec![ + CheckCode::ARG001, + CheckCode::ARG002, + CheckCode::ARG003, + CheckCode::ARG004, + CheckCode::ARG005, + ], + CheckCodePrefix::ARG001 => vec![CheckCode::ARG001], + CheckCodePrefix::ARG002 => vec![CheckCode::ARG002], + CheckCodePrefix::ARG003 => vec![CheckCode::ARG003], + CheckCodePrefix::ARG004 => vec![CheckCode::ARG004], + CheckCodePrefix::ARG005 => vec![CheckCode::ARG005], CheckCodePrefix::B => vec![ CheckCode::B002, CheckCode::B003, @@ -1698,6 +1732,14 @@ impl CheckCodePrefix { CheckCodePrefix::ANN4 => SuffixLength::One, CheckCodePrefix::ANN40 => SuffixLength::Two, CheckCodePrefix::ANN401 => SuffixLength::Three, + CheckCodePrefix::ARG => SuffixLength::Zero, + CheckCodePrefix::ARG0 => SuffixLength::One, + CheckCodePrefix::ARG00 => SuffixLength::Two, + CheckCodePrefix::ARG001 => SuffixLength::Three, + CheckCodePrefix::ARG002 => SuffixLength::Three, + CheckCodePrefix::ARG003 => SuffixLength::Three, + CheckCodePrefix::ARG004 => SuffixLength::Three, + CheckCodePrefix::ARG005 => SuffixLength::Three, CheckCodePrefix::B => SuffixLength::Zero, CheckCodePrefix::B0 => SuffixLength::One, CheckCodePrefix::B00 => SuffixLength::Two, @@ -2096,6 +2138,7 @@ impl CheckCodePrefix { pub const CATEGORIES: &[CheckCodePrefix] = &[ CheckCodePrefix::A, CheckCodePrefix::ANN, + CheckCodePrefix::ARG, CheckCodePrefix::B, CheckCodePrefix::BLE, CheckCodePrefix::C, diff --git a/src/flake8_annotations/helpers.rs b/src/flake8_annotations/helpers.rs index 1e2ae2f86e..39dfba9d1d 100644 --- a/src/flake8_annotations/helpers.rs +++ b/src/flake8_annotations/helpers.rs @@ -1,5 +1,6 @@ use rustpython_ast::{Arguments, Expr, Stmt, StmtKind}; +use crate::ast::cast; use crate::check_ast::Checker; use crate::docstrings::definition::{Definition, DefinitionKind}; use crate::visibility; @@ -32,7 +33,7 @@ pub fn overloaded_name(checker: &Checker, definition: &Definition) -> Option bool { + match &body { + [] => true, + // Also allow: raise NotImplementedError, raise NotImplemented + [stmt] => match &stmt.node { + StmtKind::Pass => true, + StmtKind::Expr { value } => match &value.node { + ExprKind::Constant { value, .. } => { + matches!(value, Constant::Str(_) | Constant::Ellipsis) + } + _ => false, + }, + _ => false, + }, + _ => false, + } +} diff --git a/src/flake8_unused_arguments/mod.rs b/src/flake8_unused_arguments/mod.rs new file mode 100644 index 0000000000..a96fb517ea --- /dev/null +++ b/src/flake8_unused_arguments/mod.rs @@ -0,0 +1,35 @@ +mod helpers; +pub mod plugins; +mod types; + +#[cfg(test)] +mod tests { + use std::convert::AsRef; + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::checks::CheckCode; + use crate::linter::test_path; + use crate::settings; + + #[test_case(CheckCode::ARG001, Path::new("ARG.py"); "ARG001")] + #[test_case(CheckCode::ARG002, Path::new("ARG.py"); "ARG002")] + #[test_case(CheckCode::ARG003, Path::new("ARG.py"); "ARG003")] + #[test_case(CheckCode::ARG004, Path::new("ARG.py"); "ARG004")] + #[test_case(CheckCode::ARG005, Path::new("ARG.py"); "ARG005")] + fn checks(check_code: CheckCode, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); + let mut checks = test_path( + Path::new("./resources/test/fixtures/flake8_unused_arguments") + .join(path) + .as_path(), + &settings::Settings::for_rule(check_code), + true, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(snapshot, checks); + Ok(()) + } +} diff --git a/src/flake8_unused_arguments/plugins.rs b/src/flake8_unused_arguments/plugins.rs new file mode 100644 index 0000000000..0ecfba0eb0 --- /dev/null +++ b/src/flake8_unused_arguments/plugins.rs @@ -0,0 +1,184 @@ +use std::iter; + +use regex::Regex; +use rustc_hash::FxHashMap; +use rustpython_ast::{Arg, Arguments}; + +use crate::ast::function_type; +use crate::ast::function_type::FunctionType; +use crate::ast::helpers::collect_arg_names; +use crate::ast::types::{Binding, BindingKind, FunctionDef, Lambda, Scope, ScopeKind}; +use crate::check_ast::Checker; +use crate::flake8_unused_arguments::helpers; +use crate::flake8_unused_arguments::types::Argumentable; +use crate::{visibility, Check}; + +/// Check a plain function for unused arguments. +fn function( + argumentable: &Argumentable, + args: &Arguments, + bindings: &FxHashMap<&str, Binding>, + dummy_variable_rgx: &Regex, +) -> Vec { + let mut checks: Vec = vec![]; + for arg_name in collect_arg_names(args) { + if let Some(binding) = bindings.get(arg_name) { + if binding.used.is_none() + && matches!(binding.kind, BindingKind::Argument) + && !dummy_variable_rgx.is_match(arg_name) + { + checks.push(Check::new( + argumentable.check_for(arg_name.to_string()), + binding.range, + )); + } + } + } + checks +} + +/// Check a method for unused arguments. +fn method( + argumentable: &Argumentable, + args: &Arguments, + bindings: &FxHashMap<&str, Binding>, + dummy_variable_rgx: &Regex, +) -> Vec { + let mut checks: Vec = vec![]; + for arg in args + .posonlyargs + .iter() + .chain(args.args.iter()) + .skip(1) + .chain(args.kwonlyargs.iter()) + .chain(iter::once::>(args.vararg.as_deref()).flatten()) + .chain(iter::once::>(args.kwarg.as_deref()).flatten()) + { + if let Some(binding) = bindings.get(&arg.node.arg.as_str()) { + if binding.used.is_none() + && matches!(binding.kind, BindingKind::Argument) + && !dummy_variable_rgx.is_match(arg.node.arg.as_str()) + { + checks.push(Check::new( + argumentable.check_for(arg.node.arg.to_string()), + binding.range, + )); + } + } + } + checks +} + +/// ARG001, ARG002, ARG003, ARG004, ARG005 +pub fn unused_arguments(checker: &Checker, parent: &Scope, scope: &Scope) -> Vec { + match &scope.kind { + ScopeKind::Function(FunctionDef { + name, + args, + body, + decorator_list, + .. + }) => { + match function_type::classify( + parent, + name, + decorator_list, + &checker.from_imports, + &checker.import_aliases, + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ) { + FunctionType::Function => { + if checker + .settings + .enabled + .contains(Argumentable::Function.check_code()) + { + function( + &Argumentable::Function, + args, + &scope.values, + &checker.settings.dummy_variable_rgx, + ) + } else { + vec![] + } + } + FunctionType::Method => { + if checker + .settings + .enabled + .contains(Argumentable::Method.check_code()) + && !helpers::is_empty(body) + && !visibility::is_abstract(checker, decorator_list) + && !visibility::is_override(checker, decorator_list) + { + method( + &Argumentable::Method, + args, + &scope.values, + &checker.settings.dummy_variable_rgx, + ) + } else { + vec![] + } + } + FunctionType::ClassMethod => { + if checker + .settings + .enabled + .contains(Argumentable::ClassMethod.check_code()) + && !helpers::is_empty(body) + && !visibility::is_abstract(checker, decorator_list) + && !visibility::is_override(checker, decorator_list) + { + method( + &Argumentable::ClassMethod, + args, + &scope.values, + &checker.settings.dummy_variable_rgx, + ) + } else { + vec![] + } + } + FunctionType::StaticMethod => { + if checker + .settings + .enabled + .contains(Argumentable::StaticMethod.check_code()) + && !helpers::is_empty(body) + && !visibility::is_abstract(checker, decorator_list) + && !visibility::is_override(checker, decorator_list) + { + function( + &Argumentable::StaticMethod, + args, + &scope.values, + &checker.settings.dummy_variable_rgx, + ) + } else { + vec![] + } + } + } + } + ScopeKind::Lambda(Lambda { args, .. }) => { + if checker + .settings + .enabled + .contains(Argumentable::Lambda.check_code()) + { + function( + &Argumentable::Lambda, + args, + &scope.values, + &checker.settings.dummy_variable_rgx, + ) + } else { + vec![] + } + } + _ => unreachable!("Expected ScopeKind::Function | ScopeKind::Lambda"), + } +} diff --git a/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG001_ARG.py.snap b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG001_ARG.py.snap new file mode 100644 index 0000000000..33065dd58b --- /dev/null +++ b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG001_ARG.py.snap @@ -0,0 +1,77 @@ +--- +source: src/flake8_unused_arguments/mod.rs +expression: checks +--- +- kind: + UnusedFunctionArgument: self + location: + row: 8 + column: 6 + end_location: + row: 8 + column: 10 + fix: ~ +- kind: + UnusedFunctionArgument: x + location: + row: 8 + column: 12 + end_location: + row: 8 + column: 13 + fix: ~ +- kind: + UnusedFunctionArgument: cls + location: + row: 12 + column: 6 + end_location: + row: 12 + column: 9 + fix: ~ +- kind: + UnusedFunctionArgument: x + location: + row: 12 + column: 11 + end_location: + row: 12 + column: 12 + fix: ~ +- kind: + UnusedFunctionArgument: self + location: + row: 16 + column: 6 + end_location: + row: 16 + column: 10 + fix: ~ +- kind: + UnusedFunctionArgument: x + location: + row: 16 + column: 12 + end_location: + row: 16 + column: 13 + fix: ~ +- kind: + UnusedFunctionArgument: cls + location: + row: 20 + column: 6 + end_location: + row: 20 + column: 9 + fix: ~ +- kind: + UnusedFunctionArgument: x + location: + row: 20 + column: 11 + end_location: + row: 20 + column: 12 + fix: ~ + diff --git a/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG002_ARG.py.snap b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG002_ARG.py.snap new file mode 100644 index 0000000000..c0a9ee37b8 --- /dev/null +++ b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG002_ARG.py.snap @@ -0,0 +1,32 @@ +--- +source: src/flake8_unused_arguments/mod.rs +expression: checks +--- +- kind: + UnusedMethodArgument: x + location: + row: 34 + column: 16 + end_location: + row: 34 + column: 17 + fix: ~ +- kind: + UnusedMethodArgument: x + location: + row: 37 + column: 19 + end_location: + row: 37 + column: 20 + fix: ~ +- kind: + UnusedMethodArgument: x + location: + row: 40 + column: 15 + end_location: + row: 40 + column: 16 + fix: ~ + diff --git a/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG003_ARG.py.snap b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG003_ARG.py.snap new file mode 100644 index 0000000000..71b7743912 --- /dev/null +++ b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG003_ARG.py.snap @@ -0,0 +1,14 @@ +--- +source: src/flake8_unused_arguments/mod.rs +expression: checks +--- +- kind: + UnusedClassMethodArgument: x + location: + row: 44 + column: 15 + end_location: + row: 44 + column: 16 + fix: ~ + diff --git a/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG004_ARG.py.snap b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG004_ARG.py.snap new file mode 100644 index 0000000000..de63f5c54c --- /dev/null +++ b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG004_ARG.py.snap @@ -0,0 +1,32 @@ +--- +source: src/flake8_unused_arguments/mod.rs +expression: checks +--- +- kind: + UnusedStaticMethodArgument: cls + location: + row: 48 + column: 10 + end_location: + row: 48 + column: 13 + fix: ~ +- kind: + UnusedStaticMethodArgument: x + location: + row: 48 + column: 15 + end_location: + row: 48 + column: 16 + fix: ~ +- kind: + UnusedStaticMethodArgument: x + location: + row: 52 + column: 10 + end_location: + row: 52 + column: 11 + fix: ~ + diff --git a/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG005_ARG.py.snap b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG005_ARG.py.snap new file mode 100644 index 0000000000..1b97ee045f --- /dev/null +++ b/src/flake8_unused_arguments/snapshots/ruff__flake8_unused_arguments__tests__ARG005_ARG.py.snap @@ -0,0 +1,14 @@ +--- +source: src/flake8_unused_arguments/mod.rs +expression: checks +--- +- kind: + UnusedLambdaArgument: x + location: + row: 27 + column: 7 + end_location: + row: 27 + column: 8 + fix: ~ + diff --git a/src/flake8_unused_arguments/types.rs b/src/flake8_unused_arguments/types.rs new file mode 100644 index 0000000000..c39078e153 --- /dev/null +++ b/src/flake8_unused_arguments/types.rs @@ -0,0 +1,32 @@ +use crate::checks::{CheckCode, CheckKind}; + +/// An AST node that can contain arguments. +pub enum Argumentable { + Function, + Method, + ClassMethod, + StaticMethod, + Lambda, +} + +impl Argumentable { + pub fn check_for(&self, name: String) -> CheckKind { + match self { + Argumentable::Function => CheckKind::UnusedFunctionArgument(name), + Argumentable::Method => CheckKind::UnusedMethodArgument(name), + Argumentable::ClassMethod => CheckKind::UnusedClassMethodArgument(name), + Argumentable::StaticMethod => CheckKind::UnusedStaticMethodArgument(name), + Argumentable::Lambda => CheckKind::UnusedLambdaArgument(name), + } + } + + pub fn check_code(&self) -> &CheckCode { + match self { + Argumentable::Function => &CheckCode::ARG001, + Argumentable::Method => &CheckCode::ARG002, + Argumentable::ClassMethod => &CheckCode::ARG003, + Argumentable::StaticMethod => &CheckCode::ARG004, + Argumentable::Lambda => &CheckCode::ARG005, + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 3ed63d3c7c..a167bee9e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ mod flake8_print; pub mod flake8_quotes; mod flake8_return; pub mod flake8_tidy_imports; +mod flake8_unused_arguments; pub mod fs; mod isort; mod lex; diff --git a/src/pep8_naming/checks.rs b/src/pep8_naming/checks.rs index f985cb48dc..50cac7c24f 100644 --- a/src/pep8_naming/checks.rs +++ b/src/pep8_naming/checks.rs @@ -1,10 +1,10 @@ use rustc_hash::{FxHashMap, FxHashSet}; use rustpython_ast::{Arguments, Expr, ExprKind, Stmt}; +use crate::ast::function_type; use crate::ast::types::{Range, Scope, ScopeKind}; use crate::checks::{Check, CheckKind}; use crate::pep8_naming::helpers; -use crate::pep8_naming::helpers::FunctionType; use crate::pep8_naming::settings::Settings; use crate::python::string::{self}; @@ -58,15 +58,16 @@ pub fn invalid_first_argument_name_for_class_method( settings: &Settings, ) -> Option { if !matches!( - helpers::function_type( + function_type::classify( scope, name, decorator_list, from_imports, import_aliases, - settings, + &settings.classmethod_decorators, + &settings.staticmethod_decorators, ), - FunctionType::ClassMethod + function_type::FunctionType::ClassMethod ) { return None; } @@ -99,15 +100,16 @@ pub fn invalid_first_argument_name_for_method( settings: &Settings, ) -> Option { if !matches!( - helpers::function_type( + function_type::classify( scope, name, decorator_list, from_imports, import_aliases, - settings, + &settings.classmethod_decorators, + &settings.staticmethod_decorators, ), - FunctionType::Method + function_type::FunctionType::Method ) { return None; } diff --git a/src/pep8_naming/helpers.rs b/src/pep8_naming/helpers.rs index c15ae928de..ffb23db431 100644 --- a/src/pep8_naming/helpers.rs +++ b/src/pep8_naming/helpers.rs @@ -1,71 +1,10 @@ use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; -use rustpython_ast::{Expr, Stmt, StmtKind}; +use rustpython_ast::{Stmt, StmtKind}; -use crate::ast::helpers::{ - collect_call_paths, dealias_call_path, match_call_path, to_module_and_member, -}; -use crate::ast::types::{Scope, ScopeKind}; -use crate::pep8_naming::settings::Settings; +use crate::ast::helpers::{collect_call_paths, match_call_path}; use crate::python::string::{is_lower, is_upper}; -const CLASS_METHODS: [&str; 3] = ["__new__", "__init_subclass__", "__class_getitem__"]; -const METACLASS_BASES: [(&str, &str); 2] = [("", "type"), ("abc", "ABCMeta")]; - -pub enum FunctionType { - Function, - Method, - ClassMethod, - StaticMethod, -} - -/// Classify a function based on its scope, name, and decorators. -pub fn function_type( - scope: &Scope, - name: &str, - decorator_list: &[Expr], - from_imports: &FxHashMap<&str, FxHashSet<&str>>, - import_aliases: &FxHashMap<&str, &str>, - settings: &Settings, -) -> FunctionType { - let ScopeKind::Class(scope) = &scope.kind else { - return FunctionType::Function; - }; - // Special-case class method, like `__new__`. - if CLASS_METHODS.contains(&name) - || scope.bases.iter().any(|expr| { - // The class itself extends a known metaclass, so all methods are class methods. - let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); - METACLASS_BASES - .iter() - .any(|(module, member)| match_call_path(&call_path, module, member, from_imports)) - }) - || decorator_list.iter().any(|expr| { - // The method is decorated with a class method decorator (like `@classmethod`). - let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); - settings.classmethod_decorators.iter().any(|decorator| { - let (module, member) = to_module_and_member(decorator); - match_call_path(&call_path, module, member, from_imports) - }) - }) - { - FunctionType::ClassMethod - } else if decorator_list.iter().any(|expr| { - // The method is decorated with a static method decorator (like - // `@staticmethod`). - let call_path = dealias_call_path(collect_call_paths(expr), import_aliases); - settings.staticmethod_decorators.iter().any(|decorator| { - let (module, member) = to_module_and_member(decorator); - match_call_path(&call_path, module, member, from_imports) - }) - }) { - FunctionType::StaticMethod - } else { - // It's an instance method. - FunctionType::Method - } -} - pub fn is_camelcase(name: &str) -> bool { !is_lower(name) && !is_upper(name) && !name.contains('_') } diff --git a/src/pydocstyle/plugins.rs b/src/pydocstyle/plugins.rs index 67deefb816..e909c7a540 100644 --- a/src/pydocstyle/plugins.rs +++ b/src/pydocstyle/plugins.rs @@ -7,8 +7,8 @@ use rustc_hash::FxHashSet; use rustpython_ast::{Constant, ExprKind, Location, StmtKind}; use crate::ast::types::Range; -use crate::ast::whitespace; use crate::ast::whitespace::LinesWithTrailingNewline; +use crate::ast::{cast, whitespace}; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckCode, CheckKind}; @@ -77,7 +77,7 @@ pub fn not_missing( false } DefinitionKind::Function(stmt) | DefinitionKind::NestedFunction(stmt) => { - if is_overload(checker, stmt) { + if is_overload(checker, cast::decorator_list(stmt)) { true } else { if checker.settings.enabled.contains(&CheckCode::D103) { @@ -90,7 +90,9 @@ pub fn not_missing( } } DefinitionKind::Method(stmt) => { - if is_overload(checker, stmt) || is_override(checker, stmt) { + if is_overload(checker, cast::decorator_list(stmt)) + || is_override(checker, cast::decorator_list(stmt)) + { true } else if is_magic(stmt) { if checker.settings.enabled.contains(&CheckCode::D105) { @@ -916,7 +918,7 @@ pub fn if_needed(checker: &mut Checker, definition: &Definition) { ) = definition.kind else { return }; - if !is_overload(checker, stmt) { + if !is_overload(checker, cast::decorator_list(stmt)) { return; } checker.add_check(Check::new( @@ -1410,7 +1412,7 @@ fn missing_args(checker: &mut Checker, definition: &Definition, docstrings_args: // If this is a non-static method, skip `cls` or `self`. usize::from( matches!(definition.kind, DefinitionKind::Method(_)) - && !is_staticmethod(checker, parent), + && !is_staticmethod(checker, cast::decorator_list(parent)), ), ) { diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index 4a5f008cbb..eea18e7804 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -7,7 +7,7 @@ use rustpython_parser::ast::{ Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind, }; -use crate::ast::types::{BindingKind, FunctionScope, Range, Scope, ScopeKind}; +use crate::ast::types::{BindingKind, Range, Scope, ScopeKind}; use crate::checks::{Check, CheckKind}; use crate::pyflakes::cformat::CFormatSummary; use crate::pyflakes::format::FormatSummary; @@ -391,13 +391,7 @@ pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option { pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec { let mut checks: Vec = vec![]; - if matches!( - scope.kind, - ScopeKind::Function(FunctionScope { - uses_locals: true, - .. - }) - ) { + if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) { return checks; } diff --git a/src/pylint/plugins/await_outside_async.rs b/src/pylint/plugins/await_outside_async.rs index e9a1908387..0ce25de5e3 100644 --- a/src/pylint/plugins/await_outside_async.rs +++ b/src/pylint/plugins/await_outside_async.rs @@ -1,6 +1,6 @@ use rustpython_ast::Expr; -use crate::ast::types::{FunctionScope, Range, ScopeKind}; +use crate::ast::types::{FunctionDef, Range, ScopeKind}; use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::Check; @@ -10,7 +10,7 @@ pub fn await_outside_async(checker: &mut Checker, expr: &Expr) { if !checker .current_scopes() .find_map(|scope| { - if let ScopeKind::Function(FunctionScope { async_, .. }) = &scope.kind { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { Some(*async_) } else { None diff --git a/src/visibility.rs b/src/visibility.rs index 7006a9cb42..8080eb5874 100644 --- a/src/visibility.rs +++ b/src/visibility.rs @@ -3,7 +3,7 @@ use std::path::Path; -use rustpython_ast::{Stmt, StmtKind}; +use rustpython_ast::{Expr, Stmt, StmtKind}; use crate::ast::helpers::match_module_member; use crate::check_ast::Checker; @@ -29,59 +29,56 @@ pub struct VisibleScope { } /// Returns `true` if a function is a "static method". -pub fn is_staticmethod(checker: &Checker, stmt: &Stmt) -> bool { - match &stmt.node { - StmtKind::FunctionDef { decorator_list, .. } - | StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list.iter().any(|expr| { - match_module_member( - expr, - "", - "staticmethod", - &checker.from_imports, - &checker.import_aliases, - ) - }), - _ => panic!("Found non-FunctionDef in is_staticmethod"), - } +pub fn is_staticmethod(checker: &Checker, decorator_list: &[Expr]) -> bool { + decorator_list.iter().any(|expr| { + match_module_member( + expr, + "", + "staticmethod", + &checker.from_imports, + &checker.import_aliases, + ) + }) } /// Returns `true` if a function is a "class method". -pub fn is_classmethod(checker: &Checker, stmt: &Stmt) -> bool { - match &stmt.node { - StmtKind::FunctionDef { decorator_list, .. } - | StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list.iter().any(|expr| { - match_module_member( - expr, - "", - "classmethod", - &checker.from_imports, - &checker.import_aliases, - ) - }), - _ => panic!("Found non-FunctionDef in is_classmethod"), - } +pub fn is_classmethod(checker: &Checker, decorator_list: &[Expr]) -> bool { + decorator_list.iter().any(|expr| { + match_module_member( + expr, + "", + "classmethod", + &checker.from_imports, + &checker.import_aliases, + ) + }) } /// Returns `true` if a function definition is an `@overload`. -pub fn is_overload(checker: &Checker, stmt: &Stmt) -> bool { - match &stmt.node { - StmtKind::FunctionDef { decorator_list, .. } - | StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list - .iter() - .any(|expr| checker.match_typing_expr(expr, "overload")), - _ => panic!("Found non-FunctionDef in is_overload"), - } +pub fn is_overload(checker: &Checker, decorator_list: &[Expr]) -> bool { + decorator_list + .iter() + .any(|expr| checker.match_typing_expr(expr, "overload")) } /// Returns `true` if a function definition is an `@override` (PEP 698). -pub fn is_override(checker: &Checker, stmt: &Stmt) -> bool { - match &stmt.node { - StmtKind::FunctionDef { decorator_list, .. } - | StmtKind::AsyncFunctionDef { decorator_list, .. } => decorator_list - .iter() - .any(|expr| checker.match_typing_expr(expr, "override")), - _ => panic!("Found non-FunctionDef in is_override"), - } +pub fn is_override(checker: &Checker, decorator_list: &[Expr]) -> bool { + decorator_list + .iter() + .any(|expr| checker.match_typing_expr(expr, "override")) +} + +/// Returns `true` if a function definition is an `@abstractmethod`. +pub fn is_abstract(checker: &Checker, decorator_list: &[Expr]) -> bool { + decorator_list.iter().any(|expr| { + match_module_member( + expr, + "abc", + "abstractmethod", + &checker.from_imports, + &checker.import_aliases, + ) + }) } /// Returns `true` if a function is a "magic method".