Preserve more information about unreachable symbols (scala-native#3537)

WojciechMazur · web-flow · commit 5b617a48e6bb · 2023-10-06T11:22:06.000+02:00
* Preserve more information about unreachable symbols including arg types and result type
* Adapt logging unreachable symbol stackreaces and improve their formatting
* Respect NO_COLOR env var when printing backtrace
diff --git a/tools/src/main/scala/scala/scalanative/build/ScalaNative.scala b/tools/src/main/scala/scala/scalanative/build/ScalaNative.scala
@@ -107,26 +107,49 @@ private[scalanative] object ScalaNative {
         analysis: ReachabilityAnalysis.Failure
     ): Unit = {
       val log = config.logger
+      // see https://no-color.org/
+      val noColor = sys.env.contains("NO_COLOR")
       def appendBackTrace(
           buf: StringBuilder,
           backtrace: List[Reach.BackTraceElement]
       ): Unit = {
+        import scala.io.AnsiColor._
         // Build stacktrace in memory to prevent its spliting when logging asynchronously
-        val padding = backtrace.foldLeft(0)(_ max _.kind.length())
-        backtrace.foreach {
-          case Reach.BackTraceElement(_, kind, name, filename, line) =>
-            val pad = " " * (padding - kind.length())
-            buf.append(s"    $pad$kind at $name($filename:$line)\n")
+        val elems = backtrace.map {
+          case elem @ Reach.BackTraceElement(_, symbol, filename, line) =>
+            import symbol.argTypes
+            val rendered = symbol.toString
+            val descriptorStart = rendered.indexOf(symbol.name)
+            val uncolored @ (modifiers, descriptor) =
+              rendered.splitAt(descriptorStart)
+
+            if (noColor) uncolored
+            else {
+              val (name, typeInfo) =
+                if (argTypes.nonEmpty)
+                  descriptor.splitAt(descriptor.indexOf("("))
+                else (descriptor, "")
+              modifiers -> s"$BOLD$YELLOW$name$RESET$typeInfo at $BOLD$filename:$line"
+            }
+        }
+        val padding = elems
+          .map(_._1.length)
+          .max
+          .min(14) + 2
+        elems.foreach {
+          case (modifiers, tracedDescriptor) =>
+            val pad = " " * (padding - modifiers.length)
+            buf.append(s"$pad$modifiers$tracedDescriptor\n")
         }
         buf.append("\n")
       }
 
       if (analysis.unreachable.nonEmpty) {
         log.error(s"Found ${analysis.unreachable.size} unreachable symbols!")
         analysis.unreachable.foreach {
-          case Reach.UnreachableSymbol(_, kind, name, backtrace) =>
+          case Reach.UnreachableSymbol(_, symbol, backtrace) =>
             val buf = new StringBuilder()
-            buf.append(s"Found unknown $kind $name, referenced from:\n")
+            buf.append(s"Unknown $symbol, referenced from:\n")
             appendBackTrace(buf, backtrace)
             log.error(buf.toString())
         }
diff --git a/tools/src/main/scala/scala/scalanative/linker/Reach.scala b/tools/src/main/scala/scala/scalanative/linker/Reach.scala
@@ -917,44 +917,113 @@ class Reach(
         unsupported.getOrElseUpdate(global, details)
       case _ =>
         unreachable.getOrElseUpdate(
-          global, {
-            val (kind, symbol) = parseSymbol(global)
-            UnreachableSymbol(
-              name = global,
-              kind = kind,
-              symbol = symbol,
-              backtrace = getBackTrace(global)
-            )
-          }
+          global,
+          UnreachableSymbol(
+            name = global,
+            symbol = parseSymbol(global),
+            backtrace = getBackTrace(global)
+          )
         )
     }
 
-  private def parseSymbol(name: Global): (String, String) = {
-    def parseSig(owner: String, sig: Sig): (String, String) =
+  private def parseSymbol(name: Global): SymbolDescriptor = {
+    def renderType(tpe: nir.Type): String = tpe match {
+      case arr: Type.Array   => s"${renderType(arr.ty)}[]"
+      case ref: Type.RefKind => ref.className.id
+      case ty                => ty.show
+    }
+    def parseArgTypes(
+        types: Seq[nir.Type],
+        isCtor: Boolean = false
+    ): Some[Seq[String]] = Some {
+      val args = types match {
+        case _ if isCtor   => types
+        case args :+ retty => args
+        case _             => Nil
+      }
+      args.map(renderType)
+    }
+
+    val Private = "private"
+    val Static = "static"
+
+    def parseResultType(types: Seq[nir.Type]): Option[String] =
+      types.lastOption.map(renderType)
+
+    def parseModifiers(scope: nir.Sig.Scope): List[String] = scope match {
+      case Sig.Scope.Public           => Nil
+      case Sig.Scope.Private(_)       => List(Private)
+      case Sig.Scope.PublicStatic     => List(Static)
+      case Sig.Scope.PrivateStatic(_) => List(Static, Private)
+    }
+
+    def parseSig(owner: String, sig: Sig): SymbolDescriptor =
       sig.unmangled match {
-        case Sig.Method(name, _, _) => "method" -> s"$owner.${name}"
-        case Sig.Ctor(tys) =>
-          val ctorTys = tys
-            .map {
-              case ty: Type.RefKind => ty.className.id
-              case ty               => ty.show
-            }
-            .mkString(",")
-          "constructor" -> s"$owner($ctorTys)"
-        case Sig.Clinit         => "static constructor" -> owner
-        case Sig.Field(name, _) => "field" -> s"$owner.$name"
+        case Sig.Method(name, types, scope) =>
+          SymbolDescriptor(
+            "method",
+            s"$owner.$name",
+            parseArgTypes(types),
+            parseResultType(types),
+            parseModifiers(scope)
+          )
+        case Sig.Ctor(types) =>
+          SymbolDescriptor(
+            "constructor",
+            owner,
+            parseArgTypes(types, isCtor = true)
+          )
+        case Sig.Clinit =>
+          SymbolDescriptor(
+            "constructor",
+            owner,
+            modifiers = List(Static)
+          )
+        case Sig.Field(name, scope) =>
+          SymbolDescriptor(
+            "field",
+            owner,
+            modifiers = parseModifiers(scope)
+          )
         case Sig.Generated(name) =>
-          "generated method" -> s"$owner.${name}"
-        case Sig.Proxy(name, _) => "proxy method" -> s"$owner.$name"
-        case Sig.Duplicate(sig, _) =>
-          val (kind, name) = parseSig(owner, sig)
-          s"duplicate $kind" -> s"$owner.name"
-        case Sig.Extern(name) => s"extern method" -> s"$owner.$name"
+          SymbolDescriptor(
+            "symbol",
+            s"$owner.$name",
+            modifiers = List("generated")
+          )
+        case Sig.Proxy(name, types) =>
+          SymbolDescriptor(
+            "method",
+            s"$owner.$name",
+            parseArgTypes(types),
+            parseResultType(types),
+            modifiers = List("proxy")
+          )
+        case Sig.Duplicate(sig, types) =>
+          val original = parseSig(owner, sig)
+          original.copy(
+            argTypes = parseArgTypes(types),
+            resultType = parseResultType(types),
+            modifiers = List("duplicate") ++ original.modifiers
+          )
+          SymbolDescriptor(
+            "method",
+            s"$owner.$name",
+            parseArgTypes(types),
+            parseResultType(types),
+            modifiers = List("duplicate")
+          )
+        case Sig.Extern(name) =>
+          SymbolDescriptor(
+            "symbol",
+            s"$owner.$name",
+            modifiers = List("extern")
+          )
       }
 
     name match {
       case Global.Member(owner, sig) => parseSig(owner.id, sig)
-      case Global.Top(id)            => "type" -> id
+      case Global.Top(id)            => SymbolDescriptor("type", id)
       case _                         => util.unreachable
     }
   }
@@ -969,11 +1038,9 @@ class Reach(
       else {
         val file = current.srcPosition.filename.getOrElse("unknown")
         val line = current.srcPosition.line
-        val (kind, symbol) = parseSymbol(current.referencedBy)
         buf += BackTraceElement(
           name = current.referencedBy,
-          kind = kind,
-          symbol = symbol,
+          symbol = parseSymbol(current.referencedBy),
           filename = file,
           line = line + 1
         )
@@ -1087,17 +1154,30 @@ object Reach {
   object ReferencedFrom {
     final val Root = ReferencedFrom(nir.Global.None, nir.Position.NoPosition)
   }
+  case class SymbolDescriptor(
+      kind: String,
+      name: String,
+      argTypes: Option[Seq[String]] = None,
+      resultType: Option[String] = None,
+      modifiers: Seq[String] = Nil
+  ) {
+    override def toString(): String = {
+      val mods =
+        if (modifiers.isEmpty) "" else modifiers.distinct.mkString("", " ", " ")
+      val argsList = argTypes.fold("")(_.mkString("(", ", ", ")"))
+      val resType = resultType.fold("")(tpe => s": $tpe")
+      s"$mods$kind $name$argsList$resType"
+    }
+  }
   case class BackTraceElement(
       name: Global,
-      kind: String,
-      symbol: String,
+      symbol: SymbolDescriptor,
       filename: String,
       line: Int
   )
   case class UnreachableSymbol(
       name: Global,
-      kind: String,
-      symbol: String,
+      symbol: SymbolDescriptor,
       backtrace: List[BackTraceElement]
   )
 
diff --git a/tools/src/test/scala/scala/scalanative/linker/MissingSymbolsTest.scala b/tools/src/test/scala/scala/scalanative/linker/MissingSymbolsTest.scala
@@ -45,11 +45,13 @@ class MissingSymbolsTest extends LinkerSpec {
         assertContainsAll(
           "kind-symbols",
           Seq(
-            "type" -> "java.sql.Time",
-            "constructor" -> "java.sql.Time(long)",
-            "method" -> "java.sql.Time.valueOf"
+            ("type", "java.sql.Time", None),
+            ("constructor", "java.sql.Time", Some(Seq("long"))),
+            ("method", "java.sql.Time.valueOf", Some(Seq("java.lang.String")))
           ),
-          result.unreachable.map(v => (v.kind, v.symbol))
+          result.unreachable
+            .map(_.symbol)
+            .map(v => (v.kind, v.name, v.argTypes))
         )
         val TimeType = Global.Top("java.sql.Time")
         val TimeCtor = TimeType.member(Sig.Ctor(Seq(Type.Long)))
@@ -68,7 +70,9 @@ class MissingSymbolsTest extends LinkerSpec {
 
         result.unreachable.foreach { symbol =>
           val backtrace =
-            symbol.backtrace.map(v => (v.kind, v.symbol, v.filename, v.line))
+            symbol.backtrace.map(v =>
+              (v.symbol.kind, v.symbol.name, v.filename, v.line)
+            )
           // format: off
           assertEquals("backtrace", List(
             ("method", "Bar$.getTimeString", sourceFile, if(symbol.name == TimeCtor) 9 else 8),
@@ -94,23 +98,22 @@ class MissingSymbolsTest extends LinkerSpec {
         """.stripMargin)
     ) {
       case (config, result) =>
-        scala.scalanative.build.ScalaNative.logLinked(config, result, "test")
         assertEquals("unreachable", 2, result.unreachable.size)
         assertContainsAll(
           "kind-symbols",
           Seq(
             "type" -> "java.sql.Time",
-            "constructor" -> "java.sql.Time(long)"
+            "constructor" -> "java.sql.Time"
           ),
-          result.unreachable.map(v => (v.kind, v.symbol))
+          result.unreachable.map(_.symbol).map(v => (v.kind, v.name))
         )
 
         result.unreachable
-          .find(_.symbol == "java.sql.Time")
+          .find(_.symbol.name == "java.sql.Time")
           .map { symbol =>
             val from = symbol.backtrace.head
-            assertEquals("type", from.kind)
-            assertEquals("Test$Foo$1", from.symbol)
+            assertEquals("type", from.symbol.kind)
+            assertEquals("Test$Foo$1", from.symbol.name)
           }
           .getOrElse(fail("Not found required unreachable symbol"))
     }
@@ -134,7 +137,7 @@ class MissingSymbolsTest extends LinkerSpec {
         // Testing if is able to get non-empty backtrace.
         // If reference tacking of delayed methods is invalid we would get empty list here
         result.unreachable
-          .find(_.symbol == "java.lang.Class.getDeclaredFields")
+          .find(_.symbol.name == "java.lang.Class.getDeclaredFields")
           .map { symbol =>
             assertTrue("no-backtrace", symbol.backtrace.nonEmpty)
           }
@@ -170,7 +173,6 @@ class MissingSymbolsTest extends LinkerSpec {
                 Reach.UnsupportedFeature.SystemThreads,
                 kind
               )
-              println(backtrace)
               assertTrue("no-backtrace", backtrace.nonEmpty)
           }
           .getOrElse(fail("Not found required unreachable symbol"))
