Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Exclude self and cls when counting method arguments #9563

Merged
merged 2 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
tmke8 marked this conversation as resolved.
Show resolved Hide resolved


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
27 changes: 23 additions & 4 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,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()
Expand All @@ -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;
}
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
|