From 259fbd23960d3f4bbb36d9a0c1ca60168dc8fcaa Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 9 Sep 2023 18:31:18 +0000 Subject: [PATCH] Fix Pyreverse optional annotation bug (#9016) (#9019) * Add alternative optional bug test cases * Fix Pyreverse optional annotation bug * Fix type error (cherry picked from commit a21f5d36a166c4da1f67ac68a5176d44d0392f2d) Co-authored-by: Nick Drozd --- doc/whatsnew/fragments/9014.bugfix | 3 +++ pylint/pyreverse/utils.py | 22 +++++++++++++------ .../annotations/attributes_annotation.dot | 2 +- .../annotations/attributes_annotation.mmd | 3 +++ .../annotations/attributes_annotation.puml | 3 +++ .../annotations/attributes_annotation.py | 3 +++ 6 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 doc/whatsnew/fragments/9014.bugfix diff --git a/doc/whatsnew/fragments/9014.bugfix b/doc/whatsnew/fragments/9014.bugfix new file mode 100644 index 0000000000..a461855e25 --- /dev/null +++ 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 index 54c093ecb9..97955f6be8 100644 --- a/pylint/pyreverse/utils.py +++ b/pylint/pyreverse/utils.py @@ -199,14 +199,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 index 94c242ccfc..6b0287c4a4 100644 --- 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|
|}>, shape="record", style="solid"]; -"attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{Dummy2|alternative_union_syntax : str \| int
class_attr : list[Dummy]
optional : Optional[Dummy]
param : str
union : Union[int, str]
|}>, shape="record", style="solid"]; +"attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{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]
|}>, 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 index 9f0b3d761c..aff946e7a8 100644 --- 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 index 1120832209..65bbb3755a 100644 --- 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 index d4fa931f51..3a4f52b4e6 100644 --- 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