Skip to content

Commit

Permalink
Properly track positional-only arguments for unannotated functions (p…
Browse files Browse the repository at this point in the history
…ython#10802)

I was originally planning to address this by adding an ARG_POS_ONLY
kind (as a concrete win for what I hoped would be a generally positive
refactor), but it was actually really easy to fix it on its own, and
then we can address the refactor purely on its own merits.
  • Loading branch information
msullivan authored Jul 15, 2021
1 parent ed09f8d commit d180456
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 32 deletions.
40 changes: 23 additions & 17 deletions mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,14 +522,13 @@ def do_func_def(self, n: Union[ast3.FunctionDef, ast3.AsyncFunctionDef],

lineno = n.lineno
args = self.transform_args(n.args, lineno, no_type_check=no_type_check)
if special_function_elide_names(n.name):
for arg in args:
arg.pos_only = True

posonlyargs = [arg.arg for arg in getattr(n.args, "posonlyargs", [])]
arg_kinds = [arg.kind for arg in args]
arg_names: List[Optional[str]] = [arg.variable.name for arg in args]
arg_names = [None if argument_elide_name(name) or name in posonlyargs else name
for name in arg_names]
if special_function_elide_names(n.name):
arg_names = [None] * len(arg_names)
arg_names = [None if arg.pos_only else arg.variable.name for arg in args]

arg_types: List[Optional[Type]] = []
if no_type_check:
arg_types = [None] * len(args)
Expand Down Expand Up @@ -602,10 +601,11 @@ def do_func_def(self, n: Union[ast3.FunctionDef, ast3.AsyncFunctionDef],
AnyType(TypeOfAny.unannotated),
_dummy_fallback)

func_def = FuncDef(n.name,
args,
self.as_required_block(n.body, lineno),
func_type)
func_def = FuncDef(
n.name,
args,
self.as_required_block(n.body, lineno),
func_type)
if isinstance(func_def.type, CallableType):
# semanal.py does some in-place modifications we want to avoid
func_def.unanalyzed_type = func_def.type.copy_modified()
Expand Down Expand Up @@ -660,17 +660,20 @@ def transform_args(self,
) -> List[Argument]:
new_args = []
names: List[ast3.arg] = []
args_args = getattr(args, "posonlyargs", cast(List[ast3.arg], [])) + args.args
posonlyargs = getattr(args, "posonlyargs", cast(List[ast3.arg], []))
args_args = posonlyargs + args.args
args_defaults = args.defaults
num_no_defaults = len(args_args) - len(args_defaults)
# positional arguments without defaults
for a in args_args[:num_no_defaults]:
new_args.append(self.make_argument(a, None, ARG_POS, no_type_check))
for i, a in enumerate(args_args[:num_no_defaults]):
pos_only = i < len(posonlyargs)
new_args.append(self.make_argument(a, None, ARG_POS, no_type_check, pos_only))
names.append(a)

# positional arguments with defaults
for a, d in zip(args_args[num_no_defaults:], args_defaults):
new_args.append(self.make_argument(a, d, ARG_OPT, no_type_check))
for i, (a, d) in enumerate(zip(args_args[num_no_defaults:], args_defaults)):
pos_only = num_no_defaults + i < len(posonlyargs)
new_args.append(self.make_argument(a, d, ARG_OPT, no_type_check, pos_only))
names.append(a)

# *arg
Expand All @@ -697,7 +700,7 @@ def transform_args(self,
return new_args

def make_argument(self, arg: ast3.arg, default: Optional[ast3.expr], kind: ArgKind,
no_type_check: bool) -> Argument:
no_type_check: bool, pos_only: bool = False) -> Argument:
if no_type_check:
arg_type = None
else:
Expand All @@ -710,7 +713,10 @@ def make_argument(self, arg: ast3.arg, default: Optional[ast3.expr], kind: ArgKi
arg_type = TypeConverter(self.errors, line=arg.lineno).visit(annotation)
else:
arg_type = self.translate_type_comment(arg, type_comment)
return Argument(Var(arg.arg), arg_type, self.visit(default), kind)
if argument_elide_name(arg.arg):
pos_only = True

return Argument(Var(arg.arg), arg_type, self.visit(default), kind, pos_only)

def fail_arg(self, msg: str, arg: ast3.arg) -> None:
self.fail(msg, arg.lineno, arg.col_offset)
Expand Down
12 changes: 8 additions & 4 deletions mypy/fastparse2.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,12 @@ def visit_FunctionDef(self, n: ast27.FunctionDef) -> Statement:
converter = TypeConverter(self.errors, line=lineno, override_column=n.col_offset,
assume_str_is_unicode=self.unicode_literals)
args, decompose_stmts = self.transform_args(n.args, lineno)
if special_function_elide_names(n.name):
for arg in args:
arg.pos_only = True

arg_kinds = [arg.kind for arg in args]
arg_names: List[Optional[str]] = [arg.variable.name for arg in args]
arg_names = [None if argument_elide_name(name) else name for name in arg_names]
if special_function_elide_names(n.name):
arg_names = [None] * len(arg_names)
arg_names = [None if arg.pos_only else arg.variable.name for arg in args]

arg_types: List[Optional[Type]] = []
type_comment = n.type_comment
Expand Down Expand Up @@ -518,6 +518,10 @@ def transform_args(self,
new_args.append(Argument(Var(n.kwarg), typ, None, ARG_STAR2))
names.append(n.kwarg)

for arg in new_args:
if argument_elide_name(arg.variable.name):
arg.pos_only = True

# We don't have any context object to give, but we have closed around the line num
def fail_arg(msg: str, arg: None) -> None:
self.fail(msg, line, 0)
Expand Down
4 changes: 2 additions & 2 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -1939,9 +1939,9 @@ def [T <: int] f(self, x: int, y: T) -> None

# If we got a "special arg" (i.e: self, cls, etc...), prepend it to the arg list
if isinstance(tp.definition, FuncDef) and tp.definition.name is not None:
definition_args = tp.definition.arg_names
definition_args = [arg.variable.name for arg in tp.definition.arguments]
if definition_args and tp.arg_names != definition_args \
and len(definition_args) > 0:
and len(definition_args) > 0 and definition_args[0]:
if s:
s = ', ' + s
s = definition_args[0] + s
Expand Down
8 changes: 5 additions & 3 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,18 +561,20 @@ def deserialize(cls, data: JsonDict) -> 'OverloadedFuncDef':
class Argument(Node):
"""A single argument in a FuncItem."""

__slots__ = ('variable', 'type_annotation', 'initializer', 'kind')
__slots__ = ('variable', 'type_annotation', 'initializer', 'kind', 'pos_only')

def __init__(self,
variable: 'Var',
type_annotation: 'Optional[mypy.types.Type]',
initializer: Optional[Expression],
kind: 'ArgKind') -> None:
kind: 'ArgKind',
pos_only: bool = False) -> None:
super().__init__()
self.variable = variable
self.type_annotation = type_annotation
self.initializer = initializer
self.kind = kind # must be an ARG_* constant
self.pos_only = pos_only

def set_line(self,
target: Union[Context, int],
Expand Down Expand Up @@ -619,7 +621,7 @@ def __init__(self,
typ: 'Optional[mypy.types.FunctionLike]' = None) -> None:
super().__init__()
self.arguments = arguments
self.arg_names = [arg.variable.name for arg in self.arguments]
self.arg_names = [None if arg.pos_only else arg.variable.name for arg in arguments]
self.arg_kinds: List[ArgKind] = [arg.kind for arg in self.arguments]
self.max_pos: int = (
self.arg_kinds.count(ARG_POS) + self.arg_kinds.count(ARG_OPT))
Expand Down
3 changes: 1 addition & 2 deletions mypy/typeops.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
)
from mypy.maptype import map_instance_to_supertype
from mypy.expandtype import expand_type_by_instance, expand_type
from mypy.sharedparse import argument_elide_name

from mypy.typevars import fill_typevars

Expand Down Expand Up @@ -564,7 +563,7 @@ def callable_type(fdef: FuncItem, fallback: Instance,
return CallableType(
args,
fdef.arg_kinds,
[None if argument_elide_name(n) else n for n in fdef.arg_names],
fdef.arg_names,
ret_type or AnyType(TypeOfAny.unannotated),
fallback,
name=fdef.name,
Expand Down
2 changes: 1 addition & 1 deletion mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ def __init__(self,
# after serialization, but it is useful in error messages.
# TODO: decide how to add more info here (file, line, column)
# without changing interface hash.
self.def_extras = {'first_arg': definition.arg_names[0]
self.def_extras = {'first_arg': definition.arguments[0].variable.name
if definition.arg_names and definition.info and
not definition.is_static else None}
else:
Expand Down
15 changes: 14 additions & 1 deletion mypyc/irbuild/mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,21 @@ def fdef_to_sig(self, fdef: FuncDef) -> FuncSignature:
else:
ret = object_rprimitive

# mypyc FuncSignatures (unlike mypy types) want to have a name
# present even when the argument is position only, since it is
# the sole way that FuncDecl arguments are tracked. This is
# generally fine except in some cases (like for computing
# init_sig) we need to produce FuncSignatures from a
# deserialized FuncDef that lacks arguments. We won't ever
# need to use those inside of a FuncIR, so we just make up
# some crap.
if hasattr(fdef, 'arguments'):
arg_names = [arg.variable.name for arg in fdef.arguments]
else:
arg_names = [name or '' for name in fdef.arg_names]

args = [RuntimeArg(arg_name, arg_type, arg_kind)
for arg_name, arg_kind, arg_type in zip(fdef.arg_names, fdef.arg_kinds, arg_types)]
for arg_name, arg_kind, arg_type in zip(arg_names, fdef.arg_kinds, arg_types)]

# We force certain dunder methods to return objects to support letting them
# return NotImplemented. It also avoids some pointless boxing and unboxing,
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -2759,7 +2759,7 @@ t = Test()
t.crash = 'test' # E: "Test" has no attribute "crash"

class A:
def __setattr__(self): ... # E: Invalid signature "def (self: __main__.A) -> Any" for "__setattr__"
def __setattr__(self): ... # E: Invalid signature "def (__main__.A) -> Any" for "__setattr__"
a = A()
a.test = 4 # E: "A" has no attribute "test"

Expand Down
19 changes: 18 additions & 1 deletion test-data/unit/check-python38.test
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,23 @@ def f(p1: bytes, p2: float, /) -> None:
reveal_type(p1) # N: Revealed type is "builtins.bytes"
reveal_type(p2) # N: Revealed type is "builtins.float"

[case testPEP570Unannotated]
def f(arg, /): ...
g = lambda arg, /: arg
def h(arg=0, /): ...
i = lambda arg=0, /: arg

f(1)
g(1)
h()
h(1)
i()
i(1)
f(arg=0) # E: Unexpected keyword argument "arg" for "f"
g(arg=0) # E: Unexpected keyword argument "arg"
h(arg=0) # E: Unexpected keyword argument "arg" for "h"
i(arg=0) # E: Unexpected keyword argument "arg"

[case testWalrus]
# flags: --strict-optional
from typing import NamedTuple, Optional, List
Expand All @@ -206,7 +223,7 @@ while b := "x":
l = [y2 := 1, y2 + 2, y2 + 3]
reveal_type(y2) # N: Revealed type is "builtins.int"
reveal_type(l) # N: Revealed type is "builtins.list[builtins.int*]"

filtered_data = [y3 for x in l if (y3 := a) is not None]
reveal_type(filtered_data) # N: Revealed type is "builtins.list[builtins.int*]"
reveal_type(y3) # N: Revealed type is "builtins.int"
Expand Down

0 comments on commit d180456

Please sign in to comment.