Added escaping of vertical bar character in annotation labels (#8610) (…

…#8631)
pylint-dev · Apr 27, 2023 · 4350c6f · 4350c6f
1 parent e507a4d
commit 4350c6f
Show file tree

Hide file tree

Showing 19 changed files with 80 additions and 25 deletions.
diff --git a/doc/whatsnew/fragments/8603.bugfix b/doc/whatsnew/fragments/8603.bugfix
@@ -0,0 +1,3 @@
+``pyreverse``: added escaping of vertical bar character in annotation labels produced by DOT printer to ensure it is not treated as field separator of record-based nodes.
+
+Closes #8603
diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py
@@ -121,11 +121,19 @@ def _build_label_for_node(self, properties: NodeProperties) -> str:
             )
             label += rf"{method_name}({', '.join(args)})"
             if func.returns:
-                label += ": " + get_annotation_label(func.returns)
+                annotation_label = get_annotation_label(func.returns)
+                label += ": " + self._escape_annotation_label(annotation_label)
             label += rf"{HTMLLabels.LINEBREAK_LEFT.value}"
         label += "}"
         return label
 
+    def _escape_annotation_label(self, annotation_label: str) -> str:
+        # Escape vertical bar characters to make them appear as a literal characters
+        # otherwise it gets treated as field separator of record-based nodes
+        annotation_label = annotation_label.replace("|", r"\|")
+
+        return annotation_label
+
     def emit_edge(
         self,
         from_node: str,

diff --git a/tests/data/nullable_pattern.py b/tests/data/nullable_pattern.py
@@ -0,0 +1,10 @@
+""" docstring for file nullable_pattern.py """
+from typing import Optional
+
+class NullablePatterns:
+    def return_nullable_1(self) -> int | None:
+        """ Nullable return type using the | operator as mentioned in PEP 604, see https://peps.python.org/pep-0604 """
+        pass
+
+    def return_nullable_2(self) -> Optional[int]:
+        pass
diff --git a/tests/pyreverse/data/classes_No_Name.dot b/tests/pyreverse/data/classes_No_Name.dot
@@ -7,6 +7,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoNothing2" [color="black", fontcolor="black", label=<{DoNothing2|<br ALIGN="LEFT"/>|}>, shape="record", style="solid"];
 "data.suppliermodule_test.DoSomething" [color="black", fontcolor="black", label=<{DoSomething|my_int : Optional[int]<br ALIGN="LEFT"/>my_int_2 : Optional[int]<br ALIGN="LEFT"/>my_string : str<br ALIGN="LEFT"/>|do_it(new_int: int): int<br ALIGN="LEFT"/>}>, shape="record", style="solid"];
 "data.suppliermodule_test.Interface" [color="black", fontcolor="black", label=<{Interface|<br ALIGN="LEFT"/>|<I>get_value</I>()<br ALIGN="LEFT"/><I>set_value</I>(value)<br ALIGN="LEFT"/>}>, shape="record", style="solid"];
+"data.nullable_pattern.NullablePatterns" [color="black", fontcolor="black", label=<{NullablePatterns|<br ALIGN="LEFT"/>|<I>return_nullable_1</I>(): int \| None<br ALIGN="LEFT"/><I>return_nullable_2</I>(): Optional[int]<br ALIGN="LEFT"/>}>, shape="record", style="solid"];
 "data.property_pattern.PropertyPatterns" [color="black", fontcolor="black", label=<{PropertyPatterns|prop1<br ALIGN="LEFT"/>prop2<br ALIGN="LEFT"/>|}>, shape="record", style="solid"];
 "data.clientmodule_test.Specialization" [color="black", fontcolor="black", label=<{Specialization|TYPE : str<br ALIGN="LEFT"/>relation<br ALIGN="LEFT"/>relation2<br ALIGN="LEFT"/>top : str<br ALIGN="LEFT"/>|from_value(value: int)<br ALIGN="LEFT"/>increment_value(): None<br ALIGN="LEFT"/>transform_value(value: int): int<br ALIGN="LEFT"/>}>, shape="record", style="solid"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];

diff --git a/tests/pyreverse/data/classes_No_Name.html b/tests/pyreverse/data/classes_No_Name.html
@@ -26,6 +26,10 @@
             get_value()*
             set_value(value)*
           }
+          class NullablePatterns {
+            return_nullable_1()* int | None
+            return_nullable_2()* Optional[int]
+          }
           class PropertyPatterns {
             prop1
             prop2
@@ -44,7 +48,7 @@
           DoNothing --* Ancestor : cls_member
           DoNothing --* Specialization : relation
           DoNothing2 --o Specialization : relation2
-
+  
        </div>
   </body>
 </html>
diff --git a/tests/pyreverse/data/classes_No_Name.mmd b/tests/pyreverse/data/classes_No_Name.mmd
@@ -21,6 +21,10 @@ classDiagram
     get_value()*
     set_value(value)*
   }
+  class NullablePatterns {
+    return_nullable_1()* int | None
+    return_nullable_2()* Optional[int]
+  }
   class PropertyPatterns {
     prop1
     prop2

diff --git a/tests/pyreverse/data/classes_No_Name.puml b/tests/pyreverse/data/classes_No_Name.puml
@@ -22,6 +22,10 @@ class "Interface" as data.suppliermodule_test.Interface {
   {abstract}get_value()
   {abstract}set_value(value)
 }
+class "NullablePatterns" as data.nullable_pattern.NullablePatterns {
+  {abstract}return_nullable_1() -> int | None
+  {abstract}return_nullable_2() -> Optional[int]
+}
 class "PropertyPatterns" as data.property_pattern.PropertyPatterns {
   prop1
   prop2

diff --git a/tests/pyreverse/data/classes_No_Name.vcg b/tests/pyreverse/data/classes_No_Name.vcg
@@ -6,50 +6,53 @@ graph:{
   manhattan_edges:yes
   node: {title:"data.clientmodule_test.Ancestor"  label:"\fbAncestor\fn\n\f____________\n\f08attr : str\n\f08cls_member\n\f____________\n\f10get_value()\n\f10set_value()"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test.CustomException"  label:"\fb09CustomException\fn\n\f_________________"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test.DoNothing"  label:"\fbDoNothing\fn\n\f___________"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test.DoNothing2"  label:"\fbDoNothing2\fn\n\f____________"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test.DoSomething"  label:"\fbDoSomething\fn\n\f__________________________\n\f08my_int : Optional[int]\n\f08my_int_2 : Optional[int]\n\f08my_string : str\n\f__________________________\n\f10do_it()"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test.Interface"  label:"\fbInterface\fn\n\f___________\n\f10get_value()\n\f10set_value()"
   shape:box
-}
+  }
+  node: {title:"data.nullable_pattern.NullablePatterns"  label:"\fbNullablePatterns\fn\n\f___________________\n\f10return_nullable_1()\n\f10return_nullable_2()"
+  shape:box
+  }
   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\f10increment_value()\n\f10transform_value()"
   shape:box
-}
+  }
   edge: {sourcename:"data.clientmodule_test.Specialization" targetname:"data.clientmodule_test.Ancestor"  arrowstyle:solid
   backarrowstyle:none
   backarrowsize:10
-}
+  }
   edge: {sourcename:"data.clientmodule_test.Ancestor" targetname:"data.suppliermodule_test.Interface"  arrowstyle:solid
   backarrowstyle:none
   linestyle:dotted
   backarrowsize:10
-}
+  }
   edge: {sourcename:"data.suppliermodule_test.DoNothing" targetname:"data.clientmodule_test.Ancestor"  arrowstyle:solid
   backarrowstyle:none
   textcolor:green
   label:"cls_member"
-}
+  }
   edge: {sourcename:"data.suppliermodule_test.DoNothing" targetname:"data.clientmodule_test.Specialization"  arrowstyle:solid
   backarrowstyle:none
   textcolor:green
   label:"relation"
-}
+  }
   edge: {sourcename:"data.suppliermodule_test.DoNothing2" targetname:"data.clientmodule_test.Specialization"  arrowstyle:solid
   backarrowstyle:none
   textcolor:green
   label:"relation2"
-}
+  }
 }
diff --git a/tests/pyreverse/data/classes_colorized.dot b/tests/pyreverse/data/classes_colorized.dot
@@ -7,6 +7,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoNothing2" [color="aliceblue", fontcolor="black", label=<{DoNothing2|<br ALIGN="LEFT"/>|}>, shape="record", style="filled"];
 "data.suppliermodule_test.DoSomething" [color="aliceblue", fontcolor="black", label=<{DoSomething|my_int : Optional[int]<br ALIGN="LEFT"/>my_int_2 : Optional[int]<br ALIGN="LEFT"/>my_string : str<br ALIGN="LEFT"/>|do_it(new_int: int): int<br ALIGN="LEFT"/>}>, shape="record", style="filled"];
 "data.suppliermodule_test.Interface" [color="aliceblue", fontcolor="black", label=<{Interface|<br ALIGN="LEFT"/>|<I>get_value</I>()<br ALIGN="LEFT"/><I>set_value</I>(value)<br ALIGN="LEFT"/>}>, shape="record", style="filled"];
+"data.nullable_pattern.NullablePatterns" [color="aliceblue", fontcolor="black", label=<{NullablePatterns|<br ALIGN="LEFT"/>|<I>return_nullable_1</I>(): int \| None<br ALIGN="LEFT"/><I>return_nullable_2</I>(): Optional[int]<br ALIGN="LEFT"/>}>, shape="record", style="filled"];
 "data.property_pattern.PropertyPatterns" [color="aliceblue", fontcolor="black", label=<{PropertyPatterns|prop1<br ALIGN="LEFT"/>prop2<br ALIGN="LEFT"/>|}>, shape="record", style="filled"];
 "data.clientmodule_test.Specialization" [color="aliceblue", fontcolor="black", label=<{Specialization|TYPE : str<br ALIGN="LEFT"/>relation<br ALIGN="LEFT"/>relation2<br ALIGN="LEFT"/>top : str<br ALIGN="LEFT"/>|from_value(value: int)<br ALIGN="LEFT"/>increment_value(): None<br ALIGN="LEFT"/>transform_value(value: int): int<br ALIGN="LEFT"/>}>, shape="record", style="filled"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];

diff --git a/tests/pyreverse/data/classes_colorized.puml b/tests/pyreverse/data/classes_colorized.puml
@@ -22,6 +22,10 @@ class "Interface" as data.suppliermodule_test.Interface #aliceblue {
   {abstract}get_value()
   {abstract}set_value(value)
 }
+class "NullablePatterns" as data.nullable_pattern.NullablePatterns #aliceblue {
+  {abstract}return_nullable_1() -> int | None
+  {abstract}return_nullable_2() -> Optional[int]
+}
 class "PropertyPatterns" as data.property_pattern.PropertyPatterns #aliceblue {
   prop1
   prop2

diff --git a/tests/pyreverse/data/packages_No_Name.dot b/tests/pyreverse/data/packages_No_Name.dot
@@ -3,6 +3,7 @@ rankdir=BT
 charset="utf-8"
 "data" [color="black", label=<data>, shape="box", style="solid"];
 "data.clientmodule_test" [color="black", label=<data.clientmodule_test>, shape="box", style="solid"];
+"data.nullable_pattern" [color="black", label=<data.nullable_pattern>, shape="box", style="solid"];
 "data.property_pattern" [color="black", label=<data.property_pattern>, shape="box", style="solid"];
 "data.suppliermodule_test" [color="black", label=<data.suppliermodule_test>, shape="box", style="solid"];
 "data.clientmodule_test" -> "data.suppliermodule_test" [arrowhead="open", arrowtail="none"];

diff --git a/tests/pyreverse/data/packages_No_Name.html b/tests/pyreverse/data/packages_No_Name.html
@@ -8,12 +8,14 @@
           }
           class clientmodule_test {
           }
+          class nullable_pattern {
+          }
           class property_pattern {
           }
           class suppliermodule_test {
           }
           clientmodule_test --> suppliermodule_test
-
+  
        </div>
   </body>
 </html>
diff --git a/tests/pyreverse/data/packages_No_Name.mmd b/tests/pyreverse/data/packages_No_Name.mmd
@@ -3,6 +3,8 @@ classDiagram
   }
   class clientmodule_test {
   }
+  class nullable_pattern {
+  }
   class property_pattern {
   }
   class suppliermodule_test {

diff --git a/tests/pyreverse/data/packages_No_Name.puml b/tests/pyreverse/data/packages_No_Name.puml
@@ -1,16 +1,14 @@
 @startuml packages_No_Name
 set namespaceSeparator none
 package "data" as data {
-
 }
 package "data.clientmodule_test" as data.clientmodule_test {
-
+}
+package "data.nullable_pattern" as data.nullable_pattern {
 }
 package "data.property_pattern" as data.property_pattern {
-
 }
 package "data.suppliermodule_test" as data.suppliermodule_test {
-
 }
 data.clientmodule_test --> data.suppliermodule_test
 @enduml
diff --git a/tests/pyreverse/data/packages_No_Name.vcg b/tests/pyreverse/data/packages_No_Name.vcg
@@ -6,18 +6,21 @@ graph:{
   manhattan_edges:yes
   node: {title:"data"  label:"\fbdata\fn"
   shape:box
-}
+  }
   node: {title:"data.clientmodule_test"  label:"\fbdata.clientmodule_test\fn"
   shape:box
-}
+  }
+  node: {title:"data.nullable_pattern"  label:"\fbdata.nullable_pattern\fn"
+  shape:box
+  }
   node: {title:"data.property_pattern"  label:"\fbdata.property_pattern\fn"
   shape:box
-}
+  }
   node: {title:"data.suppliermodule_test"  label:"\fbdata.suppliermodule_test\fn"
   shape:box
-}
+  }
   edge: {sourcename:"data.clientmodule_test" targetname:"data.suppliermodule_test"  arrowstyle:solid
   backarrowstyle:none
   backarrowsize:0
-}
+  }
 }
diff --git a/tests/pyreverse/data/packages_colorized.dot b/tests/pyreverse/data/packages_colorized.dot
@@ -3,6 +3,7 @@ rankdir=BT
 charset="utf-8"
 "data" [color="aliceblue", label=<data>, shape="box", style="filled"];
 "data.clientmodule_test" [color="aliceblue", label=<data.clientmodule_test>, shape="box", style="filled"];
+"data.nullable_pattern" [color="aliceblue", label=<data.nullable_pattern>, shape="box", style="filled"];
 "data.property_pattern" [color="aliceblue", label=<data.property_pattern>, shape="box", style="filled"];
 "data.suppliermodule_test" [color="aliceblue", label=<data.suppliermodule_test>, shape="box", style="filled"];
 "data.clientmodule_test" -> "data.suppliermodule_test" [arrowhead="open", arrowtail="none"];

diff --git a/tests/pyreverse/data/packages_colorized.puml b/tests/pyreverse/data/packages_colorized.puml
@@ -5,6 +5,9 @@ package "data" as data #aliceblue {
 }
 package "data.clientmodule_test" as data.clientmodule_test #aliceblue {
 
+}
+package "data.nullable_pattern" as data.nullable_pattern #aliceblue {
+
 }
 package "data.property_pattern" as data.property_pattern #aliceblue {
 

diff --git a/tests/pyreverse/test_diadefs.py b/tests/pyreverse/test_diadefs.py
@@ -141,6 +141,7 @@ def test_known_values1(HANDLER: DiadefsHandler, PROJECT: Project) -> None:
     assert modules == [
         (True, "data"),
         (True, "data.clientmodule_test"),
+        (True, "data.nullable_pattern"),
         (True, "data.property_pattern"),
         (True, "data.suppliermodule_test"),
     ]
@@ -154,6 +155,7 @@ def test_known_values1(HANDLER: DiadefsHandler, PROJECT: Project) -> None:
         (True, "DoNothing2"),
         (True, "DoSomething"),
         (True, "Interface"),
+        (True, "NullablePatterns"),
         (True, "PropertyPatterns"),
         (True, "Specialization"),
     ]

diff --git a/tests/pyreverse/test_inspector.py b/tests/pyreverse/test_inspector.py
@@ -136,6 +136,7 @@ def test_project_node(project: Project) -> None:
     expected = [
         "data",
         "data.clientmodule_test",
+        "data.nullable_pattern",
         "data.property_pattern",
         "data.suppliermodule_test",
     ]