Better support for converter in attrs plugin. (python#4607)

Fixes python#4583 (and more).

My first pass at this only supported converters that were reachable
from the root.  With this code the converter can be a local function
too.

Also now when we can't detect the type of the converter the init type
becomes Any.

mypy/plugin.py

@@ -2,12 +2,12 @@
 
 from abc import abstractmethod
 from functools import partial
-from typing import Callable, List, Tuple, Optional, NamedTuple, TypeVar
+from typing import Callable, List, Tuple, Optional, NamedTuple, TypeVar, Dict
 
 import mypy.plugins.attrs
 from mypy.nodes import (
     Expression, StrExpr, IntExpr, UnaryExpr, Context, DictExpr, ClassDef,
-    TypeInfo, SymbolTableNode
+    TypeInfo, SymbolTableNode, MypyFile
 )
 from mypy.tvar_scope import TypeVarScope
 from mypy.types import (
@@ -61,6 +61,7 @@ def named_generic_type(self, name: str, args: List[Type]) -> Instance:
 class SemanticAnalyzerPluginInterface:
     """Interface for accessing semantic analyzer functionality in plugins."""
 
+    modules = None  # type: Dict[str, MypyFile]
     options = None  # type: Options
     msg = None  # type: MessageBuilder
 
@@ -93,6 +94,15 @@ def class_type(self, info: TypeInfo) -> Type:
     def lookup_fully_qualified(self, name: str) -> SymbolTableNode:
         raise NotImplementedError
 
+    @abstractmethod
+    def lookup_fully_qualified_or_none(self, name: str) -> Optional[SymbolTableNode]:
+        raise NotImplementedError
+
+    @abstractmethod
+    def lookup_qualified(self, name: str, ctx: Context,
+                         suppress_errors: bool = False) -> Optional[SymbolTableNode]:
+        raise NotImplementedError
+
 
 # A context for a function hook that infers the return type of a function with
 # a special signature.

mypy/plugins/attrs.py

@@ -4,6 +4,7 @@
 
 import mypy.plugin  # To avoid circular imports.
 from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError
+from mypy.fixup import lookup_qualified_stnode
 from mypy.nodes import (
     Context, Argument, Var, ARG_OPT, ARG_POS, TypeInfo, AssignmentStmt,
     TupleExpr, ListExpr, NameExpr, CallExpr, RefExpr, FuncBase,
@@ -54,14 +55,21 @@ def argument(self, ctx: 'mypy.plugin.ClassDefContext') -> Argument:
         if self.converter_name:
             # When a converter is set the init_type is overriden by the first argument
             # of the converter method.
-            converter = ctx.api.lookup_fully_qualified(self.converter_name)
+            converter = lookup_qualified_stnode(ctx.api.modules, self.converter_name, True)
+            if not converter:
+                # The converter may be a local variable. Check there too.
+                converter = ctx.api.lookup_qualified(self.converter_name, self.info, True)
+
             if (converter
                     and converter.type
                     and isinstance(converter.type, CallableType)
                     and converter.type.arg_types):
                 init_type = converter.type.arg_types[0]
             else:
-                init_type = None
+                init_type = AnyType(TypeOfAny.from_error)
+        elif self.converter_name == '':
+            # This means we had a converter but it's not of a type we can infer.
+            init_type = AnyType(TypeOfAny.from_error)
 
         if init_type is None:
             if ctx.api.options.disallow_untyped_defs:
@@ -317,14 +325,16 @@ def _attribute_from_attrib_maker(ctx: 'mypy.plugin.ClassDefContext',
 def _get_converter_name(converter: Optional[Expression]) -> Optional[str]:
     """Return the full name of the converter if it exists and is a simple function."""
     # TODO: Support complex converters, e.g. lambdas, calls, etc.
-    if (converter
-            and isinstance(converter, RefExpr)
-            and converter.node
-            and isinstance(converter.node, FuncBase)
-            and converter.node.type
-            and isinstance(converter.node.type, CallableType)
-            and converter.node.type.arg_types):
-        return converter.node.fullname()
+    if converter:
+        if (isinstance(converter, RefExpr)
+                and converter.node
+                and isinstance(converter.node, FuncBase)
+                and converter.node.type
+                and isinstance(converter.node.type, CallableType)
+                and converter.node.type.arg_types):
+            return converter.node.fullname()
+        # Signal that we have an unsupported converter.
+        return ''
     return None
 
 

test-data/unit/check-attr.test

@@ -532,6 +532,21 @@ class C:
 
 [builtins fixtures/list.pyi]
 
+[case testAttrsUsingUnsupportedConverter]
+import attr
+class Thing:
+    def do_it(self, int) -> str:
+        ...
+thing = Thing()
+def factory(default: int):
+   ...
+@attr.s
+class C:
+    x: str = attr.ib(converter=thing.do_it)
+    y: str = attr.ib(converter=lambda x: x)
+    z: str = attr.ib(converter=factory(8))
+reveal_type(C)  # E: Revealed type is 'def (x: Any, y: Any, z: Any) -> __main__.C'
+[builtins fixtures/list.pyi]
 
 [case testAttrsUsingConverterAndSubclass]
 import attr

test-data/unit/check-incremental.test

@@ -3745,6 +3745,183 @@ class C(A, B):
 [out1]
 [out2]
 
+[case testAttrsIncrementalConverterInSubmodule]
+from a.a import A
+reveal_type(A)
+[file a/__init__.py]
+[file a/a.py]
+from typing import Optional
+def converter(s:Optional[int]) -> int:
+    ...
+
+import attr
+@attr.s
+class A:
+    x: int = attr.ib(converter=converter)
+
+[builtins fixtures/list.pyi]
+[out1]
+main:2: error: Revealed type is 'def (x: Union[builtins.int, builtins.None]) -> a.a.A'
+[out2]
+main:2: error: Revealed type is 'def (x: Union[builtins.int, builtins.None]) -> a.a.A'
+
+[case testAttrsIncrementalConverterManyStyles]
+from base import Base
+reveal_type(Base)
+from subclass import A, B
+reveal_type(A)
+reveal_type(B)
+from submodule.base import SubBase
+reveal_type(SubBase)
+from submodule.subclass import AA, BB
+reveal_type(AA)
+reveal_type(BB)
+from submodule.subsubclass import SubAA, SubBB
+reveal_type(SubAA)
+reveal_type(SubBB)
+
+[file foo.py]
+from typing import Optional
+def maybe_int(x: Optional[int]) -> int:
+   ...
+[file bar.py]
+from typing import Optional
+def maybe_bool(x: Optional[bool]) -> bool:
+   ...
+[file base.py]
+from typing import Optional
+import attr
+import bar
+from foo import maybe_int
+def maybe_str(x: Optional[str]) -> str:
+   ...
+@attr.s
+class Base:
+    x: int = attr.ib(converter=maybe_int)
+    y: str = attr.ib(converter=maybe_str)
+    z: bool = attr.ib(converter=bar.maybe_bool)
+[file subclass.py]
+from typing import Optional
+import attr
+from base import Base
+@attr.s
+class A(Base): pass
+
+import bar
+from foo import maybe_int
+def maybe_str(x: Optional[str]) -> str:
+   ...
+@attr.s
+class B(Base):
+    xx: int = attr.ib(converter=maybe_int)
+    yy: str = attr.ib(converter=maybe_str)
+    zz: bool = attr.ib(converter=bar.maybe_bool)
+
+[file submodule/__init__.py]
+[file submodule/base.py]
+from typing import Optional
+import attr
+import bar
+from foo import maybe_int
+def maybe_str(x: Optional[str]) -> str:
+   ...
+@attr.s
+class SubBase:
+    x: int = attr.ib(converter=maybe_int)
+    y: str = attr.ib(converter=maybe_str)
+    z: bool = attr.ib(converter=bar.maybe_bool)
+
+[file submodule/subclass.py]
+from typing import Optional
+import attr
+from base import Base
+@attr.s
+class AA(Base): pass
+
+import bar
+from foo import maybe_int
+def maybe_str(x: Optional[str]) -> str:
+   ...
+@attr.s
+class BB(Base):
+    xx: int = attr.ib(converter=maybe_int)
+    yy: str = attr.ib(converter=maybe_str)
+    zz: bool = attr.ib(converter=bar.maybe_bool)
+
+[file submodule/subsubclass.py]
+from typing import Optional
+import attr
+from .base import SubBase
+@attr.s
+class SubAA(SubBase): pass
+
+import bar
+from foo import maybe_int
+def maybe_str(x: Optional[str]) -> str:
+   ...
+@attr.s
+class SubBB(SubBase):
+    xx: int = attr.ib(converter=maybe_int)
+    yy: str = attr.ib(converter=maybe_str)
+    zz: bool = attr.ib(converter=bar.maybe_bool)
+[builtins fixtures/list.pyi]
+[out1]
+main:2: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> base.Base'
+main:4: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> subclass.A'
+main:5: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> subclass.B'
+main:7: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.base.SubBase'
+main:9: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.subclass.AA'
+main:10: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> submodule.subclass.BB'
+main:12: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.subsubclass.SubAA'
+main:13: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> submodule.subsubclass.SubBB'
+[out2]
+main:2: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> base.Base'
+main:4: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> subclass.A'
+main:5: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> subclass.B'
+main:7: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.base.SubBase'
+main:9: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.subclass.AA'
+main:10: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> submodule.subclass.BB'
+main:12: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None]) -> submodule.subsubclass.SubAA'
+main:13: error: Revealed type is 'def (x: Union[builtins.int, builtins.None], y: Union[builtins.str, builtins.None], z: Union[builtins.bool, builtins.None], xx: Union[builtins.int, builtins.None], yy: Union[builtins.str, builtins.None], zz: Union[builtins.bool, builtins.None]) -> submodule.subsubclass.SubBB'
+
+[case testAttrsIncrementalConverterInFunction]
+import attr
+def foo() -> None:
+    def foo(x: str) -> int:
+        ...
+    @attr.s
+    class A:
+        x: int = attr.ib(converter=foo)
+    reveal_type(A)
+[builtins fixtures/list.pyi]
+[out1]
+main:8: error: Revealed type is 'def (x: str?) -> __main__.A@5'
+[out2]
+main:8: error: Revealed type is 'def (x: str?) -> __main__.A@5'
+
+[case testAttrsIncrementalConverterInSubmoduleForwardRef]
+from a.a import A
+reveal_type(A)
+[file a/__init__.py]
+[file a/a.py]
+from typing import List
+def converter(s:F) -> int:
+    ...
+
+import attr
+@attr.s
+class A:
+    x: int = attr.ib(converter=converter)
+
+F = List[int]
+
+[builtins fixtures/list.pyi]
+[out1]
+main:2: error: Revealed type is 'def (x: builtins.list[builtins.int]) -> a.a.A'
+[out2]
+main:2: error: Revealed type is 'def (x: builtins.list[builtins.int]) -> a.a.A'
+
+
 [case testAttrsIncrementalThreeRuns]
 from a import A
 A(5)