Refactor stack traces: named frames, root frame, and builtin collapsing

CLAUDE.md

@@ -25,6 +25,8 @@ The assembly JAR is at `out/sjsonnet/jvm/3.3.7/assembly.dest/out.jar`. Run it wi
 
 ## Formatting
 
+We follow the Databricks [Scala style guide](https://raw.githubusercontent.com/databricks/scala-style-guide/refs/heads/master/README.md).
+
 Scala sources are formatted with [scalafmt](https://scalameta.org/scalafmt/) (config in `.scalafmt.conf`). The `SjsonnetCrossModule` and `bench` modules mix in `ScalafmtModule`.
 
 ```bash

sjsonnet/src/sjsonnet/Error.scala

@@ -7,30 +7,41 @@ import scala.util.control.NonFatal
  * An exception that can keep track of the Sjsonnet call-stack while it is propagating upwards. This
  * helps provide good error messages with line numbers pointing towards user code.
  */
-class Error(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
+class Error(msg: String, val stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
     extends Exception(msg, underlying.orNull) {
 
   setStackTrace(stack.reverseIterator.map(_.ste).toArray)
 
   override def fillInStackTrace: Throwable = this
 
   def addFrame(pos: Position, expr: Expr = null)(implicit ev: EvalErrorScope): Error = {
-    if (stack.isEmpty || alwaysAddPos(expr)) {
-      val exprErrorString = if (expr == null) null else expr.exprErrorString
-      addFrameString(pos, exprErrorString)
+    if (stack.isEmpty) {
+      val name = if (expr != null && isApplyOrBuiltin(expr)) expr.exprErrorString else null
+      if (name != null)
+        copy(stack = new Error.Frame(pos, name, collapseIntoMessage = true) :: Nil)
+      else
+        addFrameString(pos, name)
+    } else if (expr != null && isApplyOrBuiltin(expr)) {
+      addFrameString(pos, expr.exprErrorString)
     } else this
   }
 
   def addFrameString(pos: Position, exprErrorString: String)(implicit ev: EvalErrorScope): Error = {
-    val newFrame = new Error.Frame(pos, exprErrorString)
     stack match {
+      case s :: ss if s.exprErrorString == null && exprErrorString != null =>
+        val collapse = s.pos == pos
+        copy(stack = new Error.Frame(s.pos, exprErrorString, collapse) :: ss)
       case s :: ss if s.pos == pos =>
-        if (s.exprErrorString == null && exprErrorString != null) copy(stack = newFrame :: ss)
+        if (s.exprErrorString == null && exprErrorString != null)
+          copy(stack = new Error.Frame(pos, exprErrorString, collapseIntoMessage = true) :: ss)
         else this
-      case _ => copy(stack = newFrame :: stack)
+      case _ => copy(stack = new Error.Frame(pos, exprErrorString) :: stack)
     }
   }
 
+  def forceAddFrame(pos: Position, name: String)(implicit ev: EvalErrorScope): Error =
+    copy(stack = new Error.Frame(pos, name) :: stack)
+
   def asSeenFrom(ev: EvalErrorScope): Error =
     copy(stack = stack.map(_.asSeenFrom(ev)))
 
@@ -40,15 +51,20 @@ class Error(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Thro
       underlying: Option[Throwable] = underlying) =
     new Error(msg, stack, underlying)
 
-  private def alwaysAddPos(expr: Expr): Boolean = expr match {
-    case _: Expr.LocalExpr | _: Expr.Arr | _: Expr.ObjExtend | _: Expr.ObjBody | _: Expr.IfElse =>
-      false
-    case _ => true
+  private def isApplyOrBuiltin(expr: Expr): Boolean = expr match {
+    case _: Expr.Apply | _: Expr.Apply0 | _: Expr.Apply1 | _: Expr.Apply2 | _: Expr.Apply3 |
+        _: Expr.ApplyBuiltin | _: Expr.ApplyBuiltin0 | _: Expr.ApplyBuiltin1 |
+        _: Expr.ApplyBuiltin2 | _: Expr.ApplyBuiltin3 | _: Expr.ApplyBuiltin4 =>
+      true
+    case _ => false
   }
 }
 
 object Error {
-  final class Frame(val pos: Position, val exprErrorString: String)(implicit ev: EvalErrorScope) {
+  final class Frame(
+      val pos: Position,
+      val exprErrorString: String,
+      val collapseIntoMessage: Boolean = false)(implicit ev: EvalErrorScope) {
     val ste: StackTraceElement = {
       val cl = if (exprErrorString == null) "" else s"[$exprErrorString]"
       val (frameFile, frameLine) = ev.prettyIndex(pos) match {
@@ -59,7 +75,8 @@ object Error {
     }
 
     def asSeenFrom(ev: EvalErrorScope): Frame =
-      if (ev eq this.ev) this else new Frame(pos, exprErrorString)(ev)
+      if (ev eq this.ev) this
+      else new Frame(pos, exprErrorString, collapseIntoMessage)(ev)
   }
 
   def withStackFrame[T](expr: Expr)(implicit
@@ -70,29 +87,97 @@ object Error {
   }
 
   def fail(msg: String, expr: Expr)(implicit ev: EvalErrorScope): Nothing =
-    fail(msg, expr.pos, expr.exprErrorString)
+    fail(msg, expr.pos)
 
   def fail(msg: String, pos: Position, cl: String = null)(implicit ev: EvalErrorScope): Nothing =
     throw new Error(msg, new Frame(pos, cl) :: Nil, None)
 
   def fail(msg: String): Nothing =
     throw new Error(msg)
 
-  def formatError(e: Throwable): String = {
-    val s = new StringWriter()
-    val p = new PrintWriter(s)
-    try {
-      e.printStackTrace(p)
-      s.toString.replace("\t", "    ")
-    } finally {
-      p.close()
+  private val rootName = Util.wrapInLessThanGreaterThan("root")
+
+  def formatError(e: Throwable): String = e match {
+    case err: Error if err.stack.nonEmpty =>
+      val sb = new StringBuilder
+      sb.append(err.getClass.getName).append(": ")
+
+      val allFrames = err.stack.reverse
+      val namedFrames = allFrames.filter(_.exprErrorString != null)
+      val frames = if (namedFrames.nonEmpty) namedFrames else allFrames
+
+      var msg = err.getMessage
+      var frameStart = 0
+      while (
+        frameStart < frames.length - 1 &&
+        (frames(frameStart).collapseIntoMessage ||
+        frames(frameStart).pos == frames(frameStart + 1).pos)
+      ) {
+        val name = Option(frames(frameStart).exprErrorString).getOrElse(rootName)
+        msg = "[" + name + "] " + msg
+        frameStart += 1
+      }
+
+      if (frameStart < frames.length && frames(frameStart).collapseIntoMessage) {
+        val f = frames(frameStart)
+        val name = Option(f.exprErrorString).getOrElse(rootName)
+        msg = "[" + name + "] " + msg
+        sb.append(msg)
+        appendFrame(sb, f, rootName)
+      } else {
+        sb.append(msg)
+        var i = frameStart
+        while (i < frames.length) {
+          val f = frames(i)
+          val name =
+            if (i == frames.length - 1) {
+              val n = f.exprErrorString
+              if (n == null || n == rootName) rootName else n
+            } else
+              Option(f.exprErrorString).getOrElse(rootName)
+          appendFrame(sb, f, name)
+          i += 1
+        }
+      }
+      if (err.getCause != null) {
+        sb.append("\nCaused by: ")
+        val s = new StringWriter()
+        val p = new PrintWriter(s)
+        try {
+          err.getCause.printStackTrace(p)
+          sb.append(s.toString.replace("\t", "    "))
+        } finally {
+          p.close()
+        }
+      } else {
+        sb.append('\n')
+      }
+      sb.toString
+
+    case _ =>
+      val s = new StringWriter()
+      val p = new PrintWriter(s)
+      try {
+        e.printStackTrace(p)
+        s.toString.replace("\t", "    ")
+      } finally {
+        p.close()
+      }
+  }
+
+  private def appendFrame(sb: StringBuilder, f: Frame, name: String): Unit = {
+    sb.append("\n    at [").append(name).append("]")
+    if (f.pos != null) {
+      sb.append(".(")
+      sb.append(f.ste.getFileName).append(':').append(f.ste.getLineNumber)
+      sb.append(')')
     }
   }
 }
 
 class ParseError(
     msg: String,
-    stack: List[Error.Frame] = Nil,
+    override val stack: List[Error.Frame] = Nil,
     underlying: Option[Throwable] = None,
     val offset: Int = -1)
     extends Error(msg, stack, underlying) {
@@ -104,7 +189,10 @@ class ParseError(
     new ParseError(msg, stack, underlying, offset)
 }
 
-class StaticError(msg: String, stack: List[Error.Frame] = Nil, underlying: Option[Throwable] = None)
+class StaticError(
+    msg: String,
+    override val stack: List[Error.Frame] = Nil,
+    underlying: Option[Throwable] = None)
     extends Error(msg, stack, underlying) {
 
   override protected def copy(
@@ -116,7 +204,7 @@ class StaticError(msg: String, stack: List[Error.Frame] = Nil, underlying: Optio
 
 object StaticError {
   def fail(msg: String, expr: Expr)(implicit ev: EvalErrorScope): Nothing =
-    throw new StaticError(msg, new Error.Frame(expr.pos, expr.exprErrorString) :: Nil, None)
+    throw new StaticError(msg, new Error.Frame(expr.pos, null) :: Nil, None)
 }
 
 trait EvalErrorScope {

sjsonnet/src/sjsonnet/Evaluator.scala

@@ -826,12 +826,11 @@ class Evaluator(
         val a = asserts(i)
         if (!visitExpr(a.value)(newScope).isInstanceOf[Val.True]) {
           a.msg match {
-            case null => Error.fail("Assertion failed", a.value.pos, "Assert")
+            case null => Error.fail("Assertion failed", a.value.pos)
             case msg  =>
               Error.fail(
                 "Assertion failed: " + visitExpr(msg)(newScope).cast[Val.Str].str,
-                a.value.pos,
-                "Assert"
+                a.value.pos
               )
           }
         }

sjsonnet/src/sjsonnet/Expr.scala

@@ -45,6 +45,17 @@ trait TailstrictableExpr extends Expr {
 }
 
 object Expr {
+  private[sjsonnet] def callTargetName(value: Expr): String = value match {
+    case ValidId(_, name, _) => name
+    case Id(_, name)         => name
+    case Select(_, _, name)  => name
+    case f: Val.Builtin      => f.qualifiedName
+    case f: Val.Func         =>
+      val n = f.functionName
+      if (n != null) n else "anonymous"
+    case _ => "anonymous"
+  }
+
   private final def arrStr(a: Array[?]): String = {
     if (a == null) "null" else a.mkString("[", ", ", "]")
   }
@@ -212,18 +223,22 @@ object Expr {
       tailstrict: Boolean)
       extends TailstrictableExpr {
     final override private[sjsonnet] def tag = ExprTags.Apply
+    override def exprErrorString: String = Expr.callTargetName(value)
   }
   final case class Apply0(pos: Position, value: Expr, tailstrict: Boolean)
       extends TailstrictableExpr {
     final override private[sjsonnet] def tag = ExprTags.Apply0
+    override def exprErrorString: String = Expr.callTargetName(value)
   }
   final case class Apply1(pos: Position, value: Expr, a1: Expr, tailstrict: Boolean)
       extends TailstrictableExpr {
     final override private[sjsonnet] def tag = ExprTags.Apply1
+    override def exprErrorString: String = Expr.callTargetName(value)
   }
   final case class Apply2(pos: Position, value: Expr, a1: Expr, a2: Expr, tailstrict: Boolean)
       extends TailstrictableExpr {
     final override private[sjsonnet] def tag = ExprTags.Apply2
+    override def exprErrorString: String = Expr.callTargetName(value)
   }
   final case class Apply3(
       pos: Position,
@@ -234,6 +249,7 @@ object Expr {
       tailstrict: Boolean)
       extends TailstrictableExpr {
     final override private[sjsonnet] def tag = ExprTags.Apply3
+    override def exprErrorString: String = Expr.callTargetName(value)
   }
   final case class ApplyBuiltin(
       pos: Position,

sjsonnet/src/sjsonnet/Interpreter.scala

@@ -180,7 +180,7 @@ class Interpreter(
     (for {
       v <- evaluate(txt, path)
       r <- materialize(v, visitor)
-    } yield r).left.map(Error.formatError)
+    } yield r).left.map(e => Error.formatError(ensureRootFrame(e)))
 
   private def handleException[T](f: => T): Either[Error, T] = {
     try Right(f)
@@ -198,11 +198,41 @@ class Interpreter(
     result
   }
 
+  private val rootName = Util.wrapInLessThanGreaterThan("root")
+
+  @volatile private var lastTopLevelPos: Position = _
+
+  private def addRootFrame(e: Error, expr: Expr): Error = {
+    implicit val ev: EvalErrorScope = evaluator
+    e.forceAddFrame(expr.pos, rootName)
+  }
+
+  private def ensureRootFrame(e: Error): Error = {
+    if (e.stack.exists(_.exprErrorString == rootName)) e
+    else {
+      implicit val ev: EvalErrorScope = evaluator
+      val pos = lastTopLevelPos
+      if (pos == null) e
+      else e.forceAddFrame(pos, rootName)
+    }
+  }
+
   private def evaluateImpl(txt: String, path: Path): Either[Error, Val] = {
     val resolvedImport = StaticResolvedFile(txt)
     resolver.cache(path) = resolvedImport
     resolver.parse(path, resolvedImport)(evaluator) flatMap { case (expr, _) =>
-      handleException(evaluator.visitExpr(expr)(ValScope.empty)) flatMap {
+      lastTopLevelPos = expr.pos
+      handleException {
+        try evaluator.visitExpr(expr)(ValScope.empty)
+        catch {
+          case e: Error    => throw addRootFrame(e, expr)
+          case NonFatal(e) =>
+            throw addRootFrame(
+              new Error("Internal Error", Nil, Some(e)),
+              expr
+            )
+        }
+      } flatMap {
         case f: Val.Func =>
           val defaults2 = f.params.defaultExprs.clone()
           val tlaExpressions = collection.mutable.Set.empty[Expr]
@@ -226,7 +256,23 @@ class Interpreter(
               )
             }
           }
-          handleException(out.apply0(f.pos)(evaluator, TailstrictModeDisabled))
+          handleException {
+            implicit val ev: EvalErrorScope = evaluator
+            val funcName = {
+              val n = f.functionName
+              if (n != null) n else "anonymous"
+            }
+            try out.apply0(f.pos)(evaluator, TailstrictModeDisabled)
+            catch {
+              case e: Error =>
+                throw addRootFrame(e.addFrameString(f.pos, funcName), expr)
+              case NonFatal(e) =>
+                throw addRootFrame(
+                  new Error("Internal Error", Nil, Some(e)).addFrameString(f.pos, funcName),
+                  expr
+                )
+            }
+          }
         case x => Right(x)
       }
     }