Skip to content

Commit 7be7066

Browse files
authoredJan 18, 2024
[pylint] Exclude self and cls when counting method arguments (#9563)
## Summary This PR detects whether PLR0917 is being applied to a method or class method, and if so, it ignores the first argument for the purposes of counting the number of positional arguments. ## Test Plan New tests have been added to the corresponding fixture. Closes #9552.
1 parent 848e473 commit 7be7066

File tree

3 files changed

+79
-9
lines changed

3 files changed

+79
-9
lines changed
 

‎crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional.py

+34-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
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)
22
pass
33

44

@@ -18,13 +18,44 @@ def f(x=1, y=1, z=1): # OK
1818
pass
1919

2020

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)
2222
pass
2323

2424

2525
def f(x, y, z, *, u, v, w): # OK
2626
pass
2727

2828

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)
3030
pass
31+
32+
33+
class C:
34+
def f(self, a, b, c, d, e): # OK
35+
pass
36+
37+
def f(self, /, a, b, c, d, e): # OK
38+
pass
39+
40+
def f(weird_self_name, a, b, c, d, e): # OK
41+
pass
42+
43+
def f(self, a, b, c, d, e, g): # Too many positional arguments (6/5)
44+
pass
45+
46+
@staticmethod
47+
def f(self, a, b, c, d, e): # Too many positional arguments (6/5)
48+
pass
49+
50+
@classmethod
51+
def f(cls, a, b, c, d, e): # OK
52+
pass
53+
54+
def f(*, self, a, b, c, d, e): # OK
55+
pass
56+
57+
def f(self=1, a=1, b=1, c=1, d=1, e=1): # OK
58+
pass
59+
60+
def f(): # OK
61+
pass

‎crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs

+25-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use ruff_diagnostics::{Diagnostic, Violation};
22
use ruff_macros::{derive_message_formats, violation};
33
use ruff_python_ast::{self as ast, identifier::Identifier};
4-
use ruff_python_semantic::analyze::visibility;
4+
use ruff_python_semantic::analyze::{function_type, visibility};
55

66
use crate::checkers::ast::Checker;
77

@@ -57,6 +57,9 @@ impl Violation for TooManyPositional {
5757

5858
/// PLR0917
5959
pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
60+
let semantic = checker.semantic();
61+
62+
// Count the number of positional arguments.
6063
let num_positional_args = function_def
6164
.parameters
6265
.args
@@ -70,11 +73,30 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm
7073
})
7174
.count();
7275

76+
// Check if the function is a method or class method.
77+
let num_positional_args = if matches!(
78+
function_type::classify(
79+
&function_def.name,
80+
&function_def.decorator_list,
81+
semantic.current_scope(),
82+
semantic,
83+
&checker.settings.pep8_naming.classmethod_decorators,
84+
&checker.settings.pep8_naming.staticmethod_decorators,
85+
),
86+
function_type::FunctionType::Method | function_type::FunctionType::ClassMethod
87+
) {
88+
// If so, we need to subtract one from the number of positional arguments, since the first
89+
// argument is always `self` or `cls`.
90+
num_positional_args.saturating_sub(1)
91+
} else {
92+
num_positional_args
93+
};
94+
7395
if num_positional_args > checker.settings.pylint.max_positional_args {
7496
// Allow excessive arguments in `@override` or `@overload` methods, since they're required
7597
// to adhere to the parent signature.
76-
if visibility::is_override(&function_def.decorator_list, checker.semantic())
77-
|| visibility::is_overload(&function_def.decorator_list, checker.semantic())
98+
if visibility::is_override(&function_def.decorator_list, semantic)
99+
|| visibility::is_overload(&function_def.decorator_list, semantic)
78100
{
79101
return;
80102
}

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0917_too_many_positional.py.snap

+20-3
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,40 @@ source: crates/ruff_linter/src/rules/pylint/mod.rs
33
---
44
too_many_positional.py:1:5: PLR0917 Too many positional arguments (8/5)
55
|
6-
1 | def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/3)
6+
1 | def f(x, y, z, t, u, v, w, r): # Too many positional arguments (8/5)
77
| ^ PLR0917
88
2 | pass
99
|
1010

1111
too_many_positional.py:21:5: PLR0917 Too many positional arguments (6/5)
1212
|
13-
21 | def f(x, y, z, /, u, v, w): # Too many positional arguments (6/3)
13+
21 | def f(x, y, z, /, u, v, w): # Too many positional arguments (6/5)
1414
| ^ PLR0917
1515
22 | pass
1616
|
1717

1818
too_many_positional.py:29:5: PLR0917 Too many positional arguments (6/5)
1919
|
20-
29 | def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/3)
20+
29 | def f(x, y, z, a, b, c, *, u, v, w): # Too many positional arguments (6/5)
2121
| ^ PLR0917
2222
30 | pass
2323
|
2424

25+
too_many_positional.py:43:9: PLR0917 Too many positional arguments (6/5)
26+
|
27+
41 | pass
28+
42 |
29+
43 | def f(self, a, b, c, d, e, g): # Too many positional arguments (6/5)
30+
| ^ PLR0917
31+
44 | pass
32+
|
33+
34+
too_many_positional.py:47:9: PLR0917 Too many positional arguments (6/5)
35+
|
36+
46 | @staticmethod
37+
47 | def f(self, a, b, c, d, e): # Too many positional arguments (6/5)
38+
| ^ PLR0917
39+
48 | pass
40+
|
41+
2542

0 commit comments

Comments
 (0)
Please sign in to comment.