Fix pyreverse type hints for methods returning None (#5916)

12 files changed, 43 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index 99ad7a237..49a41526c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -17,6 +17,8 @@ Release date: TBA
 
 * Fix pyreverse diagrams type hinting for classmethods and staticmethods.
 
+* Fix pyreverse diagrams type hinting for methods returning None.
+
 * Fix matching ``--notes`` options that end in a non-word character.
 
   Closes #5840
diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst
index d0696f1db..687274a65 100644
--- a/doc/whatsnew/2.13.rst
+++ b/doc/whatsnew/2.13.rst
@@ -506,3 +506,5 @@ Other Changes
 * Fixed false positive for ``global-variable-not-assigned`` when the ``del`` statement is used
 
   Closes #5333
+
+* Fix type hints in class diagrams generated by pyreverse for class methods and methods returning None.
diff --git a/pylint/pyreverse/utils.py b/pylint/pyreverse/utils.py
index 52bfb4fbd..7fde44225 100644
--- a/pylint/pyreverse/utils.py
+++ b/pylint/pyreverse/utils.py
@@ -224,13 +224,12 @@ class LocalsVisitor(ASTWalker):
         return None
 
 
-def get_annotation_label(ann: Union[nodes.Name, nodes.Subscript]) -> str:
-    label = ""
-    if isinstance(ann, nodes.Subscript):
-        label = ann.as_string()
-    elif isinstance(ann, nodes.Name):
-        label = ann.name
-    return label
+def get_annotation_label(ann: Union[nodes.Name, nodes.NodeNG]) -> str:
+    if isinstance(ann, nodes.Name) and ann.name is not None:
+        return ann.name
+    if isinstance(ann, nodes.NodeNG):
+        return ann.as_string()
+    return ""
 
 
 def get_annotation(
diff --git a/tests/data/clientmodule_test.py b/tests/data/clientmodule_test.py
index 257aadec1..35c39684b 100644
--- a/tests/data/clientmodule_test.py
+++ b/tests/data/clientmodule_test.py
@@ -36,3 +36,6 @@ class Specialization(Ancestor):
     @staticmethod
     def transform_value(value: int) -> int:
         return value * 2
+
+    def increment_value(self) -> None:
+        self.set_value(self.get_value() + 1)
diff --git a/tests/pyreverse/data/classes_No_Name.dot b/tests/pyreverse/data/classes_No_Name.dot
index e5a304ce3..1f3f705e7 100644
--- a/tests/pyreverse/data/classes_No_Name.dot
+++ b/tests/pyreverse/data/classes_No_Name.dot
@@ -8,7 +8,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoSomething" [color="black", fontcolor="black", label="{DoSomething|my_int : Optional[int]\lmy_int_2 : Optional[int]\lmy_string : str\l|do_it(new_int: int): int\l}", shape="record", style="solid"];
 "data.suppliermodule_test.Interface" [color="black", fontcolor="black", label="{Interface|\l|get_value()\lset_value(value)\l}", shape="record", style="solid"];
 "data.property_pattern.PropertyPatterns" [color="black", fontcolor="black", label="{PropertyPatterns|prop1\lprop2\l|}", shape="record", style="solid"];
-"data.clientmodule_test.Specialization" [color="black", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|from_value(value: int)\ltransform_value(value: int): int\l}", shape="record", style="solid"];
+"data.clientmodule_test.Specialization" [color="black", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|from_value(value: int)\lincrement_value(): None\ltransform_value(value: int): int\l}", shape="record", style="solid"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];
 "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"];
 "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Ancestor" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="cls_member", style="solid"];
diff --git a/tests/pyreverse/data/classes_No_Name.html b/tests/pyreverse/data/classes_No_Name.html
index 420a869d4..b4ada04b8 100644
--- a/tests/pyreverse/data/classes_No_Name.html
+++ b/tests/pyreverse/data/classes_No_Name.html
@@ -36,6 +36,7 @@
             relation2
             top : str
             from_value(value: int)
+            increment_value() -> None
             transform_value(value: int) -> int
           }
           Specialization --|> Ancestor
diff --git a/tests/pyreverse/data/classes_No_Name.mmd b/tests/pyreverse/data/classes_No_Name.mmd
index 50da7f39c..efdaacae1 100644
--- a/tests/pyreverse/data/classes_No_Name.mmd
+++ b/tests/pyreverse/data/classes_No_Name.mmd
@@ -31,6 +31,7 @@ classDiagram
     relation2
     top : str
     from_value(value: int)
+    increment_value() -> None
     transform_value(value: int) -> int
   }
   Specialization --|> Ancestor
diff --git a/tests/pyreverse/data/classes_No_Name.puml b/tests/pyreverse/data/classes_No_Name.puml
index a0f8350d0..37767b321 100644
--- a/tests/pyreverse/data/classes_No_Name.puml
+++ b/tests/pyreverse/data/classes_No_Name.puml
@@ -32,6 +32,7 @@ class "Specialization" as data.clientmodule_test.Specialization {
   relation2
   top : str
   from_value(value: int)
+  increment_value() -> None
   transform_value(value: int) -> int
 }
 data.clientmodule_test.Specialization --|> data.clientmodule_test.Ancestor
diff --git a/tests/pyreverse/data/classes_No_Name.vcg b/tests/pyreverse/data/classes_No_Name.vcg
index 6497f26b4..4c792db69 100644
--- a/tests/pyreverse/data/classes_No_Name.vcg
+++ b/tests/pyreverse/data/classes_No_Name.vcg
@@ -25,7 +25,7 @@ graph:{
   node: {title:"data.property_pattern.PropertyPatterns"  label:"\fbPropertyPatterns\fn\n\f__________________\n\f08prop1\n\f08prop2\n\f__________________"
   shape:box
 }
-  node: {title:"data.clientmodule_test.Specialization"  label:"\fbSpecialization\fn\n\f_________________\n\f08TYPE : str\n\f08relation\n\f08relation2\n\f08top : str\n\f_________________\n\f10from_value()\n\f10transform_value()"
+  node: {title:"data.clientmodule_test.Specialization"  label:"\fbSpecialization\fn\n\f_________________\n\f08TYPE : str\n\f08relation\n\f08relation2\n\f08top : str\n\f_________________\n\f10from_value()\n\f10increment_value()\n\f10transform_value()"
   shape:box
 }
   edge: {sourcename:"data.clientmodule_test.Specialization" targetname:"data.clientmodule_test.Ancestor"  arrowstyle:solid
diff --git a/tests/pyreverse/data/classes_colorized.dot b/tests/pyreverse/data/classes_colorized.dot
index 3b68c7933..72f30658d 100644
--- a/tests/pyreverse/data/classes_colorized.dot
+++ b/tests/pyreverse/data/classes_colorized.dot
@@ -8,7 +8,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoSomething" [color="aliceblue", fontcolor="black", label="{DoSomething|my_int : Optional[int]\lmy_int_2 : Optional[int]\lmy_string : str\l|do_it(new_int: int): int\l}", shape="record", style="filled"];
 "data.suppliermodule_test.Interface" [color="aliceblue", fontcolor="black", label="{Interface|\l|get_value()\lset_value(value)\l}", shape="record", style="filled"];
 "data.property_pattern.PropertyPatterns" [color="aliceblue", fontcolor="black", label="{PropertyPatterns|prop1\lprop2\l|}", shape="record", style="filled"];
-"data.clientmodule_test.Specialization" [color="aliceblue", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|from_value(value: int)\ltransform_value(value: int): int\l}", shape="record", style="filled"];
+"data.clientmodule_test.Specialization" [color="aliceblue", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|from_value(value: int)\lincrement_value(): None\ltransform_value(value: int): int\l}", shape="record", style="filled"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];
 "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"];
 "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Ancestor" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="cls_member", style="solid"];
diff --git a/tests/pyreverse/data/classes_colorized.puml b/tests/pyreverse/data/classes_colorized.puml
index 8a37f090f..1226f7b4e 100644
--- a/tests/pyreverse/data/classes_colorized.puml
+++ b/tests/pyreverse/data/classes_colorized.puml
@@ -32,6 +32,7 @@ class "Specialization" as data.clientmodule_test.Specialization #aliceblue {
   relation2
   top : str
   from_value(value: int)
+  increment_value() -> None
   transform_value(value: int) -> int
 }
 data.clientmodule_test.Specialization --|> data.clientmodule_test.Ancestor
diff --git a/tests/pyreverse/test_utils.py b/tests/pyreverse/test_utils.py
index f571b1847..322b5bea2 100644
--- a/tests/pyreverse/test_utils.py
+++ b/tests/pyreverse/test_utils.py
@@ -17,7 +17,12 @@ import astroid
 import pytest
 from astroid import nodes
 
-from pylint.pyreverse.utils import get_annotation, get_visibility, infer_node
+from pylint.pyreverse.utils import (
+    get_annotation,
+    get_annotation_label,
+    get_visibility,
+    infer_node,
+)
 
 
 @pytest.mark.parametrize(
@@ -81,6 +86,23 @@ def test_get_annotation_assignattr(init_method, label):
             assert got == label, f"got {got} instead of {label} for value {node}"
 
 
+@pytest.mark.parametrize(
+    "node_text, expected_label",
+    [
+        ("def f() -> None: pass", "None"),
+        ("def f() -> int: pass", "int"),
+        ("def f(a) -> Optional[int]: return 1 if a else None", "Optional[int]"),
+        ("def f() -> 'MyType': pass", "'MyType'"),
+    ],
+)
+def test_get_annotation_label_of_return_type(
+    node_text: str, expected_label: str
+) -> None:
+    func = astroid.extract_node(node_text)
+    assert isinstance(func, nodes.FunctionDef)
+    assert get_annotation_label(func.returns) == expected_label
+
+
 @patch("pylint.pyreverse.utils.get_annotation")
 @patch("astroid.node_classes.NodeNG.infer", side_effect=astroid.InferenceError)
 def test_infer_node_1(mock_infer: Any, mock_get_annotation: Any) -> None: