Fix Pyreverse optional annotation bug (#9016)

* Add alternative optional bug test cases * Fix Pyreverse optional annotation bug * Fix type error
pylint-dev · Sep 9, 2023 · a21f5d3 · a21f5d3
1 parent fbbf788
commit a21f5d3
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 8 deletions.
diff --git a/doc/whatsnew/fragments/9014.bugfix b/doc/whatsnew/fragments/9014.bugfix
@@ -0,0 +1,3 @@
+Don't add `Optional` to `|` annotations with `None` in Pyreverse diagrams.
+
+Closes #9014
diff --git a/pylint/pyreverse/utils.py b/pylint/pyreverse/utils.py
@@ -194,14 +194,22 @@ def get_annotation(
         default = ""
 
     label = get_annotation_label(ann)
-    if ann:
-        label = (
-            rf"Optional[{label}]"
-            if getattr(default, "value", "value") is None
-            and not label.startswith("Optional")
-            else label
+
+    if (
+        ann
+        and getattr(default, "value", "value") is None
+        and not label.startswith("Optional")
+        and (
+            not isinstance(ann, nodes.BinOp)
+            or not any(
+                isinstance(child, nodes.Const) and child.value is None
+                for child in ann.get_children()
+            )
         )
-    if label:
+    ):
+        label = rf"Optional[{label}]"
+
+    if label and ann:
         ann.name = label
     return ann
 

diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot
@@ -2,5 +2,5 @@ digraph "classes" {
 rankdir=BT
 charset="utf-8"
 "attributes_annotation.Dummy" [color="black", fontcolor="black", label=<{Dummy|<br ALIGN="LEFT"/>|}>, shape="record", style="solid"];
-"attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{Dummy2|alternative_union_syntax : str \| int<br ALIGN="LEFT"/>class_attr : list[Dummy]<br ALIGN="LEFT"/>optional : Optional[Dummy]<br ALIGN="LEFT"/>param : str<br ALIGN="LEFT"/>union : Union[int, str]<br ALIGN="LEFT"/>|}>, shape="record", style="solid"];
+"attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{Dummy2|alternative_optional : int \| None<br ALIGN="LEFT"/>alternative_optional_swapped : None \| int<br ALIGN="LEFT"/>alternative_union_syntax : str \| int<br ALIGN="LEFT"/>class_attr : list[Dummy]<br ALIGN="LEFT"/>optional : Optional[Dummy]<br ALIGN="LEFT"/>optional_union : Optional[int \| str]<br ALIGN="LEFT"/>param : str<br ALIGN="LEFT"/>union : Union[int, str]<br ALIGN="LEFT"/>|}>, shape="record", style="solid"];
 }
diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd
@@ -2,9 +2,12 @@ classDiagram
   class Dummy {
   }
   class Dummy2 {
+    alternative_optional : int | None
+    alternative_optional_swapped : None | int
     alternative_union_syntax : str | int
     class_attr : list[Dummy]
     optional : Optional[Dummy]
+    optional_union : Optional[int | str]
     param : str
     union : Union[int, str]
   }
diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml
@@ -3,9 +3,12 @@ set namespaceSeparator none
 class "Dummy" as attributes_annotation.Dummy {
 }
 class "Dummy2" as attributes_annotation.Dummy2 {
+  alternative_optional : int | None
+  alternative_optional_swapped : None | int
   alternative_union_syntax : str | int
   class_attr : list[Dummy]
   optional : Optional[Dummy]
+  optional_union : Optional[int | str]
   param : str
   union : Union[int, str]
 }

diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.py b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.py
@@ -16,3 +16,6 @@ def __init__(self, param: str) -> None:
         self.union: Union[int, str] = ""
         self.alternative_union_syntax: str | int = 0
         self.optional: Optional[Dummy] = None
+        self.alternative_optional: int | None = None
+        self.alternative_optional_swapped: None | int = None
+        self.optional_union: int | str = None