[pylint] Implement too-many-positional (PLR0917)

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

@@ -0,0 +1,30 @@
+def f(x, y, z, t, u, v, w, r):  # Too many positional arguments (8/3)
+    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):  # OK
+    pass
+
+
+def f(x=1, y=1, z=1):  # OK
+    pass
+
+
+def f(x, y, z, /, u, v, w):  # Too many positional arguments (6/3)
+    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)
+    pass

crates/ruff_linter/resources/test/fixtures/pylint/too_many_positional_params.py

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

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -250,6 +250,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::TooManyArguments) {
                 pylint::rules::too_many_arguments(checker, function_def);
             }
+            if checker.enabled(Rule::TooManyPositional) {
+                pylint::rules::too_many_positional(checker, parameters, stmt);
+            }
             if checker.enabled(Rule::TooManyReturnStatements) {
                 if let Some(diagnostic) = pylint::rules::too_many_return_statements(
                     stmt,

crates/ruff_linter/src/codes.rs

@@ -254,6 +254,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R0913") => (RuleGroup::Stable, rules::pylint::rules::TooManyArguments),
         (Pylint, "R0915") => (RuleGroup::Stable, rules::pylint::rules::TooManyStatements),
         (Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
+        (Pylint, "R0917") => (RuleGroup::Preview, rules::pylint::rules::TooManyPositional),
         (Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls),
         (Pylint, "R1704") => (RuleGroup::Preview, rules::pylint::rules::RedefinedArgumentFromLocal),
         (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -94,6 +94,7 @@ mod tests {
     #[test_case(Rule::RedefinedLoopName, Path::new("redefined_loop_name.py"))]
     #[test_case(Rule::ReturnInInit, Path::new("return_in_init.py"))]
     #[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"))]
+    #[test_case(Rule::TooManyPositional, Path::new("too_many_positional.py"))]
     #[test_case(Rule::TooManyBranches, Path::new("too_many_branches.py"))]
     #[test_case(
         Rule::TooManyReturnStatements,
@@ -249,6 +250,22 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn max_positional_args() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("pylint/too_many_positional_params.py"),
+            &LinterSettings {
+                pylint: pylint::settings::Settings {
+                    max_positional_args: 4,
+                    ..pylint::settings::Settings::default()
+                },
+                ..LinterSettings::for_rule(Rule::TooManyPositional)
+            },
+        )?;
+        assert_messages!(diagnostics);
+        Ok(())
+    }
+
     #[test]
     fn max_branches() -> Result<()> {
         let diagnostics = test_path(

crates/ruff_linter/src/rules/pylint/rules/mod.rs

@@ -55,6 +55,7 @@ pub(crate) use sys_exit_alias::*;
 pub(crate) use too_many_arguments::*;
 pub(crate) use too_many_boolean_expressions::*;
 pub(crate) use too_many_branches::*;
+pub(crate) use too_many_positional::*;
 pub(crate) use too_many_public_methods::*;
 pub(crate) use too_many_return_statements::*;
 pub(crate) use too_many_statements::*;
@@ -131,6 +132,7 @@ mod sys_exit_alias;
 mod too_many_arguments;
 mod too_many_boolean_expressions;
 mod too_many_branches;
+mod too_many_positional;
 mod too_many_public_methods;
 mod too_many_return_statements;
 mod too_many_statements;

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

@@ -0,0 +1,79 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{identifier::Identifier, Parameters, Stmt};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for function definitions that include too many positional arguments.
+///
+/// By default, this rule allows up to five arguments, as configured by the
+/// [`pylint.max-positional-args`] option.
+///
+/// ## Why is this bad?
+/// Functions with many arguments are harder to understand, maintain, and call.
+/// This is especially true for functions with many positional arguments, as
+/// providing arguments positionally is more error-prone and less clear to
+/// readers than providing arguments by name.
+///
+/// Consider refactoring functions with many arguments into smaller functions
+/// with fewer arguments, using objects to group related arguments, or
+/// migrating to keyword-only arguments.
+///
+/// ## Example
+/// ```python
+/// def plot(x, y, z, color, mark, add_trendline):
+///     ...
+///
+///
+/// plot(1, 2, 3, "r", "*", True)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// def plot(x, y, z, *, color, mark, add_trendline):
+///     ...
+///
+///
+/// plot(1, 2, 3, color="r", mark="*", add_trendline=True)
+/// ```
+///
+/// ## Options
+/// - `pylint.max-positional-args`
+#[violation]
+pub struct TooManyPositional {
+    c_pos: usize,
+    max_pos: usize,
+}
+
+impl Violation for TooManyPositional {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let TooManyPositional { c_pos, max_pos } = self;
+        format!("Too many positional arguments: ({c_pos}/{max_pos})")
+    }
+}
+
+/// PLR0917
+pub(crate) fn too_many_positional(checker: &mut Checker, parameters: &Parameters, stmt: &Stmt) {
+    let num_positional_args = parameters
+        .args
+        .iter()
+        .chain(&parameters.posonlyargs)
+        .filter(|arg| {
+            !checker
+                .settings
+                .dummy_variable_rgx
+                .is_match(&arg.parameter.name)
+        })
+        .count();
+    if num_positional_args > checker.settings.pylint.max_positional_args {
+        checker.diagnostics.push(Diagnostic::new(
+            TooManyPositional {
+                c_pos: num_positional_args,
+                max_pos: checker.settings.pylint.max_positional_args,
+            },
+            stmt.identifier(),
+        ));
+    }
+}

crates/ruff_linter/src/rules/pylint/settings.rs

@@ -39,6 +39,7 @@ pub struct Settings {
     pub allow_magic_value_types: Vec<ConstantType>,
     pub allow_dunder_method_names: FxHashSet<String>,
     pub max_args: usize,
+    pub max_positional_args: usize,
     pub max_returns: usize,
     pub max_bool_expr: usize,
     pub max_branches: usize,
@@ -52,6 +53,7 @@ impl Default for Settings {
             allow_magic_value_types: vec![ConstantType::Str, ConstantType::Bytes],
             allow_dunder_method_names: FxHashSet::default(),
             max_args: 5,
+            max_positional_args: 5,
             max_returns: 6,
             max_bool_expr: 5,
             max_branches: 12,

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

@@ -0,0 +1,25 @@
+---
+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)
+  |     ^ 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)
+   |     ^ 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)
+   |     ^ PLR0917
+30 |     pass
+   |
+
+

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__max_positional_args.snap

@@ -0,0 +1,22 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+too_many_positional_params.py:3:5: PLR0917 Too many positional arguments: (7/4)
+  |
+1 | # Too many positional arguments (7/4) for max_positional=4
+2 | # OK for dummy_variable_rgx ~ "skip_.*"
+3 | def f(w, x, y, z, skip_t, skip_u, skip_v):
+  |     ^ PLR0917
+4 |     pass
+  |
+
+too_many_positional_params.py:9:5: PLR0917 Too many positional arguments: (7/4)
+   |
+ 7 | # Too many positional arguments (7/4) for max_args=4
+ 8 | # Too many positional arguments (7/3) for dummy_variable_rgx ~ "skip_.*"
+ 9 | def f(w, x, y, z, t, u, v):
+   |     ^ PLR0917
+10 |     pass
+   |
+
+

crates/ruff_workspace/src/options.rs

@@ -2635,6 +2635,11 @@ pub struct PylintOptions {
     #[option(default = r"5", value_type = "int", example = r"max-args = 5")]
     pub max_args: Option<usize>,
 
+    /// Maximum number of positional arguments allowed for a function or method definition
+    /// (see: `PLR0917`).
+    #[option(default = r"3", value_type = "int", example = r"max-pos-args = 3")]
+    pub max_positional_args: Option<usize>,
+
     /// Maximum number of statements allowed for a function or method body (see:
     /// `PLR0915`).
     #[option(default = r"50", value_type = "int", example = r"max-statements = 50")]
@@ -2663,6 +2668,9 @@ impl PylintOptions {
                 .unwrap_or(defaults.allow_magic_value_types),
             allow_dunder_method_names: self.allow_dunder_method_names.unwrap_or_default(),
             max_args: self.max_args.unwrap_or(defaults.max_args),
+            max_positional_args: self
+                .max_positional_args
+                .unwrap_or(defaults.max_positional_args),
             max_bool_expr: self.max_bool_expr.unwrap_or(defaults.max_bool_expr),
             max_returns: self.max_returns.unwrap_or(defaults.max_returns),
             max_branches: self.max_branches.unwrap_or(defaults.max_branches),