Improved DotSerializer layout (#5246)

Basically revives #2002

joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/io/dotgenerator/DotAstGeneratorTests.scala

@@ -32,8 +32,10 @@ class DotAstGeneratorTests extends C2CpgSuite {
       inside(cpg.method.name("my_func").dotAst.l) { case List(x) =>
         x should (
           startWith("digraph \"my_func\"") and
-            include("""[label = <(CONTROL_STRUCTURE,IF,if (y &gt; 42)""") and
-            include("""[label = <(LITERAL,42,y &gt; 42)<SUB>5</SUB>> ]""") and
+            include(
+              """[label = <CONTROL_STRUCTURE, 5<BR/>IF<BR/>if (y &gt; 42) { return y; } else { return sqrt(y); }> ]"""
+            ) and
+            include("""[label = <LITERAL, 5<BR/>42<BR/>y &gt; 42> ]""") and
             endWith("}\n")
         )
       }
@@ -53,17 +55,17 @@ class DotAstGeneratorTests extends C2CpgSuite {
 
     "allow plotting sub trees of methods" in {
       inside(cpg.method.ast.isControlStructure.code(".*y > 42.*").dotAst.l) { case List(x, _) =>
-        x should (include("y &gt; 42") and include("IDENTIFIER,y") and not include "x * 2")
+        x should (include("y &gt; 42") and include("IDENTIFIER, 5<BR/>y") and not include "x * 2")
       }
     }
 
     "allow plotting sub trees of methods correctly escaped" in {
       inside(cpg.method.name("lemon").dotAst.l) { case List(x) =>
         x should (
           startWith("digraph \"lemon\"") and
-            include("""[label = <(goog,goog(&quot;\&quot;yes\&quot;&quot;))<SUB>18</SUB>> ]""") and
+            include("""[label = <goog, 18<BR/>goog(&quot;\&quot;yes\&quot;&quot;)> ]""") and
             include(
-              """[label = <(LITERAL,&quot;\&quot;yes\&quot;&quot;,goog(&quot;\&quot;yes\&quot;&quot;))<SUB>18</SUB>> ]"""
+              """[label = <LITERAL, 18<BR/>&quot;\&quot;yes\&quot;&quot;<BR/>goog(&quot;\&quot;yes\&quot;&quot;)> ]"""
             ) and
             endWith("}\n")
         )

joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/io/dotgenerator/DotCdgGeneratorTests.scala

@@ -19,9 +19,9 @@ class DotCdgGeneratorTests extends DataFlowCodeToCpgSuite {
         inside(cpg.method.name("foo").dotCdg.l) { case List(x) =>
           x should (
             startWith("digraph \"foo\"") and
-              include("""[label = <(&lt;operator&gt;.greaterThan,x &gt; 8)<SUB>3</SUB>> ]""") and
-              include("""[label = <(&lt;operator&gt;.assignment,z = a(x))<SUB>4</SUB>> ]""") and
-              include("""[label = <(a,a(x))<SUB>4</SUB>> ]""") and
+              include("""[label = <&lt;operator&gt;.greaterThan, 3<BR/>x &gt; 8> ]""") and
+              include("""[label = <&lt;operator&gt;.assignment, 4<BR/>z = a(x)> ]""") and
+              include("""[label = <a, 4<BR/>a(x)> ]""") and
               endWith("}\n")
           )
           val lines = x.split("\n")
@@ -46,9 +46,9 @@ class DotCdgGeneratorTests extends DataFlowCodeToCpgSuite {
         inside(cpg.method.name("foo").dotCdg.l) { case List(x) =>
           x should (
             startWith("digraph \"foo\"") and
-              include("""[label = <(&lt;operator&gt;.greaterThan,x &gt; 8)<SUB>3</SUB>> ]""") and
-              include("""[label = <(&lt;operator&gt;.assignment,z = a(x))<SUB>4</SUB>> ]""") and
-              include("""[label = <(a,a(x))<SUB>4</SUB>> ]""") and
+              include("""[label = <&lt;operator&gt;.greaterThan, 3<BR/>x &gt; 8> ]""") and
+              include("""[label = <&lt;operator&gt;.assignment, 4<BR/>z = a(x)> ]""") and
+              include("""[label = <a, 4<BR/>a(x)> ]""") and
               endWith("}\n")
           )
           val lines = x.split("\n")

joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/io/dotgenerator/DotCfgGeneratorTests.scala

@@ -21,7 +21,7 @@ class DotCfgGeneratorTests extends C2CpgSuite {
       inside(cpg.method.name("main").dotCfg.l) { case List(dotStr) =>
         dotStr should (
           startWith("digraph \"main\" {") and
-            include("(<operator>.assignment,i = 0)") and
+            include("[label = <&lt;operator&gt;.assignment, 3<BR/>i = 0> ]") and
             endWith("}\n")
         )
       }
@@ -80,9 +80,9 @@ class DotCfgGeneratorTests extends C2CpgSuite {
       inside(cpg.method.name("example").dotCfg.l) { case List(dotStr) =>
         dotStr should (
           startWith("digraph \"example\" {") and
-            include("<(IDENTIFIER,a,if(a) { foo(); })<SUB>4</SUB>>") and
-            include("<(IDENTIFIER,b,if(b) { foo_2(); })<SUB>5</SUB>>") and
-            include("<(IDENTIFIER,c,if (c) { foo_3(); })<SUB>6</SUB>>") and
+            include("[label = <IDENTIFIER, 4<BR/>a<BR/>if(a) { foo(); }> ]") and
+            include("[label = <IDENTIFIER, 5<BR/>b<BR/>if(b) { foo_2(); }> ]") and
+            include("[label = <IDENTIFIER, 6<BR/>c<BR/>if (c) { foo_3(); }> ]") and
             endWith("}\n")
         )
       }

semanticcpg/src/main/scala/io/shiftleft/semanticcpg/dotgenerator/DotSerializer.scala

@@ -1,20 +1,19 @@
 package io.shiftleft.semanticcpg.dotgenerator
 
-import flatgraph.Accessors
-import io.shiftleft.codepropertygraph.generated.PropertyNames
 import io.shiftleft.codepropertygraph.generated.nodes.*
+import io.shiftleft.codepropertygraph.generated.Properties
 import io.shiftleft.semanticcpg.language.*
 import io.shiftleft.semanticcpg.utils.MemberAccess
+import org.apache.commons.lang3.StringUtils
 import org.apache.commons.text.StringEscapeUtils
 
-import java.util.Optional
 import scala.collection.immutable.HashMap
 import scala.collection.mutable
 import scala.language.postfixOps
 
 object DotSerializer {
 
-  private val charLimit = 50
+  private val CharLimit = 50
 
   case class Graph(
     vertices: List[StoredNode],
@@ -40,6 +39,8 @@ object DotSerializer {
       case Some(r) => namedGraphBegin(r)
       case None    => defaultGraphBegin()
     }
+
+    sb.append(s"""node [shape="rect"];  \n""")
     val nodeStrings = graph.vertices.map(nodeToDot)
     val edgeStrings = graph.edges.map(e => edgeToDot(e, withEdgeTypes))
     val subgraphStrings = graph.subgraph.zipWithIndex.map { case ((subgraph, nodes), idx) =>
@@ -64,48 +65,45 @@ object DotSerializer {
     sb.append(s"""digraph "$name" {  \n""")
   }
 
-  private def limit(str: String): String = if (str.length > charLimit) {
-    s"${str.take(charLimit - 3)}..."
-  } else {
-    str
-  }
+  private def limit(str: String): String = StringUtils.abbreviate(str, CharLimit)
 
   private def stringRepr(vertex: StoredNode): String = {
-    // TODO MP after the initial flatgraph migration (where we want to maintain semantics as far as
-    // possible) this might become `vertex.property(Properties.LineNumber)` which derives to `Option[Int]`
-    val lineNoMaybe = vertex.propertyOption[Int](PropertyNames.LINE_NUMBER)
-
-    StringEscapeUtils.escapeHtml4(vertex match {
-      case call: Call                            => (call.name, limit(call.code)).toString
-      case contrl: ControlStructure              => (contrl.label, contrl.controlStructureType, contrl.code).toString
-      case expr: Expression                      => (expr.label, limit(expr.code), limit(toCfgNode(expr).code)).toString
-      case method: Method                        => (method.label, method.name).toString
-      case ret: MethodReturn                     => (ret.label, ret.typeFullName).toString
-      case param: MethodParameterIn              => ("PARAM", param.code).toString
-      case local: Local                          => (local.label, s"${local.code}: ${local.typeFullName}").toString
-      case target: JumpTarget                    => (target.label, target.name).toString
-      case modifier: Modifier                    => (modifier.label, modifier.modifierType).toString()
-      case annoAssign: AnnotationParameterAssign => (annoAssign.label, annoAssign.code).toString()
-      case annoParam: AnnotationParameter        => (annoParam.label, annoParam.code).toString()
-      case typ: Type                             => (typ.label, typ.name).toString()
-      case typeDecl: TypeDecl                    => (typeDecl.label, typeDecl.name).toString()
-      case member: Member                        => (member.label, member.name).toString()
-      case _                                     => ""
-    }) + lineNoMaybe.map(lineNo => s"<SUB>$lineNo</SUB>").getOrElse("")
+    val lineOpt = vertex.property(Properties.LineNumber).map(_.toString)
+    val attrList = (vertex match {
+      case call: Call                            => List(call.name, limit(call.code))
+      case ctrl: ControlStructure                => List(ctrl.label, ctrl.controlStructureType, ctrl.code)
+      case expr: Expression                      => List(expr.label, limit(expr.code), limit(toCfgNode(expr).code))
+      case method: Method                        => List(method.label, method.name)
+      case ret: MethodReturn                     => List(ret.label, ret.typeFullName)
+      case param: MethodParameterIn              => List("PARAM", param.code)
+      case local: Local                          => List(local.label, s"${local.code}: ${local.typeFullName}")
+      case target: JumpTarget                    => List(target.label, target.name)
+      case modifier: Modifier                    => List(modifier.label, modifier.modifierType)
+      case annoAssign: AnnotationParameterAssign => List(annoAssign.label, annoAssign.code)
+      case annoParam: AnnotationParameter        => List(annoParam.label, annoParam.code)
+      case typ: Type                             => List(typ.label, typ.name)
+      case typeDecl: TypeDecl                    => List(typeDecl.label, typeDecl.name)
+      case member: Member                        => List(member.label, member.name)
+      case _                                     => List.empty
+    }).map(l => StringEscapeUtils.escapeHtml4(StringUtils.normalizeSpace(l)))
+
+    (lineOpt match {
+      case Some(line) => s"${attrList.head}, $line" :: attrList.tail
+      case None       => attrList
+    }).distinct.mkString("<BR/>")
   }
 
   private def toCfgNode(node: StoredNode): CfgNode = {
     node match {
-      case node: Identifier         => node.parentExpression.get
-      case node: MethodRef          => node.parentExpression.get
-      case node: Literal            => node.parentExpression.get
-      case node: MethodParameterIn  => node.method
-      case node: MethodParameterOut => node.method.methodReturn
-      case node: Call if MemberAccess.isGenericMemberAccessName(node.name) =>
-        node.parentExpression.get
-      case node: CallRepr     => node
-      case node: MethodReturn => node
-      case node: Expression   => node
+      case node: Identifier                                                => node.parentExpression.get
+      case node: MethodRef                                                 => node.parentExpression.get
+      case node: Literal                                                   => node.parentExpression.get
+      case node: Call if MemberAccess.isGenericMemberAccessName(node.name) => node.parentExpression.get
+      case node: MethodParameterOut                                        => node.method.methodReturn
+      case node: MethodParameterIn                                         => node.method
+      case node: CallRepr                                                  => node
+      case node: MethodReturn                                              => node
+      case node: Expression                                                => node
     }
   }
 
@@ -123,7 +121,7 @@ object DotSerializer {
     s"""  "${edge.src.id}" -> "${edge.dst.id}" """ + labelStr
   }
 
-  def nodesToSubGraphs(subgraph: String, children: Seq[StoredNode], idx: Int): String = {
+  private def nodesToSubGraphs(subgraph: String, children: Seq[StoredNode], idx: Int): String = {
     val escapedName = StringEscapeUtils.escapeHtml4(subgraph)
     val childString = children.map { c => s"    \"${c.id()}\";" }.mkString("\n")
     s"""  subgraph cluster_$idx {