Properly track positional-only arguments for unannotated functions (#10802)

msullivan · web-flow · commit d180456b584b · 2021-07-15T10:57:19.000-07:00
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.
diff --git a/mypy/fastparse.py b/mypy/fastparse.py
@@ -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)
@@ -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()
@@ -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
@@ -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:
@@ -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)
diff --git a/mypy/fastparse2.py b/mypy/fastparse2.py
@@ -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
@@ -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)
diff --git a/mypy/messages.py b/mypy/messages.py
@@ -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
diff --git a/mypy/nodes.py b/mypy/nodes.py
@@ -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],
@@ -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))
diff --git a/mypy/typeops.py b/mypy/typeops.py
@@ -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
 
@@ -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,
diff --git a/mypy/types.py b/mypy/types.py
@@ -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:
diff --git a/mypyc/irbuild/mapper.py b/mypyc/irbuild/mapper.py
@@ -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,
diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test
@@ -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"
 
diff --git a/test-data/unit/check-python38.test b/test-data/unit/check-python38.test
@@ -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
@@ -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"
