Implement too-many-positional-arguments (#9842)

Author: jacobtylerwalls

doc/data/messages/t/too-many-positional-arguments/bad.py

@@ -0,0 +1,5 @@
+class FiveArgumentMethods:
+    """The max positional arguments default is 5."""
+
+    def take_five_args(self, a, b, c, d, e):  # [too-many-positional-arguments]
+        pass

doc/data/messages/t/too-many-positional-arguments/details.rst

@@ -0,0 +1,10 @@
+Positional arguments work well for cases where the the use cases are
+self-evident, such as unittest's ``assertEqual(first, second, msg=None)``.
+Comprehensibility suffers beyond a handful of arguments, though, so for
+functions that take more inputs, require that additional arguments be
+passed by *keyword only* by preceding them with ``*``:
+
+.. code-block:: python
+
+    def make_noise(self, volume, *, color=noise.PINK, debug=True):
+        ...

doc/data/messages/t/too-many-positional-arguments/good.py

@@ -0,0 +1,5 @@
+class FiveArgumentMethods:
+    """The max positional arguments default is 5."""
+
+    def take_five_args(self, a, b, c, d, *, e=False):
+        pass

doc/data/messages/t/too-many-positional-arguments/pylintrc

@@ -0,0 +1,2 @@
+[MESSAGES CONTROL]
+disable=too-many-arguments

doc/data/messages/t/too-many-positional/related.rst renamed to doc/data/messages/t/too-many-positional-arguments/related.rst

@@ -437,10 +437,8 @@ Design checker Messages
   simpler (and so easier to use) class.
 :too-many-locals (R0914): *Too many local variables (%s/%s)*
   Used when a function or method has too many local variables.
-:too-many-positional (R0917): *Too many positional arguments in a function call.*
-  Will be implemented in https://github.com/pylint-
-  dev/pylint/issues/9099,msgid/symbol pair reserved for compatibility with
-  ruff, see https://github.com/astral-sh/ruff/issues/8946.
+:too-many-positional-arguments (R0917): *Too many positional arguments (%s/%s)*
+  Used when a function has too many positional arguments.
 :too-many-public-methods (R0904): *Too many public methods (%s/%s)*
   Used when class has too many public methods, try to reduce this to get a
   simpler (and so easier to use) class.

doc/data/messages/t/too-many-positional/details.rst

@@ -765,6 +765,13 @@ Standard Checkers
 **Default:**  ``7``
 
 
+--max-positional-arguments
+""""""""""""""""""""""""""
+*Maximum number of positional arguments for function / method.*
+
+**Default:**  ``5``
+
+
 --max-public-methods
 """"""""""""""""""""
 *Maximum number of public methods for a class (see R0904).*
@@ -822,6 +829,8 @@ Standard Checkers
 
    max-parents = 7
 
+   max-positional-arguments = 5
+
    max-public-methods = 20
 
    max-returns = 6

doc/user_guide/checkers/features.rst

@@ -539,7 +539,7 @@ All messages in the refactor category:
    refactor/too-many-instance-attributes
    refactor/too-many-locals
    refactor/too-many-nested-blocks
-   refactor/too-many-positional
+   refactor/too-many-positional-arguments
    refactor/too-many-public-methods
    refactor/too-many-return-statements
    refactor/too-many-statements

doc/user_guide/configuration/all-options.rst

@@ -0,0 +1,8 @@
+Added `too-many-positional-arguments` to allow distinguishing the configuration for too many
+total arguments (with keyword-only params specified after `*`) from the configuration
+for too many positional-or-keyword or positional-only arguments.
+
+As part of evaluating whether this check makes sense for your project, ensure you
+adjust the value of `--max-positional-arguments`.
+
+Closes #9099

doc/user_guide/messages/messages_overview.rst

@@ -319,6 +319,8 @@ max-statements=50
 # Minimum number of public methods for a class (see R0903).
 min-public-methods=2
 
+# Minimum number of public methods for a class (see R0903).
+max-positional-arguments=5
 
 [EXCEPTIONS]
 

doc/whatsnew/fragments/9099.new_check

@@ -281,6 +281,9 @@ max-statements = 50
 # Minimum number of public methods for a class (see R0903).
 min-public-methods = 2
 
+# Maximum number of positional arguments (see R0917).
+max-positional-arguments = 5
+
 [tool.pylint.exceptions]
 # Exceptions that will emit a warning when caught.
 overgeneral-exceptions = ["builtins.BaseException", "builtins.Exception"]

examples/pylintrc

@@ -16,6 +16,7 @@
 
 from pylint.checkers import BaseChecker
 from pylint.checkers.utils import is_enum, only_required_for_messages
+from pylint.interfaces import HIGH
 from pylint.typing import MessageDefinitionTuple
 
 if TYPE_CHECKING:
@@ -81,11 +82,9 @@
             "Used when an if statement contains too many boolean expressions.",
         ),
         "R0917": (
-            "Too many positional arguments in a function call.",
-            "too-many-positional",
-            "Will be implemented in https://github.com/pylint-dev/pylint/issues/9099,"
-            "msgid/symbol pair reserved for compatibility with ruff, "
-            "see https://github.com/astral-sh/ruff/issues/8946.",
+            "Too many positional arguments (%s/%s)",
+            "too-many-positional-arguments",
+            "Used when a function has too many positional arguments.",
         ),
     }
 )
@@ -311,6 +310,15 @@ class MisdesignChecker(BaseChecker):
                 "help": "Maximum number of arguments for function / method.",
             },
         ),
+        (
+            "max-positional-arguments",
+            {
+                "default": 5,
+                "type": "int",
+                "metavar": "<int>",
+                "help": "Maximum number of positional arguments for function / method.",
+            },
+        ),
         (
             "max-locals",
             {
@@ -525,6 +533,7 @@ def leave_classdef(self, node: nodes.ClassDef) -> None:
         "too-many-branches",
         "too-many-arguments",
         "too-many-locals",
+        "too-many-positional-arguments",
         "too-many-statements",
         "keyword-arg-before-vararg",
     )
@@ -536,13 +545,20 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None:
         self._returns.append(0)
         # check number of arguments
         args = node.args.args + node.args.posonlyargs + node.args.kwonlyargs
+        pos_args = node.args.args + node.args.posonlyargs
         ignored_argument_names = self.linter.config.ignored_argument_names
         if args is not None:
             ignored_args_num = 0
             if ignored_argument_names:
-                ignored_args_num = sum(
-                    1 for arg in args if ignored_argument_names.match(arg.name)
+                ignored_pos_args_num = sum(
+                    1 for arg in pos_args if ignored_argument_names.match(arg.name)
+                )
+                ignored_kwonly_args_num = sum(
+                    1
+                    for arg in node.args.kwonlyargs
+                    if ignored_argument_names.match(arg.name)
                 )
+                ignored_args_num = ignored_pos_args_num + ignored_kwonly_args_num
 
             argnum = len(args) - ignored_args_num
             if argnum > self.linter.config.max_args:
@@ -551,6 +567,16 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None:
                     node=node,
                     args=(len(args), self.linter.config.max_args),
                 )
+            pos_args_count = (
+                len(args) - len(node.args.kwonlyargs) - ignored_pos_args_num
+            )
+            if pos_args_count > self.linter.config.max_positional_arguments:
+                self.add_message(
+                    "too-many-positional-arguments",
+                    node=node,
+                    args=(pos_args_count, self.linter.config.max_positional_arguments),
+                    confidence=HIGH,
+                )
         else:
             ignored_args_num = 0
         # check number of local variables

examples/pyproject.toml

@@ -25,6 +25,7 @@ class PyreverseConfig(
 
     def __init__(
         self,
+        *,
         mode: str = "PUB_ONLY",
         classes: list[str] | None = None,
         show_ancestors: int | None = None,

pylint/checkers/design_analysis.py

@@ -433,6 +433,9 @@ max-attributes=11
 # Maximum number of statements in a try-block
 max-try-statements = 7
 
+# Maximum number of positional arguments (see R0917).
+max-positional-arguments = 12
+
 [CLASSES]
 
 # List of method names used to declare (i.e. assign) instance attributes.

pylint/testutils/pyreverse.py

@@ -22,7 +22,8 @@ async def some_method(self):
         super(OtherClass, self).test() # [bad-super-call]
 
 
-# +1: [too-many-arguments,too-many-return-statements, too-many-branches]
+# +1: [line-too-long]
+# +1: [too-many-arguments, too-many-positional-arguments, too-many-return-statements, too-many-branches]
 async def complex_function(this, function, has, more, arguments, than,
                            one, _, should, have):
     if 1:

pylintrc

@@ -1,10 +1,12 @@
 redefined-builtin:5:0:5:14:next:Redefining built-in 'next':UNDEFINED
 unused-argument:8:30:8:34:some_function:Unused argument 'arg2':HIGH
 bad-super-call:22:8:22:31:Class.some_method:Bad first argument 'OtherClass' given to super():UNDEFINED
-too-many-arguments:26:0:26:26:complex_function:Too many arguments (10/5):UNDEFINED
-too-many-branches:26:0:26:26:complex_function:Too many branches (13/12):UNDEFINED
-too-many-return-statements:26:0:26:26:complex_function:Too many return statements (10/6):UNDEFINED
-dangerous-default-value:59:0:59:14:func:Dangerous default value [] as argument:UNDEFINED
-duplicate-argument-name:59:18:59:19:func:Duplicate argument name 'a' in function definition:HIGH
-disallowed-name:64:0:64:13:foo:"Disallowed name ""foo""":HIGH
-empty-docstring:64:0:64:13:foo:Empty function docstring:HIGH
+line-too-long:26:0:None:None::Line too long (104/100):UNDEFINED
+too-many-arguments:27:0:27:26:complex_function:Too many arguments (10/5):UNDEFINED
+too-many-branches:27:0:27:26:complex_function:Too many branches (13/12):UNDEFINED
+too-many-positional-arguments:27:0:27:26:complex_function:Too many positional arguments (9/5):HIGH
+too-many-return-statements:27:0:27:26:complex_function:Too many return statements (10/6):UNDEFINED
+dangerous-default-value:60:0:60:14:func:Dangerous default value [] as argument:UNDEFINED
+duplicate-argument-name:60:18:60:19:func:Duplicate argument name 'a' in function definition:HIGH
+disallowed-name:65:0:65:13:foo:"Disallowed name ""foo""":HIGH
+empty-docstring:65:0:65:13:foo:Empty function docstring:HIGH

tests/functional/a/async_functions.py

@@ -383,7 +383,7 @@ def test_finds_args_with_xref_type_google(named_arg, **kwargs):
 
 def test_ignores_optional_specifier_google(
     param1, param2, param3=(), param4=[], param5=[], param6=True
-):
+):  # pylint: disable=too-many-positional-arguments
     """Do something.
 
     Args:
@@ -411,7 +411,7 @@ def test_finds_multiple_complex_types_google(
     named_arg_eight,
     named_arg_nine,
     named_arg_ten,
-):
+):  # pylint: disable=too-many-positional-arguments
     """The google docstring
 
     Args:

tests/functional/a/async_functions.txt

@@ -344,7 +344,7 @@ def my_func(
     named_arg_six,
     named_arg_seven,
     named_arg_eight,
-):
+):  # pylint: disable=too-many-positional-arguments
     """The docstring
 
     Args

tests/functional/ext/docparams/parameter/missing_param_doc_required_Google.py

@@ -1,6 +1,7 @@
 # pylint: disable=missing-docstring,wrong-import-position,unnecessary-dunder-call
 
-def stupid_function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9): # [too-many-arguments]
+# +1: [too-many-arguments, too-many-positional-arguments]
+def stupid_function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9):
     return arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9
 
 

tests/functional/ext/docparams/parameter/missing_param_doc_required_Numpy.py

@@ -1,2 +1,3 @@
-too-many-arguments:3:0:3:19:stupid_function:Too many arguments (9/5):UNDEFINED
-too-many-arguments:36:0:36:9:name1:Too many arguments (6/5):UNDEFINED
+too-many-arguments:4:0:4:19:stupid_function:Too many arguments (9/5):UNDEFINED
+too-many-positional-arguments:4:0:4:19:stupid_function:Too many positional arguments (9/5):HIGH
+too-many-arguments:37:0:37:9:name1:Too many arguments (6/5):UNDEFINED

tests/functional/t/too/too_many_arguments.py

@@ -29,7 +29,8 @@ def too_many_locals_function(): # [too-many-locals]
     args15 = args14 * 15
     return args15
 
-def too_many_arguments_function(arga, argu, argi, arge, argt, args): # [too-many-arguments]
+# +1: [too-many-arguments, too-many-positional-arguments]
+def too_many_arguments_function(arga, argu, argi, arge, argt, args):
     """pylint will complain about too many arguments."""
     arga = argu
     arga += argi

tests/functional/t/too/too_many_arguments.txt

@@ -1,3 +1,4 @@
 too-many-locals:4:0:4:12:function:Too many local variables (16/15):UNDEFINED
 too-many-locals:12:0:12:28:too_many_locals_function:Too many local variables (16/15):UNDEFINED
-too-many-arguments:32:0:32:31:too_many_arguments_function:Too many arguments (6/5):UNDEFINED
+too-many-arguments:33:0:33:31:too_many_arguments_function:Too many arguments (6/5):UNDEFINED
+too-many-positional-arguments:33:0:33:31:too_many_arguments_function:Too many positional arguments (6/5):HIGH

tests/functional/t/too/too_many_locals.py

@@ -0,0 +1,9 @@
+# pylint: disable=missing-function-docstring, missing-module-docstring
+class FiveArgumentMethods:
+    """The max positional arguments default is 5."""
+    def fail1(self, a, b, c, d, e):  # [too-many-arguments, too-many-positional-arguments]
+        pass
+    def fail2(self, a, b, c, d, /, e):  # [too-many-arguments, too-many-positional-arguments]
+        pass
+    def okay1(self, a, b, c, d, *, e=True):  # [too-many-arguments]
+        pass

tests/functional/t/too/too_many_locals.txt

@@ -0,0 +1,5 @@
+too-many-arguments:4:4:4:13:FiveArgumentMethods.fail1:Too many arguments (6/5):UNDEFINED
+too-many-positional-arguments:4:4:4:13:FiveArgumentMethods.fail1:Too many positional arguments (6/5):HIGH
+too-many-arguments:6:4:6:13:FiveArgumentMethods.fail2:Too many arguments (6/5):UNDEFINED
+too-many-positional-arguments:6:4:6:13:FiveArgumentMethods.fail2:Too many positional arguments (6/5):HIGH
+too-many-arguments:8:4:8:13:FiveArgumentMethods.okay1:Too many arguments (6/5):UNDEFINED

tests/functional/t/too/too_many_positional_arguments.py

@@ -74,6 +74,7 @@ class Valid:
     def __new__(cls, test, multiple, args):
         pass
 
+    # pylint: disable-next=too-many-positional-arguments
     def __init__(self, this, can, have, multiple, args, as_well):
         pass