Fix partial mypy plugin: make positionals keyword-only after a keyword-bound positional (#2191)

CHANGELOG.md

@@ -6,6 +6,16 @@ incremental in minor, bugfixes only are patches.
 See [0Ver](https://0ver.org/).
 
 
+## 0.28.1 WIP
+
+### Bugfixes
+
+- Fixes `partial` mypy plugin inferring the wrong signature when a positional
+  argument is applied by keyword: the remaining parameters are now correctly
+  marked as keyword-only, so passing them positionally is a type error instead
+  of a runtime `TypeError`
+
+
 ## 0.28.0
 
 ### Features

returns/contrib/mypy/_features/partial.py

@@ -169,7 +169,7 @@ def _create_partial_case(
         fallback: CallableType,
     ) -> CallableType:
         partial = CallableInference(
-            Functions(case_function, intermediate).diff(),
+            Functions(case_function, intermediate).diff(self._applied_args),
             self._ctx,
             fallback=fallback,
         ).from_usage(self._applied_args)

returns/contrib/mypy/_typeops/transform_callable.py

@@ -1,6 +1,14 @@
 from typing import ClassVar, final
 
-from mypy.nodes import ARG_OPT, ARG_POS, ARG_STAR, ARG_STAR2, ArgKind
+from mypy.nodes import (
+    ARG_NAMED,
+    ARG_NAMED_OPT,
+    ARG_OPT,
+    ARG_POS,
+    ARG_STAR,
+    ARG_STAR2,
+    ArgKind,
+)
 from mypy.typeops import get_type_vars
 from mypy.types import (
     AnyType,
@@ -14,6 +22,10 @@
 
 from returns.contrib.mypy._structures.args import FuncArg
 
+#: Kinds of arguments that consume the leftover positional or keyword
+#: arguments (``*args`` and ``**kwargs``) and therefore cannot be applied.
+_VARIADIC_KINDS: frozenset[ArgKind] = frozenset((ARG_STAR, ARG_STAR2))
+
 
 def proper_type(
     case_functions: list[CallableType],
@@ -119,6 +131,18 @@ class Functions:
     For example, one can need a diff of two callables.
     """
 
+    #: Kinds of arguments that can be passed positionally.
+    _positional_kinds: ClassVar[frozenset[ArgKind]] = frozenset((
+        ARG_POS,
+        ARG_OPT,
+    ))
+
+    #: Maps a positional argument kind onto its keyword-only counterpart.
+    _keyword_only_kinds: ClassVar[dict[ArgKind, ArgKind]] = {
+        ARG_POS: ARG_NAMED,
+        ARG_OPT: ARG_NAMED_OPT,
+    }
+
     def __init__(
         self,
         original: CallableType,
@@ -128,36 +152,80 @@ def __init__(
         self._original = original
         self._intermediate = intermediate
 
-    def diff(self) -> CallableType:
+    def diff(self, applied_args: list[FuncArg]) -> CallableType:
         """Finds a diff between two functions' arguments."""
         intermediate_names = [
             arg.name for arg in FuncArg.from_callable(self._intermediate)
         ]
-        new_function_args = []
+        original_args = FuncArg.from_callable(self._original)
+        # A positional parameter applied by keyword leaves a hole that can no
+        # longer be filled positionally, so every remaining positional
+        # parameter after it must become keyword-only. Otherwise the partial
+        # would accept positional arguments that raise a ``TypeError`` at
+        # runtime. See issue #2191.
+        keyword_hole_at = self._keyword_hole_at(
+            original_args,
+            applied_args,
+            intermediate_names,
+        )
+        return Intermediate(self._original).with_signature([
+            self._remaining_arg(arg, keyword_only=index > keyword_hole_at)
+            for index, arg in enumerate(original_args)
+            if not self._was_applied(arg, index, intermediate_names)
+        ])
 
-        for index, arg in enumerate(FuncArg.from_callable(self._original)):
-            should_be_copied = (
-                arg.kind in {ARG_STAR, ARG_STAR2}
-                or arg.name not in intermediate_names
-                or
-                # We need to treat unnamed args differently, because python3.8
-                # has pos_only_args, all their names are `None`.
-                # This is also true for `lambda` functions where `.name`
-                # might be missing for some reason.
-                (
-                    not arg.name
-                    and not (
-                        index < len(intermediate_names)
-                        and
-                        # If this is also unnamed arg, then ignoring it.
-                        not intermediate_names[index]
-                    )
-                )
-            )
-            if should_be_copied:
-                new_function_args.append(arg)
-        return Intermediate(self._original).with_signature(
-            new_function_args,
+    def _keyword_hole_at(
+        self,
+        original_args: list[FuncArg],
+        applied_args: list[FuncArg],
+        intermediate_names: list[str | None],
+    ) -> int:
+        """Index of the first positional parameter applied by keyword.
+
+        Positional arguments always fill the leading positional parameters,
+        so a positional parameter applied beyond that prefix was applied by
+        keyword. Returns an index past the end when there is no such hole.
+        """
+        positional_prefix = sum(
+            applied.name is None and applied.kind not in _VARIADIC_KINDS
+            for applied in applied_args
+        )
+        seen_positional = 0
+        for index, arg in enumerate(original_args):
+            if arg.kind not in self._positional_kinds:
+                continue
+            applied = self._was_applied(arg, index, intermediate_names)
+            if applied and seen_positional >= positional_prefix:
+                return index
+            seen_positional += 1
+        return len(original_args)
+
+    def _remaining_arg(self, arg: FuncArg, *, keyword_only: bool) -> FuncArg:
+        """Copies an unapplied argument, making it keyword-only if needed."""
+        keyword_kind = self._keyword_only_kinds.get(arg.kind)
+        if keyword_only and keyword_kind is not None:
+            return FuncArg(arg.name, arg.type, keyword_kind)
+        return arg
+
+    def _was_applied(
+        self,
+        arg: FuncArg,
+        index: int,
+        intermediate_names: list[str | None],
+    ) -> bool:
+        """Tells whether an original argument was already applied."""
+        if arg.kind in _VARIADIC_KINDS:
+            return False
+        if arg.name is not None:
+            return arg.name in intermediate_names
+        # We need to treat unnamed args differently, because python3.8
+        # has pos_only_args, all their names are `None`.
+        # This is also true for `lambda` functions where `.name`
+        # might be missing for some reason.
+        return (
+            index < len(intermediate_names)
+            # If this is also an unnamed arg, then it was applied.
+            and not intermediate_names[index]
         )
 
 

typesafety/test_curry/test_partial/test_partial.yml

@@ -42,6 +42,79 @@
     reveal_type(partial(two_args, second=1.0))  # N: Revealed type is "def (first: int) -> str"
 
 
+- case: partial_positional_arg_by_keyword_makes_rest_kwonly
+  disable_cache: false
+  main: |
+    from returns.curry import partial
+
+    def two_args(first: int, second: float) -> str:
+        ...
+
+    # `first` is a positional argument bound by keyword, so `second`
+    # can no longer be passed positionally at runtime and must be
+    # reported as keyword-only:
+    reveal_type(partial(two_args, first=1))  # N: Revealed type is "def (*, second: float) -> str"
+
+
+- case: partial_keyword_only_arg_stays_keyword_only
+  disable_cache: false
+  main: |
+    from returns.curry import partial
+
+    def two_args(first: int, *, second: float) -> str:
+        ...
+
+    # `second` is already keyword-only, so binding the positional `first`
+    # by keyword leaves it keyword-only just as before:
+    reveal_type(partial(two_args, first=1))  # N: Revealed type is "def (*, second: float) -> str"
+
+
+- case: partial_positional_arg_by_keyword_rejects_positional_call
+  disable_cache: false
+  main: |
+    from returns.curry import partial
+
+    def two_args(first: int, second: float) -> str:
+        ...
+
+    bound = partial(two_args, first=1)
+    bound(second=2.0)  # ok
+    bound(2.0)  # E: Too many positional arguments  [misc]
+
+
+- case: partial_middle_positional_arg_by_keyword_makes_rest_kwonly
+  disable_cache: false
+  main: |
+    from returns.curry import partial
+
+    def multiple(
+        first: int,
+        second: float,
+        third: str,
+        fourth: bool,
+    ) -> str:
+        ...
+
+    # `second` is bound by keyword, so every positional argument after
+    # it (`third`, `fourth`) becomes keyword-only, while `first` which
+    # comes before the hole is still reachable positionally:
+    reveal_type(partial(multiple, second=0.5))  # N: Revealed type is "def (first: int, *, third: str, fourth: bool) -> str"
+
+
+- case: partial_positional_only_before_keyword_bound_arg
+  disable_cache: false
+  main: |
+    from returns.curry import partial
+
+    def multiple(first: int, /, second: int, third: str) -> str:
+        ...
+
+    # `second` is bound by keyword, so `third` after the hole becomes
+    # keyword-only, while the positional-only `first` before the hole
+    # stays positional-only (rendered by mypy without a name):
+    reveal_type(partial(multiple, second=1))  # N: Revealed type is "def (int, *, third: str) -> str"
+
+
 - case: partial_multiple_args
   disable_cache: false
   main: |