Prohibit access via class for instance-only attributes (python#20855)

Fixes python#240 (yes, you read that
right, three digits)

I am taking a conservative approach and flag only cases that would
definitely fail at runtime. Note we already have a more strict check for
this for generics, but it is tricky to avoid double errors, so I think
it is fine to have two errors sometimes.

I also update an outdated comment that confused me for a while.

Author: ilevkivskyi

mypy/checkmember.py

@@ -1191,10 +1191,13 @@ def analyze_class_attribute_access(
     if info.slots and name in info.slots:
         mx.fail(message_registry.CLASS_VAR_CONFLICTS_SLOTS.format(name))
 
-    # If a final attribute was declared on `self` in `__init__`, then it
-    # can't be accessed on the class object.
-    if node.implicit and isinstance(node.node, Var) and node.node.is_final:
-        mx.fail(message_registry.CANNOT_ACCESS_FINAL_INSTANCE_ATTR.format(node.node.name))
+    if node.implicit and isinstance(node.node, Var):
+        if node.node.is_final:
+            # If a final attribute was declared on `self` in `__init__`, then it
+            # can't be accessed on the class object.
+            mx.fail(message_registry.CANNOT_ACCESS_FINAL_INSTANCE_ATTR.format(node.node.name))
+        elif not mx.is_lvalue and not defined_in_superclass(info, name):
+            mx.fail(message_registry.CANNOT_ACCESS_INSTANCE_ONLY_ATTR.format(node.node.name))
 
     # An assignment to final attribute on class object is also always an error,
     # independently of types.
@@ -1574,3 +1577,12 @@ def meta_has_operator(item: Type, op_method: str, named_type: Callable[[str], In
     item = instance_fallback(item, named_type)
     meta = item.type.metaclass_type or named_type("builtins.type")
     return meta.type.has_readable_member(op_method)
+
+
+def defined_in_superclass(info: TypeInfo, name: str) -> bool:
+    """Check if a variable has an explicit value at class level in any of superclasses."""
+    for base in info.mro[1:]:
+        if (node := base.names.get(name)) is not None:
+            if not node.implicit and isinstance(node.node, Var) and node.node.has_explicit_value:
+                return True
+    return False

mypy/message_registry.py

@@ -238,6 +238,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
 CANNOT_ACCESS_FINAL_INSTANCE_ATTR: Final = (
     'Cannot access final instance attribute "{}" on class object'
 )
+CANNOT_ACCESS_INSTANCE_ONLY_ATTR: Final = (
+    'Cannot access instance-only attribute "{}" on class object'
+)
 CANNOT_MAKE_DELETABLE_FINAL: Final = ErrorMessage("Deletable attribute cannot be final")
 
 # Disjoint bases

mypy/nodes.py

@@ -1312,7 +1312,8 @@ def __init__(self, name: str, type: mypy.types.Type | None = None) -> None:
         self.is_cls = False
         self.is_ready = True  # If inferred, is the inferred type available?
         self.is_inferred = self.type is None
-        # Is this initialized explicitly to a non-None value in class body?
+        # Is this variable declared in class body? The name is confusing, but it
+        # is a very old attribute, and changing will break some plugins.
         self.is_initialized_in_class = False
         self.is_staticmethod = False
         self.is_classmethod = False

test-data/unit/check-classes.test

@@ -7180,7 +7180,22 @@ b = B(2) # E: Cannot instantiate abstract class "B" with abstract attribute "__i
 B.c # E: "type[B]" has no attribute "c"
 c = C(3)
 c.c
-C.c
+C.c  # E: Cannot access instance-only attribute "c" on class object
+
+[case testAccessInstanceVarOnClass]
+class A:
+    def __init__(self) -> None:
+        self.x = 0
+
+A.x  # E: Cannot access instance-only attribute "x" on class object
+A.x = 1  # We allow this, since it works at runtime, even though it is weird
+
+class B:
+    x = 1
+class C(B):
+    def __init__(self) -> None:
+        self.x: int = 2
+C.x # Again, this works at runtime, so we don't prohibit this
 
 [case testDecoratedConstructors]
 from typing import TypeVar, Callable, Any

test-data/unit/check-dataclasses.test

@@ -839,7 +839,8 @@ class A(Generic[T]):
   @classmethod
   def foo(cls) -> None:
       reveal_type(cls)  # N: Revealed type is "type[__main__.A[T`1]]"
-      cls.x  # E: Access to generic instance variables via class is ambiguous
+      cls.x  # E: Cannot access instance-only attribute "x" on class object \
+             # E: Access to generic instance variables via class is ambiguous
 
   @classmethod
   def other(cls, x: T) -> A[T]: ...

test-data/unit/check-generics.test

@@ -2175,7 +2175,8 @@ class C(Generic[T]):
 
     @classmethod
     def meth(cls) -> None:
-        cls.x  # E: Access to generic instance variables via class is ambiguous
+        cls.x  # E: Cannot access instance-only attribute "x" on class object \
+               # E: Access to generic instance variables via class is ambiguous
 [builtins fixtures/classmethod.pyi]
 
 [case testGenericClassMethodUnboundOnClassNonMatchingIdNonGeneric]

test-data/unit/check-incremental.test

@@ -2938,7 +2938,7 @@ class Namespace:
 
 [file main.py]
 import ns
-user = ns.Namespace.user
+user = ns.Namespace().user
 
 [out1]
 tmp/main.py:2: error: Expression has type "Any"
@@ -5490,7 +5490,7 @@ class A:
 from a import A
 [file b.py.2]
 from a import A
-reveal_type(A.D.x)
+reveal_type(A.D().x)
 [builtins fixtures/isinstance.pyi]
 [out]
 [out2]