Skip to content

Commit

Permalink
Ignore self and cls when counting arguments (#12367)
Browse files Browse the repository at this point in the history
## Summary

Closes #12320.
  • Loading branch information
charliermarsh authored Jul 17, 2024
1 parent 79b5355 commit 80f0116
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,32 @@ def f(x, y, z, a, b, c, *, u, v, w): # OK
@overload
def f(x, y, z, a, b, c, *, u, v, w): # OK
pass


class C:
def f(self, y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
pass

def f(self, y, z, a, b, c): # OK
pass

@classmethod
def f(cls, y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
pass

@classmethod
def f(cls, y, z, a, b, c): # OK
pass

@staticmethod
def f(y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
pass

@staticmethod
def f(y, z, a, b, c, d): # OK
pass

@staticmethod
def f(y, z, a, b, c): # OK
pass

59 changes: 43 additions & 16 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_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 @@ -59,6 +59,8 @@ impl Violation for TooManyArguments {

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

let num_arguments = function_def
.parameters
.iter_non_variadic_params()
Expand All @@ -70,21 +72,46 @@ pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::Stmt
})
.count();

if num_arguments > checker.settings.pylint.max_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())
{
return;
}
if num_arguments <= checker.settings.pylint.max_args {
return;
}

// 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())
{
return;
}

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

checker.diagnostics.push(Diagnostic::new(
TooManyArguments {
c_args: num_arguments,
max_args: checker.settings.pylint.max_args,
},
function_def.identifier(),
));
if num_arguments <= checker.settings.pylint.max_args {
return;
}

checker.diagnostics.push(Diagnostic::new(
TooManyArguments {
c_args: num_arguments,
max_args: checker.settings.pylint.max_args,
},
function_def.identifier(),
));
}
38 changes: 22 additions & 16 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm
})
.count();

if num_positional_args <= checker.settings.pylint.max_positional_args {
return;
}

// 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, semantic)
|| visibility::is_overload(&function_def.decorator_list, semantic)
{
return;
}

// Check if the function is a method or class method.
let num_positional_args = if matches!(
function_type::classify(
Expand All @@ -92,21 +104,15 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm
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, semantic)
|| visibility::is_overload(&function_def.decorator_list, semantic)
{
return;
}

checker.diagnostics.push(Diagnostic::new(
TooManyPositional {
c_pos: num_positional_args,
max_pos: checker.settings.pylint.max_positional_args,
},
function_def.identifier(),
));
if num_positional_args <= checker.settings.pylint.max_positional_args {
return;
}

checker.diagnostics.push(Diagnostic::new(
TooManyPositional {
c_pos: num_positional_args,
max_pos: checker.settings.pylint.max_positional_args,
},
function_def.identifier(),
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,34 @@ too_many_arguments.py:33:5: PLR0913 Too many arguments in function definition (9
34 | pass
|

too_many_arguments.py:51:9: PLR0913 Too many arguments in function definition (8 > 5)
|
50 | class C:
51 | def f(self, y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
| ^ PLR0913
52 | pass
|

too_many_arguments.py:58:9: PLR0913 Too many arguments in function definition (8 > 5)
|
57 | @classmethod
58 | def f(cls, y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
| ^ PLR0913
59 | pass
|

too_many_arguments.py:66:9: PLR0913 Too many arguments in function definition (8 > 5)
|
65 | @staticmethod
66 | def f(y, z, a, b, c, *, u, v, w): # Too many arguments (8/5)
| ^ PLR0913
67 | pass
|

too_many_arguments.py:70:9: PLR0913 Too many arguments in function definition (6 > 5)
|
69 | @staticmethod
70 | def f(y, z, a, b, c, d): # OK
| ^ PLR0913
71 | pass
|

0 comments on commit 80f0116

Please sign in to comment.