From 278a09537ac51a131b1298e4d1ba015bac3fc8c5 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 25 Nov 2022 17:27:37 +0000 Subject: [PATCH] Fix crash on overriding with frozen attrs (#14186) Fixes #6715 Fix is straightforward, currently we assume that if we have a variable in MRO, and its name appears in current class, it is from this class, which in fact may not be the case when a variable is overridden with a property or method. I also add a test case for a crash that was previously reported in the same issue but is already (accidentally?) fixed. --- mypy/plugins/attrs.py | 6 ++++- test-data/unit/check-attr.test | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/mypy/plugins/attrs.py b/mypy/plugins/attrs.py index 17f1794d8c75d..ce0f45967152c 100644 --- a/mypy/plugins/attrs.py +++ b/mypy/plugins/attrs.py @@ -736,7 +736,11 @@ def _make_frozen(ctx: mypy.plugin.ClassDefContext, attributes: list[Attribute]) if attribute.name in ctx.cls.info.names: # This variable belongs to this class so we can modify it. node = ctx.cls.info.names[attribute.name].node - assert isinstance(node, Var) + if not isinstance(node, Var): + # The superclass attribute was overridden with a non-variable. + # No need to do anything here, override will be verified during + # type checking. + continue node.is_property = True else: # This variable belongs to a super class so create new Var so we diff --git a/test-data/unit/check-attr.test b/test-data/unit/check-attr.test index fe123acfa0016..4d27d5f39d1e7 100644 --- a/test-data/unit/check-attr.test +++ b/test-data/unit/check-attr.test @@ -1788,3 +1788,49 @@ class C: c = C(x=[C.D()]) reveal_type(c.x) # N: Revealed type is "builtins.list[__main__.C.D]" [builtins fixtures/list.pyi] + +[case testRedefinitionInFrozenClassNoCrash] +import attr + +@attr.s +class MyData: + is_foo: bool = attr.ib() + + @staticmethod # E: Name "is_foo" already defined on line 5 + def is_foo(string: str) -> bool: ... +[builtins fixtures/classmethod.pyi] + +[case testOverrideWithPropertyInFrozenClassNoCrash] +from attrs import frozen + +@frozen(kw_only=True) +class Base: + name: str + +@frozen(kw_only=True) +class Sub(Base): + first_name: str + last_name: str + + @property + def name(self) -> str: ... +[builtins fixtures/property.pyi] + +[case testOverrideWithPropertyInFrozenClassChecked] +from attrs import frozen + +@frozen(kw_only=True) +class Base: + name: str + +@frozen(kw_only=True) +class Sub(Base): + first_name: str + last_name: str + + @property + def name(self) -> int: ... # E: Signature of "name" incompatible with supertype "Base" + +# This matches runtime semantics +reveal_type(Sub) # N: Revealed type is "def (*, name: builtins.str, first_name: builtins.str, last_name: builtins.str) -> __main__.Sub" +[builtins fixtures/property.pyi]