From a8e2f5c472f621942da4f6edfea71837a9dcaa79 Mon Sep 17 00:00:00 2001 From: Thomas M Kehrenberg Date: Wed, 17 Jan 2024 15:00:00 +0100 Subject: [PATCH] PLR0917: Ignore first argument in methods closes #9552 --- .../fixtures/pylint/too_many_positional.py | 37 +++++++++++++++++-- .../rules/pylint/rules/too_many_positional.rs | 27 ++++++++++++-- ...tests__PLR0917_too_many_positional.py.snap | 23 ++++++++++-- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional.py b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional.py index 3c0fe2a2f5fb4..b769acf63777a 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional.py @@ -1,4 +1,4 @@ -def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/3) +def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/5) pass @@ -18,7 +18,7 @@ def f(x=1, y=1, z=1): # OK pass -def f(x, y, z, /, u, v, w): # Too many positional arguments (6/3) +def f(x, y, z, /, u, v, w): # Too many positional arguments (6/5) pass @@ -26,5 +26,36 @@ def f(x, y, z, *, u, v, w): # OK pass -def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/3) +def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/5) pass + + +class C: + def f(self, a, b, c, d, e): # OK + pass + + def f(self, /, a, b, c, d, e): # OK + pass + + def f(weird_self_name, a, b, c, d, e): # OK + pass + + def f(self, a, b, c, d, e, g): # Too many positional arguments (6/5) + pass + + @staticmethod + def f(self, a, b, c, d, e): # Too many positional arguments (6/5) + pass + + @classmethod + def f(cls, a, b, c, d, e): # OK + pass + + def f(*, self, a, b, c, d, e): # OK + pass + + def f(self=1, a=1, b=1, c=1, d=1, e=1): # OK + pass + + def f(): # OK + pass diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs index c30331f88de44..50df275ff5fe0 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs @@ -1,7 +1,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, identifier::Identifier}; -use ruff_python_semantic::analyze::visibility; +use ruff_python_semantic::analyze::{function_type, visibility}; use crate::checkers::ast::Checker; @@ -57,7 +57,7 @@ impl Violation for TooManyPositional { /// PLR0917 pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { - let num_positional_args = function_def + let mut num_positional_args = function_def .parameters .args .iter() @@ -70,11 +70,30 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm }) .count(); + let semantic = checker.semantic(); + + // Check if the function is a method or class method. + if matches!( + function_type::classify( + &function_def.name, + &function_def.decorator_list, + semantic.current_scope(), + semantic, + &checker.settings.pep8_naming.classmethod_decorators, + &checker.settings.pep8_naming.staticmethod_decorators, + ), + function_type::FunctionType::Method | function_type::FunctionType::ClassMethod + ) { + // If so, we need to subtract one from the number of positional arguments, since the first + // argument is always `self` or `cls`. + num_positional_args = num_positional_args.saturating_sub(1); + } + if num_positional_args > checker.settings.pylint.max_positional_args { // Allow excessive arguments in `@override` or `@overload` methods, since they're required // to adhere to the parent signature. - if visibility::is_override(&function_def.decorator_list, checker.semantic()) - || visibility::is_overload(&function_def.decorator_list, checker.semantic()) + if visibility::is_override(&function_def.decorator_list, semantic) + || visibility::is_overload(&function_def.decorator_list, semantic) { return; } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional.py.snap index 8b5057680a211..8f4ee17a79446 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional.py.snap @@ -3,23 +3,40 @@ source: crates/ruff_linter/src/rules/pylint/mod.rs --- too_many_positional.py:1:5: PLR0917 Too many positional arguments (8/5) | -1 | def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/3) +1 | def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/5) | ^ PLR0917 2 | pass | too_many_positional.py:21:5: PLR0917 Too many positional arguments (6/5) | -21 | def f(x, y, z, /, u, v, w): # Too many positional arguments (6/3) +21 | def f(x, y, z, /, u, v, w): # Too many positional arguments (6/5) | ^ PLR0917 22 | pass | too_many_positional.py:29:5: PLR0917 Too many positional arguments (6/5) | -29 | def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/3) +29 | def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/5) | ^ PLR0917 30 | pass | +too_many_positional.py:43:9: PLR0917 Too many positional arguments (6/5) + | +41 | pass +42 | +43 | def f(self, a, b, c, d, e, g): # Too many positional arguments (6/5) + | ^ PLR0917 +44 | pass + | + +too_many_positional.py:47:9: PLR0917 Too many positional arguments (6/5) + | +46 | @staticmethod +47 | def f(self, a, b, c, d, e): # Too many positional arguments (6/5) + | ^ PLR0917 +48 | pass + | +