From 7989a2c1924bec2f2cd59f34848335364756d27d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 19 Nov 2021 15:31:31 +0300 Subject: [PATCH 1/5] Enums with annotations and no values are fine to be subclasses --- mypy/semanal.py | 27 ++++++++++++++++++++------- test-data/unit/check-enum.test | 20 +++++++++++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 9a5076beb6f4..9c5b5832db74 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1552,17 +1552,12 @@ def configure_base_classes(self, elif isinstance(base, Instance): if base.type.is_newtype: self.fail('Cannot subclass "NewType"', defn) - if ( - base.type.is_enum - and base.type.fullname not in ENUM_BASES - and base.type.names - and any(not isinstance(n.node, (FuncBase, Decorator)) - for n in base.type.names.values()) - ): + if self.enum_has_final_values(base): # This means that are trying to subclass a non-default # Enum class, with defined members. This is not possible. # In runtime, it will raise. We need to mark this type as final. # However, methods can be defined on a type: only values can't. + # We also don't count values with annotations only. base.type.is_final = True base_types.append(base) elif isinstance(base, AnyType): @@ -1601,6 +1596,24 @@ def configure_base_classes(self, return self.calculate_class_mro(defn, self.object_type) + def enum_has_final_values(self, base: Instance) -> bool: + if ( + base.type.is_enum + and base.type.fullname not in ENUM_BASES + and base.type.names + and base.type.defn + ): + # TODO: if `Var` ever has `.has_explicit_value` prop, remove `ast` traversal. + # Check that enum members are really initialized, ignore methods. + # We need at least one assigned member to call this Enum `final`. + for node in base.type.defn.defs.body: + if isinstance(node, (FuncBase, Decorator)): + continue # A method + if isinstance(node, AssignmentStmt) and isinstance(node.rvalue, TempNode): + continue # Corner case: assignments like `x: int` are fine. + return True + return False + def configure_tuple_base_class(self, defn: ClassDef, base: TupleType, diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index dd94bb6f82ef..fcdbba041cbb 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1678,7 +1678,6 @@ class A(Enum): class B(A): pass # E: Cannot inherit from final class "A" [builtins fixtures/bool.pyi] - [case testEnumFinalSpecialProps] # https://github.com/python/mypy/issues/11699 from enum import Enum, IntEnum @@ -1702,3 +1701,22 @@ class EI(IntEnum): E._order_ = 'a' # E: Cannot assign to final attribute "_order_" EI.value = 2 # E: Cannot assign to final attribute "value" [builtins fixtures/bool.pyi] + +[case testEnumNotFinalWithMethodsAndUninitializedValues] +# https://github.com/python/mypy/issues/11578 +from enum import Enum +from typing import Final + +class A(Enum): + x: int + def method(self) -> int: pass + +class B(A): + x = 1 # E: Cannot override writable attribute "x" with a final one + +class A1(Enum): + x: int = 1 + +class B1(A1): # E: Cannot inherit from final class "A1" + pass +[builtins fixtures/bool.pyi] From 901ca52cdec4ac26d96b0fa491ec4563e3eace3d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 19 Nov 2021 19:17:29 +0300 Subject: [PATCH 2/5] Address review --- mypy/semanal.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 9c5b5832db74..b37deb9597a7 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1609,7 +1609,9 @@ def enum_has_final_values(self, base: Instance) -> bool: for node in base.type.defn.defs.body: if isinstance(node, (FuncBase, Decorator)): continue # A method - if isinstance(node, AssignmentStmt) and isinstance(node.rvalue, TempNode): + if (isinstance(node, AssignmentStmt) + and isinstance(node.rvalue, TempNode) + and node.rvalue.no_rhs): continue # Corner case: assignments like `x: int` are fine. return True return False From e4d4112c104aeeb1e4fa6fea36b027757be0ca93 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 20 Nov 2021 21:40:29 +0300 Subject: [PATCH 3/5] Adds `.pyi` test for uninitialized values --- mypy/semanal.py | 7 +++++-- test-data/unit/check-enum.test | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index b37deb9597a7..d5dd06f4fde4 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1609,10 +1609,13 @@ def enum_has_final_values(self, base: Instance) -> bool: for node in base.type.defn.defs.body: if isinstance(node, (FuncBase, Decorator)): continue # A method - if (isinstance(node, AssignmentStmt) + if (not self.is_stub_file + and isinstance(node, AssignmentStmt) and isinstance(node.rvalue, TempNode) and node.rvalue.no_rhs): - continue # Corner case: assignments like `x: int` are fine. + # Corner case: assignments like `x: int` are fine. + # But, only in `.py` files. Stub files can still be unintialized. + continue return True return False diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index fcdbba041cbb..312b2a22d725 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1710,13 +1710,34 @@ from typing import Final class A(Enum): x: int def method(self) -> int: pass - class B(A): x = 1 # E: Cannot override writable attribute "x" with a final one class A1(Enum): x: int = 1 - class B1(A1): # E: Cannot inherit from final class "A1" pass + +class A2(Enum): + x = 2 +class B2(A2): # E: Cannot inherit from final class "A2" + pass +[builtins fixtures/bool.pyi] + +[case testEnumNotFinalWithMethodsAndUninitializedValuesStub] +import lib +reveal_type(lib.A) +reveal_type(lib.B) + +[file lib.pyi] +from enum import Enum +class A(Enum): + x: int +class B(A): + x = 1 +[out] +tmp/lib.pyi:4: error: Cannot inherit from final class "A" +tmp/lib.pyi:5: error: Cannot override writable attribute "x" with a final one +main:2: note: Revealed type is "def (value: builtins.object) -> lib.A*" +main:3: note: Revealed type is "def (value: builtins.object) -> lib.B*" [builtins fixtures/bool.pyi] From 04afff578e5ea12b1395ca1df4240bd1921c637c Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 19 Dec 2021 00:36:00 +0300 Subject: [PATCH 4/5] Now using `Var.has_explicit_value` boolean flag instead of AST traversal --- mypy/nodes.py | 4 +++ mypy/semanal.py | 64 +++++++++++++++++++++++----------- test-data/unit/check-enum.test | 14 ++++---- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/mypy/nodes.py b/mypy/nodes.py index 156d756030ae..cbec8337ba10 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -870,6 +870,7 @@ class Var(SymbolNode): 'is_suppressed_import', 'explicit_self_type', 'from_module_getattr', + 'has_explicit_value', ) def __init__(self, name: str, type: 'Optional[mypy.types.Type]' = None) -> None: @@ -914,6 +915,9 @@ def __init__(self, name: str, type: 'Optional[mypy.types.Type]' = None) -> None: self.explicit_self_type = False # If True, this is an implicit Var created due to module-level __getattr__. self.from_module_getattr = False + # Var can be created with an explicit value `a = 1` or without one `a: int`, + # we need a way to tell which one is which. + self.has_explicit_value = False @property def name(self) -> str: diff --git a/mypy/semanal.py b/mypy/semanal.py index d5dd06f4fde4..f29965f6ff8f 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1603,20 +1603,16 @@ def enum_has_final_values(self, base: Instance) -> bool: and base.type.names and base.type.defn ): - # TODO: if `Var` ever has `.has_explicit_value` prop, remove `ast` traversal. - # Check that enum members are really initialized, ignore methods. - # We need at least one assigned member to call this Enum `final`. - for node in base.type.defn.defs.body: - if isinstance(node, (FuncBase, Decorator)): + for sym in base.type.names.values(): + if isinstance(sym.node, (FuncBase, Decorator)): continue # A method - if (not self.is_stub_file - and isinstance(node, AssignmentStmt) - and isinstance(node.rvalue, TempNode) - and node.rvalue.no_rhs): - # Corner case: assignments like `x: int` are fine. - # But, only in `.py` files. Stub files can still be unintialized. - continue - return True + if not isinstance(sym.node, Var): + return True # Can be a class + if self.is_stub_file or sym.node.has_explicit_value: + # Corner case: assignments like `x: int` are fine in `.py` files. + # But, not is `.pyi` files, because we don't know + # if there's aactually a value or not. + return True return False def configure_tuple_base_class(self, @@ -2058,7 +2054,7 @@ def visit_import_all(self, i: ImportAll) -> None: def visit_assignment_expr(self, s: AssignmentExpr) -> None: s.value.accept(self) - self.analyze_lvalue(s.target, escape_comprehensions=True) + self.analyze_lvalue(s.target, escape_comprehensions=True, has_explicit_value=True) def visit_assignment_stmt(self, s: AssignmentStmt) -> None: self.statement = s @@ -2351,10 +2347,20 @@ def analyze_lvalues(self, s: AssignmentStmt) -> None: assert isinstance(s.unanalyzed_type, UnboundType) if not s.unanalyzed_type.args: explicit = False + + if s.rvalue: + if isinstance(s.rvalue, TempNode): + has_explicit_value = not s.rvalue.no_rhs + else: + has_explicit_value = True + else: + has_explicit_value = False + for lval in s.lvalues: self.analyze_lvalue(lval, explicit_type=explicit, - is_final=s.is_final_def) + is_final=s.is_final_def, + has_explicit_value=has_explicit_value) def apply_dynamic_class_hook(self, s: AssignmentStmt) -> None: if not isinstance(s.rvalue, CallExpr): @@ -2794,7 +2800,8 @@ def analyze_lvalue(self, nested: bool = False, explicit_type: bool = False, is_final: bool = False, - escape_comprehensions: bool = False) -> None: + escape_comprehensions: bool = False, + has_explicit_value: bool = False) -> None: """Analyze an lvalue or assignment target. Args: @@ -2808,7 +2815,11 @@ def analyze_lvalue(self, if escape_comprehensions: assert isinstance(lval, NameExpr), "assignment expression target must be NameExpr" if isinstance(lval, NameExpr): - self.analyze_name_lvalue(lval, explicit_type, is_final, escape_comprehensions) + self.analyze_name_lvalue( + lval, explicit_type, is_final, + escape_comprehensions, + has_explicit_value=has_explicit_value, + ) elif isinstance(lval, MemberExpr): self.analyze_member_lvalue(lval, explicit_type, is_final) if explicit_type and not self.is_self_member_ref(lval): @@ -2832,7 +2843,8 @@ def analyze_name_lvalue(self, lvalue: NameExpr, explicit_type: bool, is_final: bool, - escape_comprehensions: bool) -> None: + escape_comprehensions: bool, + has_explicit_value: bool) -> None: """Analyze an lvalue that targets a name expression. Arguments are similar to "analyze_lvalue". @@ -2862,7 +2874,7 @@ def analyze_name_lvalue(self, if (not existing or isinstance(existing.node, PlaceholderNode)) and not outer: # Define new variable. - var = self.make_name_lvalue_var(lvalue, kind, not explicit_type) + var = self.make_name_lvalue_var(lvalue, kind, not explicit_type, has_explicit_value) added = self.add_symbol(name, var, lvalue, escape_comprehensions=escape_comprehensions) # Only bind expression if we successfully added name to symbol table. if added: @@ -2913,7 +2925,9 @@ def is_alias_for_final_name(self, name: str) -> bool: existing = self.globals.get(orig_name) return existing is not None and is_final_node(existing.node) - def make_name_lvalue_var(self, lvalue: NameExpr, kind: int, inferred: bool) -> Var: + def make_name_lvalue_var( + self, lvalue: NameExpr, kind: int, inferred: bool, has_explicit_value: bool, + ) -> Var: """Return a Var node for an lvalue that is a name expression.""" v = Var(lvalue.name) v.set_line(lvalue) @@ -2928,6 +2942,7 @@ def make_name_lvalue_var(self, lvalue: NameExpr, kind: int, inferred: bool) -> V # fullanme should never stay None v._fullname = lvalue.name v.is_ready = False # Type not inferred yet + v.has_explicit_value = has_explicit_value return v def make_name_lvalue_point_to_existing_def( @@ -2971,7 +2986,14 @@ def analyze_tuple_or_list_lvalue(self, lval: TupleExpr, if len(star_exprs) == 1: star_exprs[0].valid = True for i in items: - self.analyze_lvalue(i, nested=True, explicit_type=explicit_type) + self.analyze_lvalue( + lval=i, + nested=True, + explicit_type=explicit_type, + # Lists and tuples always have explicit values defined: + # `a, b, c = value` + has_explicit_value=True, + ) def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: bool) -> None: """Analyze lvalue that is a member expression. diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index 312b2a22d725..c2f395eed242 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1726,18 +1726,16 @@ class B2(A2): # E: Cannot inherit from final class "A2" [case testEnumNotFinalWithMethodsAndUninitializedValuesStub] import lib -reveal_type(lib.A) -reveal_type(lib.B) [file lib.pyi] from enum import Enum class A(Enum): x: int -class B(A): +class B(A): # E: Cannot inherit from final class "A" + x = 1 # E: Cannot override writable attribute "x" with a final one + +class C(Enum): x = 1 -[out] -tmp/lib.pyi:4: error: Cannot inherit from final class "A" -tmp/lib.pyi:5: error: Cannot override writable attribute "x" with a final one -main:2: note: Revealed type is "def (value: builtins.object) -> lib.A*" -main:3: note: Revealed type is "def (value: builtins.object) -> lib.B*" +class D(C): # E: Cannot inherit from final class "C" + x: int # E: Cannot assign to final name "x" [builtins fixtures/bool.pyi] From af90ec4266ffb443424b710c9db7359ab06ccbd9 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 20 Dec 2021 15:43:03 +0300 Subject: [PATCH 5/5] Ensure that cache works --- mypy/nodes.py | 1 + test-data/unit/check-enum.test | 7 +++++++ test-data/unit/check-incremental.test | 23 +++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/mypy/nodes.py b/mypy/nodes.py index cbec8337ba10..78a018f94a78 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -840,6 +840,7 @@ def deserialize(cls, data: JsonDict) -> 'Decorator': 'is_classmethod', 'is_property', 'is_settable_property', 'is_suppressed_import', 'is_classvar', 'is_abstract_var', 'is_final', 'final_unset_in_class', 'final_set_in_init', 'explicit_self_type', 'is_ready', 'from_module_getattr', + 'has_explicit_value', ] diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index c2f395eed242..a393df079730 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1722,6 +1722,13 @@ class A2(Enum): x = 2 class B2(A2): # E: Cannot inherit from final class "A2" pass + +# We leave this `Final` without a value, +# because we need to test annotation only mode: +class A3(Enum): + x: Final[int] # type: ignore +class B3(A3): + x = 1 # E: Cannot override final attribute "x" (previously declared in base class "A3") [builtins fixtures/bool.pyi] [case testEnumNotFinalWithMethodsAndUninitializedValuesStub] diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 1d2262ab303d..ec1dff4977d4 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -5610,3 +5610,26 @@ class C: pass [rechecked] [stale] + + +[case testEnumAreStillFinalAfterCache] +import a +class Ok(a.RegularEnum): + x = 1 +class NotOk(a.FinalEnum): + x = 1 +[file a.py] +from enum import Enum +class RegularEnum(Enum): + x: int +class FinalEnum(Enum): + x = 1 +[builtins fixtures/isinstance.pyi] +[out] +main:3: error: Cannot override writable attribute "x" with a final one +main:4: error: Cannot inherit from final class "FinalEnum" +main:5: error: Cannot override final attribute "x" (previously declared in base class "FinalEnum") +[out2] +main:3: error: Cannot override writable attribute "x" with a final one +main:4: error: Cannot inherit from final class "FinalEnum" +main:5: error: Cannot override final attribute "x" (previously declared in base class "FinalEnum")