Fix forward references and generic inheritance in attrs classes (#12772)

Move the attrs plugin to a later pass, so that we won't have placeholders. Fix
various issues related to forward references and generic inheritance, including
some crashes.

This is follow-up to #12762 and related to #12656 and #12633.

Author: JukkaL

mypy/plugins/attrs.py

@@ -7,7 +7,6 @@
 
 import mypy.plugin  # To avoid circular imports.
 from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError
-from mypy.lookup import lookup_fully_qualified
 from mypy.nodes import (
     Context, Argument, Var, ARG_OPT, ARG_POS, TypeInfo, AssignmentStmt,
     TupleExpr, ListExpr, NameExpr, CallExpr, RefExpr, FuncDef,
@@ -61,10 +60,12 @@ class Converter:
     """Holds information about a `converter=` argument"""
 
     def __init__(self,
-                 name: Optional[str] = None,
-                 is_attr_converters_optional: bool = False) -> None:
-        self.name = name
+                 type: Optional[Type] = None,
+                 is_attr_converters_optional: bool = False,
+                 is_invalid_converter: bool = False) -> None:
+        self.type = type
         self.is_attr_converters_optional = is_attr_converters_optional
+        self.is_invalid_converter = is_invalid_converter
 
 
 class Attribute:
@@ -89,29 +90,14 @@ def argument(self, ctx: 'mypy.plugin.ClassDefContext') -> Argument:
 
         init_type = self.init_type or self.info[self.name].type
 
-        if self.converter.name:
+        if self.converter.type and not self.converter.is_invalid_converter:
             # When a converter is set the init_type is overridden by the first argument
             # of the converter method.
-            converter = lookup_fully_qualified(self.converter.name, ctx.api.modules,
-                                               raise_on_missing=False)
-            if not converter:
-                # The converter may be a local variable. Check there too.
-                converter = ctx.api.lookup_qualified(self.converter.name, self.info, True)
-
-            # Get the type of the converter.
-            converter_type: Optional[Type] = None
-            if converter and isinstance(converter.node, TypeInfo):
-                from mypy.checkmember import type_object_type  # To avoid import cycle.
-                converter_type = type_object_type(converter.node, ctx.api.named_type)
-            elif converter and isinstance(converter.node, OverloadedFuncDef):
-                converter_type = converter.node.type
-            elif converter and converter.type:
-                converter_type = converter.type
-
+            converter_type = self.converter.type
             init_type = None
             converter_type = get_proper_type(converter_type)
             if isinstance(converter_type, CallableType) and converter_type.arg_types:
-                init_type = ctx.api.anal_type(converter_type.arg_types[0])
+                init_type = converter_type.arg_types[0]
             elif isinstance(converter_type, Overloaded):
                 types: List[Type] = []
                 for item in converter_type.items:
@@ -124,8 +110,7 @@ def argument(self, ctx: 'mypy.plugin.ClassDefContext') -> Argument:
                     types.append(item.arg_types[0])
                 # Make a union of all the valid types.
                 if types:
-                    args = make_simplified_union(types)
-                    init_type = ctx.api.anal_type(args)
+                    init_type = make_simplified_union(types)
 
             if self.converter.is_attr_converters_optional and init_type:
                 # If the converter was attr.converter.optional(type) then add None to
@@ -135,9 +120,8 @@ def argument(self, ctx: 'mypy.plugin.ClassDefContext') -> Argument:
             if not init_type:
                 ctx.api.fail("Cannot determine __init__ type from converter", self.context)
                 init_type = AnyType(TypeOfAny.from_error)
-        elif self.converter.name == '':
+        elif self.converter.is_invalid_converter:
             # This means we had a converter but it's not of a type we can infer.
-            # Error was shown in _get_converter_name
             init_type = AnyType(TypeOfAny.from_error)
 
         if init_type is None:
@@ -170,8 +154,9 @@ def serialize(self) -> JsonDict:
             'has_default': self.has_default,
             'init': self.init,
             'kw_only': self.kw_only,
-            'converter_name': self.converter.name,
+            'converter_type': self.converter.type.serialize() if self.converter.type else None,
             'converter_is_attr_converters_optional': self.converter.is_attr_converters_optional,
+            'converter_is_invalid_converter': self.converter.is_invalid_converter,
             'context_line': self.context.line,
             'context_column': self.context.column,
             'init_type': self.init_type.serialize() if self.init_type else None,
@@ -185,22 +170,26 @@ def deserialize(cls, info: TypeInfo,
         raw_init_type = data['init_type']
         init_type = deserialize_and_fixup_type(raw_init_type, api) if raw_init_type else None
 
+        converter_type = None
+        if data['converter_type']:
+            converter_type = deserialize_and_fixup_type(data['converter_type'], api)
         return Attribute(data['name'],
             info,
             data['has_default'],
             data['init'],
             data['kw_only'],
-            Converter(data['converter_name'], data['converter_is_attr_converters_optional']),
+            Converter(converter_type, data['converter_is_attr_converters_optional'],
+                      data['converter_is_invalid_converter']),
             Context(line=data['context_line'], column=data['context_column']),
             init_type)
 
     def expand_typevar_from_subtype(self, sub_type: TypeInfo) -> None:
         """Expands type vars in the context of a subtype when an attribute is inherited
         from a generic super type."""
-        if not isinstance(self.init_type, TypeVarType):
-            return
-
-        self.init_type = map_type_from_supertype(self.init_type, sub_type, self.info)
+        if self.init_type:
+            self.init_type = map_type_from_supertype(self.init_type, sub_type, self.info)
+        else:
+            self.init_type = None
 
 
 def _determine_eq_order(ctx: 'mypy.plugin.ClassDefContext') -> bool:
@@ -258,9 +247,19 @@ def _get_decorator_optional_bool_argument(
         return default
 
 
+def attr_tag_callback(ctx: 'mypy.plugin.ClassDefContext') -> None:
+    """Record that we have an attrs class in the main semantic analysis pass.
+
+    The later pass implemented by attr_class_maker_callback will use this
+    to detect attrs lasses in base classes.
+    """
+    # The value is ignored, only the existence matters.
+    ctx.cls.info.metadata['attrs_tag'] = {}
+
+
 def attr_class_maker_callback(ctx: 'mypy.plugin.ClassDefContext',
                               auto_attribs_default: Optional[bool] = False,
-                              frozen_default: bool = False) -> None:
+                              frozen_default: bool = False) -> bool:
     """Add necessary dunder methods to classes decorated with attr.s.
 
     attrs is a package that lets you define classes without writing dull boilerplate code.
@@ -271,6 +270,9 @@ def attr_class_maker_callback(ctx: 'mypy.plugin.ClassDefContext',
     into properties.
 
     See http://www.attrs.org/en/stable/how-does-it-work.html for information on how attrs works.
+
+    If this returns False, some required metadata was not ready yet and we need another
+    pass.
     """
     info = ctx.cls.info
 
@@ -283,30 +285,37 @@ def attr_class_maker_callback(ctx: 'mypy.plugin.ClassDefContext',
     kw_only = _get_decorator_bool_argument(ctx, 'kw_only', False)
     match_args = _get_decorator_bool_argument(ctx, 'match_args', True)
 
+    early_fail = False
     if ctx.api.options.python_version[0] < 3:
         if auto_attribs:
             ctx.api.fail("auto_attribs is not supported in Python 2", ctx.reason)
-            return
+            early_fail = True
         if not info.defn.base_type_exprs:
             # Note: This will not catch subclassing old-style classes.
             ctx.api.fail("attrs only works with new-style classes", info.defn)
-            return
+            early_fail = True
         if kw_only:
             ctx.api.fail(KW_ONLY_PYTHON_2_UNSUPPORTED, ctx.reason)
-            return
+            early_fail = True
+    if early_fail:
+        _add_empty_metadata(info)
+        return True
+
+    for super_info in ctx.cls.info.mro[1:-1]:
+        if 'attrs_tag' in super_info.metadata and 'attrs' not in super_info.metadata:
+            # Super class is not ready yet. Request another pass.
+            return False
 
     attributes = _analyze_class(ctx, auto_attribs, kw_only)
 
     # Check if attribute types are ready.
     for attr in attributes:
         node = info.get(attr.name)
         if node is None:
-            # This name is likely blocked by a star import. We don't need to defer because
-            # defer() is already called by mark_incomplete().
-            return
-        if node.type is None and not ctx.api.final_iteration:
-            ctx.api.defer()
-            return
+            # This name is likely blocked by some semantic analysis error that
+            # should have been reported already.
+            _add_empty_metadata(info)
+            return True
 
     _add_attrs_magic_attribute(ctx, [(attr.name, info[attr.name].type) for attr in attributes])
     if slots:
@@ -330,6 +339,8 @@ def attr_class_maker_callback(ctx: 'mypy.plugin.ClassDefContext',
     if frozen:
         _make_frozen(ctx, attributes)
 
+    return True
+
 
 def _get_frozen(ctx: 'mypy.plugin.ClassDefContext', frozen_default: bool) -> bool:
     """Return whether this class is frozen."""
@@ -423,6 +434,14 @@ def _analyze_class(ctx: 'mypy.plugin.ClassDefContext',
     return attributes
 
 
+def _add_empty_metadata(info: TypeInfo) -> None:
+    """Add empty metadata to mark that we've finished processing this class."""
+    info.metadata['attrs'] = {
+        'attributes': [],
+        'frozen': False,
+    }
+
+
 def _detect_auto_attribs(ctx: 'mypy.plugin.ClassDefContext') -> bool:
     """Return whether auto_attribs should be enabled or disabled.
 
@@ -602,12 +621,13 @@ def _parse_converter(ctx: 'mypy.plugin.ClassDefContext',
             if (isinstance(converter.node, FuncDef)
                     and converter.node.type
                     and isinstance(converter.node.type, FunctionLike)):
-                return Converter(converter.node.fullname)
+                return Converter(converter.node.type)
             elif (isinstance(converter.node, OverloadedFuncDef)
                     and is_valid_overloaded_converter(converter.node)):
-                return Converter(converter.node.fullname)
+                return Converter(converter.node.type)
             elif isinstance(converter.node, TypeInfo):
-                return Converter(converter.node.fullname)
+                from mypy.checkmember import type_object_type  # To avoid import cycle.
+                return Converter(type_object_type(converter.node, ctx.api.named_type))
 
         if (isinstance(converter, CallExpr)
                 and isinstance(converter.callee, RefExpr)
@@ -625,7 +645,7 @@ def _parse_converter(ctx: 'mypy.plugin.ClassDefContext',
             "Unsupported converter, only named functions and types are currently supported",
             converter
         )
-        return Converter('')
+        return Converter(None, is_invalid_converter=True)
     return Converter(None)
 
 

mypy/plugins/default.py

@@ -94,10 +94,34 @@ def get_attribute_hook(self, fullname: str
 
     def get_class_decorator_hook(self, fullname: str
                                  ) -> Optional[Callable[[ClassDefContext], None]]:
+        from mypy.plugins import dataclasses
         from mypy.plugins import attrs
+
+        # These dataclass and attrs hooks run in the main semantic analysis pass
+        # and only tag known dataclasses/attrs classes, so that the second
+        # hooks (in get_class_decorator_hook_2) can detect dataclasses/attrs classes
+        # in the MRO.
+        if fullname in dataclasses.dataclass_makers:
+            return dataclasses.dataclass_tag_callback
+        if (fullname in attrs.attr_class_makers
+                or fullname in attrs.attr_dataclass_makers
+                or fullname in attrs.attr_frozen_makers
+                or fullname in attrs.attr_define_makers):
+            return attrs.attr_tag_callback
+
+        return None
+
+    def get_class_decorator_hook_2(self, fullname: str
+                                   ) -> Optional[Callable[[ClassDefContext], bool]]:
         from mypy.plugins import dataclasses
+        from mypy.plugins import functools
+        from mypy.plugins import attrs
 
-        if fullname in attrs.attr_class_makers:
+        if fullname in dataclasses.dataclass_makers:
+            return dataclasses.dataclass_class_maker_callback
+        elif fullname in functools.functools_total_ordering_makers:
+            return functools.functools_total_ordering_maker_callback
+        elif fullname in attrs.attr_class_makers:
             return attrs.attr_class_maker_callback
         elif fullname in attrs.attr_dataclass_makers:
             return partial(
@@ -115,20 +139,6 @@ def get_class_decorator_hook(self, fullname: str
                 attrs.attr_class_maker_callback,
                 auto_attribs_default=None,
             )
-        elif fullname in dataclasses.dataclass_makers:
-            return dataclasses.dataclass_tag_callback
-
-        return None
-
-    def get_class_decorator_hook_2(self, fullname: str
-                                   ) -> Optional[Callable[[ClassDefContext], bool]]:
-        from mypy.plugins import dataclasses
-        from mypy.plugins import functools
-
-        if fullname in dataclasses.dataclass_makers:
-            return dataclasses.dataclass_class_maker_callback
-        elif fullname in functools.functools_total_ordering_makers:
-            return functools.functools_total_ordering_maker_callback
 
         return None