Implement Pylint's too-many-arguments rule (PLR0913) (#2308)

This commit is contained in:
Akhil 2023-01-30 18:04:37 +05:30 committed by GitHub
parent 1e325edfb1
commit 8e5a944ce1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 254 additions and 7 deletions

View file

@ -1296,6 +1296,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/) on PyPI.
| PLR0133 | constant-comparison | Two constants compared in a comparison, consider replacing `{left_constant} {op} {right_constant}` | |
| PLR0206 | property-with-parameters | Cannot have defined parameters for properties | |
| PLR0402 | consider-using-from-import | Use `from {module} import {name}` in lieu of alias | |
| PLR0913 | too-many-args | Too many arguments to function call ({c_args}/{max_args}) | |
| PLR1701 | consider-merging-isinstance | Merge these isinstance calls: `isinstance({obj}, ({types}))` | |
| PLR1722 | use-sys-exit | Use `sys.exit()` instead of `{name}` | 🛠 |
| PLR2004 | magic-value-comparison | Magic value used in comparison, consider replacing {value} with a constant variable | |
@ -3751,7 +3752,7 @@ convention = "google"
#### [`allow-magic-value-types`](#allow-magic-value-types)
Constant types to ignore when used as "magic values".
Constant types to ignore when used as "magic values" (see: `PLR2004`).
**Default value**: `["str"]`
@ -3766,6 +3767,23 @@ allow-magic-value-types = ["int"]
---
#### [`max-args`](#max-args)
Maximum number of arguments allowed for a function definition (see: `PLR0913`).
**Default value**: `5`
**Type**: `usize`
**Example usage**:
```toml
[tool.ruff.pylint]
max_args = 5
```
---
### `pyupgrade`
#### [`keep-runtime-typing`](#keep-runtime-typing)

View file

@ -0,0 +1,34 @@
def f(x, y, z, t, u, v, w, r): # Too many arguments (8/5)
pass
def f(x, y, z, t, u): # OK
pass
def f(x): # OK
pass
def f(x, y, z, _t, _u, _v, _w, r): # OK (underscore-prefixed names are ignored
pass
def f(x, y, z, u=1, v=1, r=1): # Too many arguments (6/5)
pass
def f(x=1, y=1, z=1): # OK
pass
def f(x, y, z, /, u, v, w): # OK
pass
def f(x, y, z, *, u, v, w): # OK
pass
def f(x, y, z, a, b, c, *, u, v, w): # Too many arguments (6/5)
pass

View file

@ -0,0 +1,10 @@
# Too many args (6/4) for max_args=4
# OK for dummy_variable_rgx ~ "skip_.*"
def f(x, y, z, skip_t, skip_u, skip_v):
pass
# Too many args (6/4) for max_args=4
# Too many args (6/5) for dummy_variable_rgx ~ "skip_.*"
def f(x, y, z, t, u, v):
pass

View file

@ -1164,7 +1164,7 @@
"type": "object",
"properties": {
"allow-magic-value-types": {
"description": "Constant types to ignore when used as \"magic values\".",
"description": "Constant types to ignore when used as \"magic values\" (see: `PLR2004`).",
"type": [
"array",
"null"
@ -1172,6 +1172,15 @@
"items": {
"$ref": "#/definitions/ConstantType"
}
},
"max-args": {
"description": "Maximum number of arguments allowed for a function definition (see: `PLR0913`).",
"type": [
"integer",
"null"
],
"format": "uint",
"minimum": 0.0
}
},
"additionalProperties": false
@ -1641,6 +1650,9 @@
"PLR04",
"PLR040",
"PLR0402",
"PLR09",
"PLR091",
"PLR0913",
"PLR1",
"PLR17",
"PLR170",

View file

@ -694,6 +694,9 @@ where
context,
},
);
if self.settings.rules.enabled(&Rule::TooManyArgs) {
pylint::rules::too_many_args(self, args, stmt);
}
}
StmtKind::Return { .. } => {
if self.settings.rules.enabled(&Rule::ReturnOutsideFunction) {

View file

@ -93,6 +93,7 @@ ruff_macros::define_rule_mapping!(
PLR2004 => violations::MagicValueComparison,
PLW0120 => violations::UselessElseOnLoop,
PLW0602 => violations::GlobalVariableNotAssigned,
PLR0913 => rules::pylint::rules::TooManyArgs,
// flake8-builtins
A001 => violations::BuiltinVariableShadowing,
A002 => violations::BuiltinArgumentShadowing,

View file

@ -4,9 +4,9 @@ pub mod settings;
#[cfg(test)]
mod tests {
use std::path::Path;
use anyhow::Result;
use regex::Regex;
use std::path::Path;
use test_case::test_case;
use crate::assert_yaml_snapshot;
@ -36,6 +36,7 @@ mod tests {
#[test_case(Rule::GlobalVariableNotAssigned, Path::new("global_variable_not_assigned.py"); "PLW0602")]
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"); "PLE0605")]
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"); "PLE0604")]
#[test_case(Rule::TooManyArgs, Path::new("too_many_args.py"); "PLR0913")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
let diagnostics = test_path(
@ -55,6 +56,7 @@ mod tests {
&Settings {
pylint: pylint::settings::Settings {
allow_magic_value_types: vec![pylint::settings::ConstantType::Int],
..pylint::settings::Settings::default()
},
..Settings::for_rules(vec![Rule::MagicValueComparison])
},
@ -62,4 +64,33 @@ mod tests {
assert_yaml_snapshot!(diagnostics);
Ok(())
}
#[test]
fn max_args() -> Result<()> {
let diagnostics = test_path(
Path::new("./resources/test/fixtures/pylint/too_many_args_params.py"),
&Settings {
pylint: pylint::settings::Settings {
max_args: 4,
..pylint::settings::Settings::default()
},
..Settings::for_rules(vec![Rule::TooManyArgs])
},
)?;
assert_yaml_snapshot!(diagnostics);
Ok(())
}
#[test]
fn max_args_with_dummy_variables() -> Result<()> {
let diagnostics = test_path(
Path::new("./resources/test/fixtures/pylint/too_many_args_params.py"),
&Settings {
dummy_variable_rgx: Regex::new(r"skip_.*").unwrap().into(),
..Settings::for_rules(vec![Rule::TooManyArgs])
},
)?;
assert_yaml_snapshot!(diagnostics);
Ok(())
}
}

View file

@ -5,6 +5,7 @@ pub use invalid_all_object::{invalid_all_object, InvalidAllObject};
pub use magic_value_comparison::magic_value_comparison;
pub use merge_isinstance::merge_isinstance;
pub use property_with_parameters::property_with_parameters;
pub use too_many_args::{too_many_args, TooManyArgs};
pub use unnecessary_direct_lambda_call::unnecessary_direct_lambda_call;
pub use use_from_import::use_from_import;
pub use use_sys_exit::use_sys_exit;
@ -19,6 +20,7 @@ mod invalid_all_object;
mod magic_value_comparison;
mod merge_isinstance;
mod property_with_parameters;
mod too_many_args;
mod unnecessary_direct_lambda_call;
mod use_from_import;
mod use_sys_exit;

View file

@ -0,0 +1,41 @@
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::define_violation;
use crate::registry::Diagnostic;
use crate::violation::Violation;
use ruff_macros::derive_message_formats;
use rustpython_ast::{Arguments, Stmt};
define_violation!(
pub struct TooManyArgs {
pub c_args: usize,
pub max_args: usize,
}
);
impl Violation for TooManyArgs {
#[derive_message_formats]
fn message(&self) -> String {
let TooManyArgs { c_args, max_args } = self;
format!("Too many arguments to function call ({c_args}/{max_args})")
}
}
/// PLR0913
pub fn too_many_args(checker: &mut Checker, args: &Arguments, stmt: &Stmt) {
let num_args = args
.args
.iter()
.filter(|arg| !checker.settings.dummy_variable_rgx.is_match(&arg.node.arg))
.count();
if num_args > checker.settings.pylint.max_args {
checker.diagnostics.push(Diagnostic::new(
TooManyArgs {
c_args: num_args,
max_args: checker.settings.pylint.max_args,
},
Range::from_located(stmt),
));
}
}

View file

@ -5,7 +5,7 @@ use ruff_macros::ConfigurationOptions;
use rustpython_ast::Constant;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use std::hash::Hash;
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Hash, JsonSchema)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub enum ConstantType {
@ -51,29 +51,36 @@ pub struct Options {
allow-magic-value-types = ["int"]
"#
)]
/// Constant types to ignore when used as "magic values".
/// Constant types to ignore when used as "magic values" (see: `PLR2004`).
pub allow_magic_value_types: Option<Vec<ConstantType>>,
#[option(default = r"5", value_type = "usize", example = r"max_args = 5")]
/// Maximum number of arguments allowed for a function definition (see: `PLR0913`).
pub max_args: Option<usize>,
}
#[derive(Debug, Hash)]
pub struct Settings {
pub allow_magic_value_types: Vec<ConstantType>,
pub max_args: usize,
}
impl Default for Settings {
fn default() -> Self {
Self {
allow_magic_value_types: vec![ConstantType::Str],
max_args: 5,
}
}
}
impl From<Options> for Settings {
fn from(options: Options) -> Self {
let defaults = Settings::default();
Self {
allow_magic_value_types: options
.allow_magic_value_types
.unwrap_or_else(|| vec![ConstantType::Str]),
.unwrap_or(defaults.allow_magic_value_types),
max_args: options.max_args.unwrap_or(defaults.max_args),
}
}
}
@ -82,6 +89,7 @@ impl From<Settings> for Options {
fn from(settings: Settings) -> Self {
Self {
allow_magic_value_types: Some(settings.allow_magic_value_types),
max_args: Some(settings.max_args),
}
}
}

View file

@ -0,0 +1,41 @@
---
source: src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
TooManyArgs:
c_args: 8
max_args: 5
location:
row: 1
column: 0
end_location:
row: 2
column: 8
fix: ~
parent: ~
- kind:
TooManyArgs:
c_args: 6
max_args: 5
location:
row: 17
column: 0
end_location:
row: 18
column: 8
fix: ~
parent: ~
- kind:
TooManyArgs:
c_args: 6
max_args: 5
location:
row: 33
column: 0
end_location:
row: 34
column: 8
fix: ~
parent: ~

View file

@ -0,0 +1,29 @@
---
source: src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
TooManyArgs:
c_args: 6
max_args: 4
location:
row: 3
column: 0
end_location:
row: 4
column: 8
fix: ~
parent: ~
- kind:
TooManyArgs:
c_args: 6
max_args: 4
location:
row: 9
column: 0
end_location:
row: 10
column: 8
fix: ~
parent: ~

View file

@ -0,0 +1,17 @@
---
source: src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
TooManyArgs:
c_args: 6
max_args: 5
location:
row: 9
column: 0
end_location:
row: 10
column: 8
fix: ~
parent: ~