Attrs/tweak magic attribute (#13522)

This is an attempt to fix the attrs magic attribute handling.

The attrs magic attribute functionality was added by me a few months
ago, but inexpertly. Since we're starting to see usage in the wild, I'm
attempting to fix it.

Context: every attrs class has a magic attribute, a final classvar
available under `Class.__attrs_attrs__`. This is an instance of an
ad-hoc tuple subclass, and it contains an instance of
`attrs.Attribute[T]` for each class attribute. The reason it's a tuple
subclass is that is has properties for each attribute, so instead of
`Class.__attrs_attrs__[0]` we can use `Class.__attrs_attrs__.a`. Users
aren't really supposed to use this attribute directly, but through
`attrs.fields()` (a subject of a future PR), but having Mypy know about
it is useful for f.e. protocols (and the newest release of attrs does
contain a protocol using the magic attribute).

When I implemented the magic attribute here initially, I set it as
`no_serialize=True`. This is incorrect for some use cases.

My first issue: is my approach OK?

Like I mentioned, the type of each magic attribute is an instance of an
ad-hoc tuple subclass. I need to stash this subclass somewhere so cached
runs work properly. Right now I'm trying to serialize it under
`Class.__Class_AttrsAttributes__`; maybe there's a better way? I needed
to incorporate the fully qualified class name in the attribute name so
Mypy doesn't complain about incompatible fields when subclassing.

My second issue is some incremental, fine-grained tests are failing now
and I cannot figure out how to fix them, so it'd be great to get some
advice here too.

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

mypy/plugins/attrs.py

@@ -75,7 +75,7 @@
 
 SELF_TVAR_NAME: Final = "_AT"
 MAGIC_ATTR_NAME: Final = "__attrs_attrs__"
-MAGIC_ATTR_CLS_NAME: Final = "_AttrsAttributes"  # The namedtuple subclass name.
+MAGIC_ATTR_CLS_NAME_TEMPLATE: Final = "__{}_AttrsAttributes__"  # The tuple subclass pattern.
 
 
 class Converter:
@@ -257,7 +257,7 @@ 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.
+    to detect attrs classes in base classes.
     """
     # The value is ignored, only the existence matters.
     ctx.cls.info.metadata["attrs_tag"] = {}
@@ -792,25 +792,30 @@ def _add_attrs_magic_attribute(
         "builtins.tuple", [ctx.api.named_type_or_none("attr.Attribute", [any_type]) or any_type]
     )
 
-    ti = ctx.api.basic_new_typeinfo(MAGIC_ATTR_CLS_NAME, fallback_type, 0)
-    ti.is_named_tuple = True
+    attr_name = MAGIC_ATTR_CLS_NAME_TEMPLATE.format(ctx.cls.fullname.replace(".", "_"))
+    ti = ctx.api.basic_new_typeinfo(attr_name, fallback_type, 0)
     for (name, _), attr_type in zip(attrs, attributes_types):
         var = Var(name, attr_type)
+        var._fullname = name
         var.is_property = True
         proper_type = get_proper_type(attr_type)
         if isinstance(proper_type, Instance):
             var.info = proper_type.type
         ti.names[name] = SymbolTableNode(MDEF, var, plugin_generated=True)
     attributes_type = Instance(ti, [])
 
-    # TODO: refactor using `add_attribute_to_class`
-    var = Var(name=MAGIC_ATTR_NAME, type=TupleType(attributes_types, fallback=attributes_type))
-    var.info = ctx.cls.info
-    var.is_classvar = True
-    var._fullname = f"{ctx.cls.fullname}.{MAGIC_ATTR_CLS_NAME}"
-    var.allow_incompatible_override = True
-    ctx.cls.info.names[MAGIC_ATTR_NAME] = SymbolTableNode(
-        kind=MDEF, node=var, plugin_generated=True, no_serialize=True
+    # We need to stash the type of the magic attribute so it can be
+    # loaded on cached runs.
+    ctx.cls.info.names[attr_name] = SymbolTableNode(MDEF, ti, plugin_generated=True)
+
+    add_attribute_to_class(
+        ctx.api,
+        ctx.cls,
+        MAGIC_ATTR_NAME,
+        TupleType(attributes_types, fallback=attributes_type),
+        fullname=f"{ctx.cls.fullname}.{attr_name}",
+        override_allow_incompatible=True,
+        is_classvar=True,
     )
 
 

mypy/plugins/common.py

@@ -217,6 +217,8 @@ def add_attribute_to_class(
     final: bool = False,
     no_serialize: bool = False,
     override_allow_incompatible: bool = False,
+    fullname: str | None = None,
+    is_classvar: bool = False,
 ) -> None:
     """
     Adds a new attribute to a class definition.
@@ -234,11 +236,17 @@ def add_attribute_to_class(
     node = Var(name, typ)
     node.info = info
     node.is_final = final
+    node.is_classvar = is_classvar
     if name in ALLOW_INCOMPATIBLE_OVERRIDE:
         node.allow_incompatible_override = True
     else:
         node.allow_incompatible_override = override_allow_incompatible
-    node._fullname = info.fullname + "." + name
+
+    if fullname:
+        node._fullname = fullname
+    else:
+        node._fullname = info.fullname + "." + name
+
     info.names[name] = SymbolTableNode(
         MDEF, node, plugin_generated=True, no_serialize=no_serialize
     )

test-data/unit/check-attr.test

@@ -1428,51 +1428,76 @@ class B(A):
 reveal_type(B)  # N: Revealed type is "def (foo: builtins.int) -> __main__.B"
 [builtins fixtures/bool.pyi]
 
-[case testAttrsClassHasAttributeWithAttributes]
+[case testAttrsClassHasMagicAttribute]
 import attr
 
 @attr.s
 class A:
     b: int = attr.ib()
     c: str = attr.ib()
 
-reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[builtins.int], attr.Attribute[builtins.str], fallback=__main__.A._AttrsAttributes]"
+reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[builtins.int], attr.Attribute[builtins.str], fallback=__main__.A.____main___A_AttrsAttributes__]"
 reveal_type(A.__attrs_attrs__[0])  # N: Revealed type is "attr.Attribute[builtins.int]"
 reveal_type(A.__attrs_attrs__.b)  # N: Revealed type is "attr.Attribute[builtins.int]"
-A.__attrs_attrs__.x  # E: "_AttrsAttributes" has no attribute "x"
+A.__attrs_attrs__.x  # E: "____main___A_AttrsAttributes__" has no attribute "x"
 
 [builtins fixtures/attr.pyi]
 
-[case testAttrsBareClassHasAttributeWithAttributes]
+[case testAttrsBareClassHasMagicAttribute]
 import attr
 
 @attr.s
 class A:
     b = attr.ib()
     c = attr.ib()
 
-reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[Any], attr.Attribute[Any], fallback=__main__.A._AttrsAttributes]"
+reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[Any], attr.Attribute[Any], fallback=__main__.A.____main___A_AttrsAttributes__]"
 reveal_type(A.__attrs_attrs__[0])  # N: Revealed type is "attr.Attribute[Any]"
 reveal_type(A.__attrs_attrs__.b)  # N: Revealed type is "attr.Attribute[Any]"
-A.__attrs_attrs__.x  # E: "_AttrsAttributes" has no attribute "x"
+A.__attrs_attrs__.x  # E: "____main___A_AttrsAttributes__" has no attribute "x"
 
 [builtins fixtures/attr.pyi]
 
-[case testAttrsNGClassHasAttributeWithAttributes]
+[case testAttrsNGClassHasMagicAttribute]
 import attr
 
 @attr.define
 class A:
     b: int
     c: str
 
-reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[builtins.int], attr.Attribute[builtins.str], fallback=__main__.A._AttrsAttributes]"
+reveal_type(A.__attrs_attrs__)  # N: Revealed type is "Tuple[attr.Attribute[builtins.int], attr.Attribute[builtins.str], fallback=__main__.A.____main___A_AttrsAttributes__]"
 reveal_type(A.__attrs_attrs__[0])  # N: Revealed type is "attr.Attribute[builtins.int]"
 reveal_type(A.__attrs_attrs__.b)  # N: Revealed type is "attr.Attribute[builtins.int]"
-A.__attrs_attrs__.x  # E: "_AttrsAttributes" has no attribute "x"
+A.__attrs_attrs__.x  # E: "____main___A_AttrsAttributes__" has no attribute "x"
 
 [builtins fixtures/attr.pyi]
 
+[case testAttrsMagicAttributeProtocol]
+import attr
+from typing import Any, Protocol, Type, ClassVar
+
+class AttrsInstance(Protocol):
+    __attrs_attrs__: ClassVar[Any]
+
+@attr.define
+class A:
+    b: int
+    c: str
+
+def takes_attrs_cls(cls: Type[AttrsInstance]) -> None:
+    pass
+
+def takes_attrs_instance(inst: AttrsInstance) -> None:
+    pass
+
+takes_attrs_cls(A)
+takes_attrs_instance(A(1, ""))
+
+takes_attrs_cls(A(1, ""))  # E: Argument 1 to "takes_attrs_cls" has incompatible type "A"; expected "Type[AttrsInstance]"
+takes_attrs_instance(A)  # E: Argument 1 to "takes_attrs_instance" has incompatible type "Type[A]"; expected "AttrsInstance" # N: ClassVar protocol member AttrsInstance.__attrs_attrs__ can never be matched by a class object
+[builtins fixtures/attr.pyi]
+
 [case testAttrsClassWithSlots]
 import attr
 

test-data/unit/fine-grained-attr.test

@@ -21,3 +21,28 @@ class A:
 [out]
 ==
 main:5: error: Incompatible return value type (got "Attribute[float]", expected "Attribute[int]")
+
+[case magicAttributeConsistency]
+import m
+
+[file c.py]
+from attr import define
+
+@define
+class A:
+    a: float
+    b: int
+[builtins fixtures/attr.pyi]
+
+[file m.py]
+from c import A
+
+A.__attrs_attrs__.a
+
+[file m.py.2]
+from c import A
+
+A.__attrs_attrs__.b
+
+[out]
+==

test-data/unit/fine-grained.test

@@ -24,7 +24,7 @@
 -- as changed in the initial run with the cache while modules that depended on them
 -- should be.
 --
--- Modules that are require a full-module reprocessing by update can be checked with
+-- Modules that require a full-module reprocessing by update can be checked with
 -- [rechecked ...]. This should include any files detected as having changed as well
 -- as any files that contain targets that need to be reprocessed but which haven't
 -- been loaded yet. If there is no [rechecked...] directive, it inherits the value of