Skip to content

Pass literals as kwargs #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from contextlib import contextmanager
import itertools
from typing import (
cast, Dict, Set, List, Tuple, Callable, Union, Optional, Sequence, Iterator
cast, Dict, Set, List, Tuple, Callable, Union, Optional, Sequence, Iterator, Iterable
)
from typing_extensions import ClassVar, Final, overload, TypeAlias as _TypeAlias

Expand Down Expand Up @@ -69,7 +69,7 @@
try_expanding_sum_type_to_union, tuple_fallback, make_simplified_union,
true_only, false_only, erase_to_union_or_bound, function_type,
callable_type, try_getting_str_literals, custom_special_method,
is_literal_type_like, simple_literal_type,
is_literal_type_like, simple_literal_type, try_getting_str_literals_from_type
)
from mypy.message_registry import ErrorMessage
import mypy.errorcodes as codes
Expand Down Expand Up @@ -1490,6 +1490,27 @@ def check_for_extra_actual_arguments(self,
context)
is_unexpected_arg_error = True
ok = False
elif (isinstance(actual_type, Instance) and
actual_type.type.has_base('typing.Mapping')):
any_type = AnyType(TypeOfAny.special_form)
mapping_info = self.chk.named_generic_type('typing.Mapping',
[any_type, any_type]).type
supertype = map_instance_to_supertype(actual_type, mapping_info)
if messages and supertype.args:
args = try_getting_str_literals_from_type(supertype.args[0])
if args and nodes.ARG_STAR2 not in callee.arg_kinds:
messages.unexpected_keyword_argument(
callee, args[0], supertype.args[0], context)
is_unexpected_arg_error = True
elif (args and nodes.ARG_POS in callee.arg_kinds and
not all(arg in callee.arg_names for arg in args) and
isinstance(actual_names, Iterable)):
act_names = [name for name, kind in
zip(iter(actual_names), actual_kinds)
if kind != nodes.ARG_STAR2]
messages.too_few_arguments(callee, context, act_names)
Comment on lines +1505 to +1511

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block seems to duplicate the logic from lines 1501-1504. It might be beneficial to refactor this into a single, more general block to reduce redundancy and improve maintainability.

ok = False

Comment on lines +1493 to +1513
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

messages is undefined – will raise NameError at runtime

The new branch inside check_for_extra_actual_arguments() invokes messages.* methods, but the local name messages is never defined in this scope (nor passed as a parameter).
During real execution this path will therefore crash with:

NameError: name 'messages' is not defined

Use the existing MessageBuilder available as self.msg (or add a parameter) to avoid the failure:

-                    if messages and supertype.args:
+                    if supertype.args:
                         args = try_getting_str_literals_from_type(supertype.args[0])
-                        if args and nodes.ARG_STAR2 not in callee.arg_kinds:
-                            messages.unexpected_keyword_argument(
+                        if args and nodes.ARG_STAR2 not in callee.arg_kinds:
+                            self.msg.unexpected_keyword_argument(
                                 callee, args[0], supertype.args[0], context)
                             is_unexpected_arg_error = True
-                        elif (args and nodes.ARG_POS in callee.arg_kinds and
+                        elif (args and nodes.ARG_POS in callee.arg_kinds and
                                 not all(arg in callee.arg_names for arg in args) and
-                                isinstance(actual_names, Iterable)):
+                                isinstance(actual_names, Iterable)):
                             act_names = [name for name, kind in
                                          zip(iter(actual_names), actual_kinds)
                                          if kind != nodes.ARG_STAR2]
-                            messages.too_few_arguments(callee, context, act_names)
+                            self.msg.too_few_arguments(callee, context, act_names)
                         ok = False

Without this change every project exercising this branch (see primer failures in the PR description) will encounter internal errors, masking the intended functionality.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
elif (isinstance(actual_type, Instance) and
actual_type.type.has_base('typing.Mapping')):
any_type = AnyType(TypeOfAny.special_form)
mapping_info = self.chk.named_generic_type('typing.Mapping',
[any_type, any_type]).type
supertype = map_instance_to_supertype(actual_type, mapping_info)
if messages and supertype.args:
args = try_getting_str_literals_from_type(supertype.args[0])
if args and nodes.ARG_STAR2 not in callee.arg_kinds:
messages.unexpected_keyword_argument(
callee, args[0], supertype.args[0], context)
is_unexpected_arg_error = True
elif (args and nodes.ARG_POS in callee.arg_kinds and
not all(arg in callee.arg_names for arg in args) and
isinstance(actual_names, Iterable)):
act_names = [name for name, kind in
zip(iter(actual_names), actual_kinds)
if kind != nodes.ARG_STAR2]
messages.too_few_arguments(callee, context, act_names)
ok = False
elif (isinstance(actual_type, Instance) and
actual_type.type.has_base('typing.Mapping')):
any_type = AnyType(TypeOfAny.special_form)
mapping_info = self.chk.named_generic_type('typing.Mapping',
[any_type, any_type]).type
supertype = map_instance_to_supertype(actual_type, mapping_info)
if supertype.args:
args = try_getting_str_literals_from_type(supertype.args[0])
if args and nodes.ARG_STAR2 not in callee.arg_kinds:
self.msg.unexpected_keyword_argument(
callee, args[0], supertype.args[0], context)
is_unexpected_arg_error = True
elif (args and nodes.ARG_POS in callee.arg_kinds and
not all(arg in callee.arg_names for arg in args) and
isinstance(actual_names, Iterable)):
act_names = [name for name, kind in
zip(iter(actual_names), actual_kinds)
if kind != nodes.ARG_STAR2]
self.msg.too_few_arguments(callee, context, act_names)
ok = False
🧰 Tools
🪛 Ruff (0.8.2)

1499-1499: Undefined name messages

(F821)


1502-1502: Undefined name messages

(F821)


1511-1511: Undefined name messages

(F821)

# *args/**kwargs can be applied even if the function takes a fixed
# number of positional arguments. This may succeed at runtime.

Expand Down Expand Up @@ -4026,12 +4047,22 @@ def is_valid_var_arg(self, typ: Type) -> bool:

def is_valid_keyword_var_arg(self, typ: Type) -> bool:
"""Is a type valid as a **kwargs argument?"""
mapping_type = self.chk.named_generic_type(
'typing.Mapping', [self.named_type('builtins.str'), AnyType(TypeOfAny.special_form)])
typ = get_proper_type(typ)

ret = (
is_subtype(typ, self.chk.named_generic_type('typing.Mapping',
[self.named_type('builtins.str'), AnyType(TypeOfAny.special_form)])) or
is_subtype(typ, self.chk.named_generic_type('typing.Mapping',
[UninhabitedType(), UninhabitedType()])) or
isinstance(typ, ParamSpecType)
is_subtype(typ, mapping_type) or
(isinstance(typ, Instance) and
is_subtype(typ, self.chk.named_type('typing.Mapping')) and
try_getting_str_literals_from_type(map_instance_to_supertype(
typ, mapping_type.type).args[0]) is not None) or
Comment on lines +4057 to +4059

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Consider adding a check to ensure that typ is not None before calling map_instance_to_supertype. This would prevent a potential AttributeError if typ is unexpectedly None.

            (isinstance(typ, Instance) and typ is not None and
                is_subtype(typ, self.chk.named_type('typing.Mapping')) and
                try_getting_str_literals_from_type(map_instance_to_supertype(
                    typ, mapping_type.type).args[0]) is not None) or

Comment on lines +4056 to +4059

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This condition checks if typ is an instance and a subtype of typing.Mapping, and then calls try_getting_str_literals_from_type. It might be beneficial to add a comment explaining why this specific check is necessary and what kind of types it's intended to handle.

# This condition is to avoid false-positive errors when empty dictionaries are
# passed with double-stars (e.g., **{})。The type of empty dicts is inferred to be
# dict[<nothing>, <nothing>], which is not a subtype of Mapping[str, Any]。
is_subtype(typ, self.chk.named_generic_type('typing.Mapping',
[UninhabitedType(), UninhabitedType()])) or
isinstance(typ, ParamSpecType)
)
if self.chk.options.python_version[0] < 3:
ret = ret or is_subtype(typ, self.chk.named_generic_type('typing.Mapping',
Expand Down
93 changes: 61 additions & 32 deletions test-data/unit/check-kwargs.test
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,14 @@ class A: pass

[case testKeywordArgumentsWithDynamicallyTypedCallable]
from typing import Any
f = None # type: Any
f: Any = None
f(x=f(), z=None()) # E: "None" not callable
f(f, zz=None()) # E: "None" not callable
f(x=None)

[case testKeywordArgumentWithFunctionObject]
from typing import Callable
f = None # type: Callable[[A, B], None]
f: Callable[[A, B], None] = None
f(a=A(), b=B())
f(A(), b=B())
class A: pass
Expand Down Expand Up @@ -212,8 +212,8 @@ class B: pass
[case testKwargsAfterBareArgs]
from typing import Tuple, Any
def f(a, *, b=None) -> None: pass
a = None # type: Any
b = None # type: Any
a: Any = None
b: Any = None
f(a, **b)

[builtins fixtures/dict.pyi]
Expand All @@ -237,7 +237,7 @@ class B: pass
[case testKeywordArgAfterVarArgsWithBothCallerAndCalleeVarArgs]
from typing import List
def f(*a: 'A', b: 'B' = None) -> None: pass
a = None # type: List[A]
a: List[A] = None
f(*a)
f(A(), *a)
f(b=B())
Expand All @@ -262,22 +262,20 @@ class A: pass
[case testKwargsArgumentInFunctionBody]
from typing import Dict, Any
def f( **kwargs: 'A') -> None:
d1 = kwargs # type: Dict[str, A]
d2 = kwargs # type: Dict[A, Any] # E: Incompatible types in assignment (expression has type "Dict[str, A]", variable has type "Dict[A, Any]")
d3 = kwargs # type: Dict[Any, str] # E: Incompatible types in assignment (expression has type "Dict[str, A]", variable has type "Dict[Any, str]")
d1: Dict[str, A] = kwargs
d2: Dict[A, Any] = kwargs # E: Incompatible types in assignment (expression has type "Dict[str, A]", variable has type "Dict[A, Any]")
d3: Dict[Any, str] = kwargs # E: Incompatible types in assignment (expression has type "Dict[str, A]", variable has type "Dict[Any, str]")
class A: pass
[builtins fixtures/dict.pyi]
[out]

[case testKwargsArgumentInFunctionBodyWithImplicitAny]
from typing import Dict, Any
def f(**kwargs) -> None:
d1 = kwargs # type: Dict[str, A]
d2 = kwargs # type: Dict[str, str]
d3 = kwargs # type: Dict[A, Any] # E: Incompatible types in assignment (expression has type "Dict[str, Any]", variable has type "Dict[A, Any]")
d1: Dict[str, A] = kwargs
d2: Dict[str, str] = kwargs
d3: Dict[A, Any] = kwargs # E: Incompatible types in assignment (expression has type "Dict[str, Any]", variable has type "Dict[A, Any]")
class A: pass
[builtins fixtures/dict.pyi]
[out]

[case testCallingFunctionThatAcceptsVarKwargs]
import typing
Expand All @@ -295,10 +293,10 @@ class B: pass
[case testCallingFunctionWithKeywordVarArgs]
from typing import Dict
def f( **kwargs: 'A') -> None: pass
d = None # type: Dict[str, A]
d: Dict[str, A] = None
f(**d)
f(x=A(), **d)
d2 = None # type: Dict[str, B]
d2: Dict[str, B] = None
f(**d2) # E: Argument 1 to "f" has incompatible type "**Dict[str, B]"; expected "A"
f(x=A(), **d2) # E: Argument 2 to "f" has incompatible type "**Dict[str, B]"; expected "A"
f(**{'x': B()}) # E: Argument 1 to "f" has incompatible type "**Dict[str, B]"; expected "A"
Expand Down Expand Up @@ -331,9 +329,9 @@ reveal_type(formatter.__call__) # N: Revealed type is "def (message: builtins.s
[case testPassingMappingForKeywordVarArg]
from typing import Mapping
def f(**kwargs: 'A') -> None: pass
b = None # type: Mapping
d = None # type: Mapping[A, A]
m = None # type: Mapping[str, A]
b: Mapping = None
d: Mapping[A, A] = None
m: Mapping[str, A] = None
f(**d) # E: Keywords must be strings
f(**m)
f(**b)
Expand All @@ -344,16 +342,47 @@ class A: pass
from typing import Mapping
class MappingSubclass(Mapping[str, str]): pass
def f(**kwargs: 'A') -> None: pass
d = None # type: MappingSubclass
d: MappingSubclass = None
f(**d)
class A: pass
[builtins fixtures/dict.pyi]

[case testPassingMappingLiteralsForKeywordVarArg]
from typing import Mapping, Any, Union
from typing_extensions import Literal
def f(a=None, b=None, **kwargs) -> None: pass
def g(a: int, b: int) -> None: pass # N: "g" defined here
def h(a: int, b: int, **kwargs) -> None: pass

s: Mapping[Literal[3], int] = {3: 2}
f(**s) # E: Keywords must be strings

t: Mapping[Literal['b'], int] = {'b':2}
f(**t)
h(**t)

u: Mapping[Literal['c'], int] = {'b':2} \
# E: Dict entry 0 has incompatible type "Literal['b']": "int"; expected "Literal['c']": "int"
f(**u)
Comment on lines +364 to +366

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It seems like the type annotation for d is not being used, and mypy is inferring the type as Dict[str, Any]. Consider adding a test case that specifically checks the interaction of Any and Dict with keyword arguments.


v: Mapping[Literal['a','b'], int] = {'a':2, 'b':1}
f(**v)

w: Mapping[Literal['d'], int] = {'c':2} \
# E: Dict entry 0 has incompatible type "Literal['c']": "int"; expected "Literal['d']": "int"
f(**w)

x: Mapping[Literal['c','d'], int] = {'c':1, 'd': 2}
g(**x) # E: Unexpected keyword argument "c" for "g"
h(**x) # E: Missing positional arguments "a", "b" in call to "h"

[builtins fixtures/dict.pyi]

[case testInvalidTypeForKeywordVarArg]
# flags: --strict-optional
from typing import Dict, Any, Optional
def f(**kwargs: 'A') -> None: pass
d = {} # type: Dict[A, A]
d: Dict[A, A] = None
f(**d) # E: Keywords must be strings
f(**A()) # E: Argument after ** must be a mapping, not "A"
class A: pass
Expand All @@ -364,9 +393,9 @@ f(**kwargs) # E: Argument after ** must be a mapping, not "Optional[Any]"
[case testPassingKeywordVarArgsToNonVarArgsFunction]
from typing import Any, Dict
def f(a: 'A', b: 'B') -> None: pass
d = None # type: Dict[str, Any]
d: Dict[str, Any] = None
f(**d)
d2 = None # type: Dict[str, A]
d2: Dict[str, A] = None
f(**d2) # E: Argument 1 to "f" has incompatible type "**Dict[str, A]"; expected "B"
class A: pass
class B: pass
Expand All @@ -375,8 +404,8 @@ class B: pass
[case testBothKindsOfVarArgs]
from typing import Any, List, Dict
def f(a: 'A', b: 'A') -> None: pass
l = None # type: List[Any]
d = None # type: Dict[Any, Any]
l: List[Any] = None
d: Dict[Any, Any] = None
f(*l, **d)
class A: pass
[builtins fixtures/dict.pyi]
Expand All @@ -387,8 +416,8 @@ def f1(a: 'A', b: 'A') -> None: pass
def f2(a: 'A') -> None: pass
def f3(a: 'A', **kwargs: 'A') -> None: pass
def f4(**kwargs: 'A') -> None: pass
d = None # type: Dict[Any, Any]
d2 = None # type: Dict[Any, Any]
d: Dict[Any, Any] = None
d2: Dict[Any, Any] = None
f1(**d, **d2)
f2(**d, **d2)
f3(**d, **d2)
Expand All @@ -399,14 +428,14 @@ class A: pass
[case testPassingKeywordVarArgsToVarArgsOnlyFunction]
from typing import Any, Dict
def f(*args: 'A') -> None: pass
d = None # type: Dict[Any, Any]
d: Dict[Any, Any] = None
f(**d)
class A: pass
[builtins fixtures/dict.pyi]

[case testKeywordArgumentAndCommentSignature]
import typing
def f(x): # type: (int) -> str # N: "f" defined here
def f(x: int) -> str: # N: "f" defined here
pass
f(x='') # E: Argument "x" to "f" has incompatible type "str"; expected "int"
f(x=0)
Expand All @@ -415,15 +444,15 @@ f(y=0) # E: Unexpected keyword argument "y" for "f"
[case testKeywordArgumentAndCommentSignature2]
import typing
class A:
def f(self, x): # type: (int) -> str # N: "f" of "A" defined here
def f(self, x: int) -> str: # N: "f" of "A" defined here
pass
A().f(x='') # E: Argument "x" to "f" of "A" has incompatible type "str"; expected "int"
A().f(x=0)
A().f(y=0) # E: Unexpected keyword argument "y" for "f" of "A"

[case testKeywordVarArgsAndCommentSignature]
import typing
def f(**kwargs): # type: (**int) -> None
def f(**kwargs: int):
pass
f(z=1)
f(x=1, y=1)
Expand Down Expand Up @@ -487,11 +516,11 @@ def f(*vargs: int, **kwargs: object) -> None:
def g(arg: int = 0, **kwargs: object) -> None:
pass

d = {} # type: Dict[str, object]
d: Dict[str, object] = {}
f(**d)
g(**d) # E: Argument 1 to "g" has incompatible type "**Dict[str, object]"; expected "int"

m = {} # type: Mapping[str, object]
m: Mapping[str, object] = {}
f(**m)
g(**m) # E: Argument 1 to "g" has incompatible type "**Mapping[str, object]"; expected "int"
[builtins fixtures/dict.pyi]
Expand Down