Repository: wemake-services/wemake-python-styleguide

Issue:
Forbid to mix several argument types
# Rule request

<!-- Thanks for coming up with a proposal! -->

## Thesis

<!-- What exactly do you propose? Please, provide code examples. -->

This code contains two types of similar but different arguments:

```python
def first(first: int = 0, *, second: int): ...

def second(first: int = 0, *, second: int = 0): ...
```

I would also say that these two groups does not work well together:

```python
def first(first: int = 0, *args): ...
```

## Reasoning

It is really hard to call these functions, their arguments are confusing. The best practice is not to mix positional with defaults and kw-only, positional with default and args.

This is a best-practice rule. 

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/wemake-services/wemake-python-styleguide/pull/3173?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wemake-services) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`32d32e6`)](https://app.codecov.io/gh/wemake-services/wemake-python-styleguide/commit/32d32e6314b946e113e6cc3e67ed18dfb09bd6a4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wemake-services) to head [(`63782d5`)](https://app.codecov.io/gh/wemake-services/wemake-python-styleguide/commit/63782d55f804e3fd61da38031ef4ef663496b91d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wemake-services).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##            master     #3173   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          370       371    +1     
  Lines        12497     12483   -14     
  Branches       853       850    -3     
=========================================
- Hits         12497     12483   -14     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/wemake-services/wemake-python-styleguide/pull/3173?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wemake-services).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wemake-services).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c21a73f7..125dbcb5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -83,6 +83,7 @@ Semantic versioning in our case means:
   for `WPS211` complexity check anymore, #2394
 - `WPS303`: Underscore (`_`) now only allowed with 3 digits after it, #3120
 - Adds new rule to forbid `lambda` assigns to special attributes, #1733
+- Adds new rule to check problematic function params, #1343
 - Adds support to run `wemake-python-styleguide` as a `pre-commit` hook, #2588
 
 ### Bugfixes
diff --git a/wemake_python_styleguide/logic/tree/functions.py b/wemake_python_styleguide/logic/tree/functions.py
index 0d29d2c4..435174af 100644
--- a/wemake_python_styleguide/logic/tree/functions.py
+++ b/wemake_python_styleguide/logic/tree/functions.py
@@ -80,8 +80,7 @@ def is_generator(node: AnyFunctionDef) -> bool:
 
 def get_function_exit_nodes(node: AnyFunctionDef) -> _ControlTransferIterable:
     """Yields nodes that cause a control transfer from a function."""
-    control_transfer_nodes = (Return, Yield, YieldFrom)
     for body_item in node.body:
         for sub_node in walk(body_item):
-            if isinstance(sub_node, control_transfer_nodes):
+            if isinstance(sub_node, _AnyControlTransfers):
                 yield sub_node
diff --git a/wemake_python_styleguide/violations/best_practices.py b/wemake_python_styleguide/violations/best_practices.py
index b60821ed..723083ae 100644
--- a/wemake_python_styleguide/violations/best_practices.py
+++ b/wemake_python_styleguide/violations/best_practices.py
@@ -91,6 +91,7 @@ Summary
    GettingElementByUnpackingViolation
    WrongEmptyLinesCountViolation
    ImportObjectCollisionViolation
+   ProblematicFunctionParamsViolation
 
 Best practices
 --------------
@@ -170,6 +171,7 @@ Best practices
 .. autoclass:: GettingElementByUnpackingViolation
 .. autoclass:: WrongEmptyLinesCountViolation
 .. autoclass:: ImportObjectCollisionViolation
+.. autoclass:: ProblematicFunctionParamsViolation
 
 """
 
@@ -2875,3 +2877,37 @@ class ImportObjectCollisionViolation(ASTViolation):
 
     error_template = 'Found import object collision: {0}'
     code = 474
+
+
+@final
+class ProblematicFunctionParamsViolation(ASTViolation):
+    """
+    Do not use problematic function parameters.
+
+    Patterns that we don't allow:
+    - More than one pos-only params with defaults,
+      you cannot really pass the second one without passing the first one anyway
+    - Pos-only or regular param with default before `*args`,
+      this way you won't be able to pass just `*args`
+
+    Reasoning:
+        It would be hard to pass arguments to these functions.
+
+    Solution:
+        Change the signature: remove defaults or change the parameter kinds.
+
+    Example::
+
+        # Correct:
+        def function(first, second): ...
+
+        # Wrong:
+        def function(first=0, second=1, /): ...
+        def function(first=0, *args): ...
+
+    .. versionadded:: 1.0.0
+
+    """
+
+    error_template = 'Found problematic function parameters'
+    code = 475
diff --git a/wemake_python_styleguide/visitors/ast/functions.py b/wemake_python_styleguide/visitors/ast/functions.py
index cedacaf2..52c9781e 100644
--- a/wemake_python_styleguide/visitors/ast/functions.py
+++ b/wemake_python_styleguide/visitors/ast/functions.py
@@ -34,6 +34,7 @@ from wemake_python_styleguide.violations.best_practices import (
     ComplexDefaultValueViolation,
     FloatingNanViolation,
     GetterWithoutReturnViolation,
+    ProblematicFunctionParamsViolation,
     StopIterationInsideGeneratorViolation,
     WrongFunctionCallViolation,
 )
@@ -441,6 +442,7 @@ class FunctionSignatureVisitor(base.BaseNodeVisitor):
     ) -> None:
         """Checks function and lambda defs."""
         self._check_complex_argument_defaults(node)
+        self._check_problematic_params(node)
         if not isinstance(node, ast.Lambda):
             self._check_getter_without_return(node)
         self.generic_visit(node)
@@ -462,9 +464,6 @@ class FunctionSignatureVisitor(base.BaseNodeVisitor):
         if not has_explicit_function_exit:
             self.add_violation(GetterWithoutReturnViolation(node))
 
-    def _is_concrete_getter(self, node: AnyFunctionDef) -> bool:
-        return node.name.startswith('get_') and not stubs.is_stub(node)
-
     def _check_complex_argument_defaults(
         self,
         node: AnyFunctionDefAndLambda,
@@ -481,10 +480,7 @@ class FunctionSignatureVisitor(base.BaseNodeVisitor):
             real_arg = operators.unwrap_unary_node(arg)
             parts = (
                 attributes.parts(real_arg)
-                if isinstance(
-                    real_arg,
-                    ast.Attribute,
-                )
+                if isinstance(real_arg, ast.Attribute)
                 else [real_arg]
             )
 
@@ -493,3 +489,22 @@ class FunctionSignatureVisitor(base.BaseNodeVisitor):
                 for part in parts
             ):
                 self.add_violation(ComplexDefaultValueViolation(arg))
+
+    def _check_problematic_params(
+        self,
+        node: AnyFunctionDefAndLambda,
+    ) -> None:
+        is_problematic = False
+        if len(node.args.defaults) - len(node.args.args) >= 2:
+            # This means that we have at least 2 pos-only with defaults.
+            is_problematic = True
+        if node.args.defaults and node.args.vararg:
+            # Won't be able to pass only `*args`,
+            # will have to pass param before it.
+            is_problematic = True
+
+        if is_problematic:
+            self.add_violation(ProblematicFunctionParamsViolation(node))
+
+    def _is_concrete_getter(self, node: AnyFunctionDef) -> bool:
+        return node.name.startswith('get_') and not stubs.is_stub(node)


Test patch:
diff --git a/tests/fixtures/noqa/noqa.py b/tests/fixtures/noqa/noqa.py
index edecf72e..a22486fc 100644
--- a/tests/fixtures/noqa/noqa.py
+++ b/tests/fixtures/noqa/noqa.py
@@ -740,3 +740,7 @@ match user:
         raise ValueError(k)
     case [objs]:  # noqa: WPS110
         raise ValueError(objs)
+
+
+def pos_only_problem(first_argpm=0, second_argpm=1, /):  # noqa: WPS475
+    my_print(first_argpm, second_argpm)
diff --git a/tests/test_checker/test_noqa.py b/tests/test_checker/test_noqa.py
index 03339884..f277b50b 100644
--- a/tests/test_checker/test_noqa.py
+++ b/tests/test_checker/test_noqa.py
@@ -230,6 +230,7 @@ SHOULD_BE_RAISED = types.MappingProxyType(
         'WPS472': 1,
         'WPS473': 0,
         'WPS474': 1,
+        'WPS475': 1,
         'WPS500': 1,
         'WPS501': 1,
         'WPS502': 0,  # disabled since 1.0.0
diff --git a/tests/test_visitors/test_ast/test_functions/test_problematic_params.py b/tests/test_visitors/test_ast/test_functions/test_problematic_params.py
new file mode 100644
index 00000000..f93161c6
--- /dev/null
+++ b/tests/test_visitors/test_ast/test_functions/test_problematic_params.py
@@ -0,0 +1,114 @@
+import pytest
+
+from wemake_python_styleguide.violations.best_practices import (
+    ProblematicFunctionParamsViolation,
+)
+from wemake_python_styleguide.visitors.ast.functions import (
+    FunctionSignatureVisitor,
+)
+
+# Wrong:
+
+problematic_function1 = """
+def function(arg1, arg2=2, arg3=3, /):
+    ...
+"""
+
+problematic_function2 = """
+def function(arg1, arg2=2, arg3=3, arg4=4, /):
+    ...
+"""
+
+problematic_function3 = """
+def function(arg1, arg2=2, arg3=3, arg4=4, /, arg5=5):
+    ...
+"""
+
+problematic_function4 = """
+def function(arg1=1, /, *args):
+    ...
+"""
+
+problematic_function5 = """
+def function(arg1=1, *args):
+    ...
+"""
+
+problematic_function6 = 'lambda x=1, y=2, /: ...'
+
+# Correct:
+
+correct_function1 = """
+def function(arg1, *args):
+    ...
+"""
+
+correct_function2 = """
+def function(arg1, /, *args):
+    ...
+"""
+
+correct_function3 = """
+def function(arg1=1, /):
+    ...
+"""
+
+correct_function4 = """
+def function(arg1, arg2=2, /):
+    ...
+"""
+
+correct_function5 = 'lambda arg1, arg2=2, /: ...'
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        problematic_function1,
+        problematic_function2,
+        problematic_function3,
+        problematic_function4,
+        problematic_function5,
+        problematic_function6,
+    ],
+)
+def test_wrong_function_params(
+    assert_errors,
+    parse_ast_tree,
+    default_options,
+    code,
+    mode,
+):
+    """Testing that wrong function params are forbidden."""
+    tree = parse_ast_tree(mode(code))
+
+    visitor = FunctionSignatureVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [ProblematicFunctionParamsViolation])
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        correct_function1,
+        correct_function2,
+        correct_function3,
+        correct_function4,
+        correct_function5,
+    ],
+)
+def test_correct_function_params(
+    assert_errors,
+    parse_ast_tree,
+    default_options,
+    code,
+    mode,
+):
+    """Testing that wrong function params are forbidden."""
+    tree = parse_ast_tree(mode(code))
+
+    visitor = FunctionSignatureVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])