Skip to content

Commit

Permalink
[pylint] Exclude self and cls when counting method arguments (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
tmke8 authored Jan 18, 2024
1 parent 848e473 commit 7be7066
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -18,13 +18,44 @@ 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


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
28 changes: 25 additions & 3 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -57,6 +57,9 @@ impl Violation for TooManyPositional {

/// PLR0917
pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
let semantic = checker.semantic();

// Count the number of positional arguments.
let num_positional_args = function_def
.parameters
.args
Expand All @@ -70,11 +73,30 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm
})
.count();

// Check if the function is a method or class method.
let num_positional_args = 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.saturating_sub(1)
} else {
num_positional_args
};

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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|


0 comments on commit 7be7066

Please sign in to comment.