From 60e01c126df4e0529fe3806f9c2637a5a45dd138 Mon Sep 17 00:00:00 2001
From: Victor Westerhuis <viccie30@users.noreply.github.com>
Date: Thu, 27 Apr 2023 14:31:46 +0200
Subject: [PATCH] fix: Respect `ClassVar` annotation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

According to [PEP 526](https://peps.python.org/pep-0526/#class-and-instance-variable-annotations)
annotated attributes in a class body are instance variables by default,
unless explicitly overridden by using
[`typing.ClassVar`](https://docs.python.org/3/library/typing.html#typing.ClassVar).

Griffe considers annotated attributes with a default value as class and instance attributes.

PR #150: https://github.com/mkdocstrings/griffe/pull/150
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
---
 src/griffe/agents/visitor.py | 21 +++++++++++++++---
 src/griffe/expressions.py    |  9 ++++++++
 tests/test_visitor.py        | 42 ++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/src/griffe/agents/visitor.py b/src/griffe/agents/visitor.py
index b41b952b..5556a864 100644
--- a/src/griffe/agents/visitor.py
+++ b/src/griffe/agents/visitor.py
@@ -42,13 +42,14 @@
     Parameters,
 )
 from griffe.exceptions import AliasResolutionError, CyclicAliasError, LastNodeError, NameResolutionError
+from griffe.expressions import Expression
 from griffe.extensions import Extensions
 
 if TYPE_CHECKING:
     from pathlib import Path
 
     from griffe.docstrings.parsers import Parser
-    from griffe.expressions import Expression, Name
+    from griffe.expressions import Name
 
 
 builtin_decorators = {
@@ -557,7 +558,19 @@ def handle_attribute(
                 names = get_names(node)
             except KeyError:  # unsupported nodes, like subscript
                 return
-            labels.add("class-attribute")
+
+            if isinstance(annotation, Expression) and annotation.is_classvar:
+                # explicit classvar: class attribute only
+                annotation = annotation[2]
+                labels.add("class-attribute")
+            elif node.value:
+                # attribute assigned at class-level: available in instances as well
+                labels.add("class-attribute")
+                labels.add("instance-attribute")
+            else:
+                # annotated attribute only: not available at class-level
+                labels.add("instance-attribute")
+
         elif parent.kind is Kind.FUNCTION:
             if parent.name != "__init__":
                 return
@@ -592,9 +605,11 @@ def handle_attribute(
 
                 with suppress(AliasResolutionError, CyclicAliasError):
                     labels |= parent.members[name].labels  # type: ignore[misc]
-                    # forward previous docstring instead of erasing it
+                    # forward previous docstring and annotation instead of erasing them
                     if parent.members[name].docstring and not docstring:
                         docstring = parent.members[name].docstring
+                    if parent.attributes[name].annotation and not annotation:
+                        annotation = parent.attributes[name].annotation
 
             attribute = Attribute(
                 name=name,
diff --git a/src/griffe/expressions.py b/src/griffe/expressions.py
index 0745575d..7e40b782 100644
--- a/src/griffe/expressions.py
+++ b/src/griffe/expressions.py
@@ -162,6 +162,15 @@ def is_generator(self) -> bool:
         """
         return self.kind == "generator"
 
+    @property
+    def is_classvar(self) -> bool:
+        """Tell whether this expression represents a ClassVar.
+
+        Returns:
+            True or False.
+        """
+        return isinstance(self[0], Name) and self[0].full == "typing.ClassVar"
+
     @cached_property
     def non_optional(self) -> Expression:
         """Return the same expression as non-optional.
diff --git a/tests/test_visitor.py b/tests/test_visitor.py
index 482d8203..1a7c3efe 100644
--- a/tests/test_visitor.py
+++ b/tests/test_visitor.py
@@ -297,3 +297,45 @@ def __init__(self, attr: int) -> None:
         ''',
     ) as module:
         assert module["C.attr"].docstring
+
+
+def test_classvar_annotations() -> None:
+    """Assert class variable and instance variable annotations are correctly parsed and merged."""
+    with temporary_visited_module(
+        """
+        from typing import ClassVar
+
+        class C:
+            w: ClassVar[str] = "foo"
+            x: ClassVar[int]
+            y: str
+            z: int = 5
+
+            def __init__(self) -> None:
+                self.a: ClassVar[float]
+                self.y = ""
+                self.b: bytes
+        """,
+    ) as module:
+        assert module["C.w"].annotation.full == "str"
+        assert module["C.w"].labels == {"class-attribute"}
+        assert module["C.w"].value == "'foo'"
+
+        assert module["C.x"].annotation.full == "int"
+        assert module["C.x"].labels == {"class-attribute"}
+
+        assert module["C.y"].annotation.full == "str"
+        assert module["C.y"].labels == {"instance-attribute"}
+        assert module["C.y"].value == "''"
+
+        assert module["C.z"].annotation.full == "int"
+        assert module["C.z"].labels == {"class-attribute", "instance-attribute"}
+        assert module["C.z"].value == "5"
+
+        # This is syntactically valid, but semantically invalid
+        assert module["C.a"].annotation[0].full == "typing.ClassVar"
+        assert module["C.a"].annotation[2].full == "float"
+        assert module["C.a"].labels == {"instance-attribute"}
+
+        assert module["C.b"].annotation.full == "bytes"
+        assert module["C.b"].labels == {"instance-attribute"}