diff --git a/CHANGELOG.md b/CHANGELOG.md index ba551989..ed74adcf 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/returns/contrib/mypy/_features/partial.py b/returns/contrib/mypy/_features/partial.py index 9d24489a..eb08f20e 100644 --- a/returns/contrib/mypy/_features/partial.py +++ b/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) diff --git a/returns/contrib/mypy/_typeops/transform_callable.py b/returns/contrib/mypy/_typeops/transform_callable.py index 360a019d..c379284a 100644 --- a/returns/contrib/mypy/_typeops/transform_callable.py +++ b/returns/contrib/mypy/_typeops/transform_callable.py @@ -1,6 +1,14 @@ -from typing import ClassVar, final +from typing import ClassVar, Final, 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,19 @@ 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: Final = frozenset((ARG_STAR, ARG_STAR2)) + +#: Kinds of arguments that can be passed positionally. +_POSITIONAL_KINDS: Final = frozenset((ARG_POS, ARG_OPT)) + +#: Maps a positional argument kind onto its keyword-only counterpart. +_KEYWORD_ONLY_KINDS: Final = { + ARG_POS: ARG_NAMED, + ARG_OPT: ARG_NAMED_OPT, +} + def proper_type( case_functions: list[CallableType], @@ -128,36 +149,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 _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 = _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] ) diff --git a/typesafety/test_curry/test_partial/test_partial.yml b/typesafety/test_curry/test_partial/test_partial.yml index 9a7fa430..1b017436 100644 --- a/typesafety/test_curry/test_partial/test_partial.yml +++ b/typesafety/test_curry/test_partial/test_partial.yml @@ -42,6 +42,92 @@ reveal_type(partial(two_args, second=1.0)) # N: Revealed type is "def (first: int) -> str" +- case: partial_last_arg_by_keyword_keeps_first_positional + disable_cache: false + main: | + from returns.curry import partial + + def two_args(first: int, second: float) -> str: + ... + + # `second` is the last parameter, so binding it by keyword leaves no + # positional parameter after the hole and `first` stays positional: + reveal_type(partial(two_args, second=2)) # 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: |