Fix --string non-string error reporting

sjsonnet/src-jvm-native/sjsonnet/SjsonnetMainBase.scala

@@ -258,12 +258,32 @@ object SjsonnetMainBase {
     else if (config.expectString.value)
       new SimpleVisitor[Writer, Writer] {
         val expectedMsg = "expected string result"
+        private def rejectType(tpe: String): Nothing =
+          throw new Error(s"$expectedMsg, got: $tpe")
         override def visitString(s: CharSequence, index: Int): Writer = {
           wr.write(s.toString)
           wr
         }
-        override def visitNull(index: Int): Writer =
-          throw new upickle.core.Abort(s"$expectedMsg got null")
+        override def visitNull(index: Int): Writer = rejectType("null")
+        override def visitFalse(index: Int): Writer = rejectType("boolean")
+        override def visitTrue(index: Int): Writer = rejectType("boolean")
+        override def visitFloat64(d: Double, index: Int): Writer = rejectType("number")
+        override def visitFloat32(d: Float, index: Int): Writer = rejectType("number")
+        override def visitInt32(i: Int, index: Int): Writer = rejectType("number")
+        override def visitInt64(i: Long, index: Int): Writer = rejectType("number")
+        override def visitUInt64(i: Long, index: Int): Writer = rejectType("number")
+        override def visitFloat64StringParts(
+            s: CharSequence,
+            decIndex: Int,
+            expIndex: Int,
+            index: Int): Writer = rejectType("number")
+        override def visitFloat64String(s: String, index: Int): Writer = rejectType("number")
+        override def visitArray(length: Int, index: Int): upickle.core.ArrVisitor[Writer, Writer] =
+          rejectType("array")
+        override def visitObject(
+            length: Int,
+            jsonableKeys: Boolean,
+            index: Int): upickle.core.ObjVisitor[Writer, Writer] = rejectType("object")
       }
     else new Renderer(wr, indent = config.indent)
 
@@ -273,6 +293,13 @@ object SjsonnetMainBase {
       case e                      => e.toString
     }
 
+  private def handleRenderError[T](f: => T): Either[String, T] =
+    try Right(f)
+    catch {
+      case e: Error if e.stack.isEmpty => Left(s"sjsonnet.Error: ${e.getMessage}")
+      case e: Error                    => Left(Error.formatError(e))
+    }
+
   private def writeFile(
       config: Config,
       f: os.Path,
@@ -483,8 +510,10 @@ object SjsonnetMainBase {
                   rendered <- {
                     val writer = new StringWriter()
                     val renderer = rendererForConfig(writer, config, () => currentPos)
-                    ujson.transform(v, renderer)
-                    Right(writer.toString)
+                    handleRenderError {
+                      ujson.transform(v, renderer)
+                      writer.toString
+                    }
                   }
                   relPath = (os.FilePath(multiPath) / os.RelPath(f)).asInstanceOf[os.FilePath]
                   _ <- writeFile(config, relPath.resolveFrom(wd), rendered, trailingNewline)
@@ -512,27 +541,29 @@ object SjsonnetMainBase {
         interp.interpret(jsonnetCode, path).flatMap {
           case arr: ujson.Arr =>
             writeToFile(config, wd) { writer =>
-              arr.value.toSeq match {
-                case Nil         => // donothing
-                case Seq(single) =>
-                  val renderer = rendererForConfig(writer, config, () => currentPos)
-                  single.transform(renderer)
-                  writer.write(if (isScalar(single)) "\n..." else "")
-                case multiple =>
-                  for ((v, i) <- multiple.zipWithIndex) {
-                    if (i > 0) writer.write('\n')
-                    if (isScalar(v)) writer.write("--- ")
-                    else if (i != 0) writer.write("---\n")
-                    val renderer = rendererForConfig(
-                      writer,
-                      config.copy(yamlOut = mainargs.Flag(true)),
-                      () => currentPos
-                    )
-                    v.transform(renderer)
-                  }
+              handleRenderError {
+                arr.value.toSeq match {
+                  case Nil         => // donothing
+                  case Seq(single) =>
+                    val renderer = rendererForConfig(writer, config, () => currentPos)
+                    single.transform(renderer)
+                    writer.write(if (isScalar(single)) "\n..." else "")
+                  case multiple =>
+                    for ((v, i) <- multiple.zipWithIndex) {
+                      if (i > 0) writer.write('\n')
+                      if (isScalar(v)) writer.write("--- ")
+                      else if (i != 0) writer.write("---\n")
+                      val renderer = rendererForConfig(
+                        writer,
+                        config.copy(yamlOut = mainargs.Flag(true)),
+                        () => currentPos
+                      )
+                      v.transform(renderer)
+                    }
+                }
+                writer.write('\n')
+                ""
               }
-              writer.write('\n')
-              Right("")
             }
 
           case _ =>

sjsonnet/test/resources/db/cli_string_non_string.jsonnet

@@ -0,0 +1 @@
+{a: 1}

sjsonnet/test/resources/db/cli_string_non_string.jsonnet.golden

@@ -0,0 +1,2 @@
+sjsonnet.Error: expected string result, got: object
+    at [<root>].(sjsonnet/test/resources/db/cli_string_non_string.jsonnet:1:1)

sjsonnet/test/src-jvm/sjsonnet/MainTests.scala

@@ -54,6 +54,21 @@ object MainTests extends TestSuite {
       assert(err1.nonEmpty, err3.nonEmpty)
     }
 
+    test("stringModeNonStringReportsCleanError") {
+      val sourceRel =
+        os.RelPath("sjsonnet") / "test" / "resources" / "db" /
+          "cli_string_non_string.jsonnet"
+      val source = workspaceRoot / sourceRel
+      val golden = os.read(os.Path(source.toString + ".golden"))
+      val sourceArg: os.Shellable = sourceRel
+      val (res, out, err) = runMain("--string", sourceArg)
+      assert(res == 1)
+      assert(out.isEmpty)
+      assert(normalizeOutput(err) == normalizeOutput(golden))
+      assert(!err.contains("Internal error"))
+      assert(!err.contains("upickle.core.Abort"))
+    }
+
     val streamedOut =
       """--- 1
         |--- 2
@@ -171,25 +186,29 @@ object MainTests extends TestSuite {
       val (res, out, err) = runMain("null", "--exec", "--string")
       assert(res == 1)
       assert(out.isEmpty)
-      assert(err.contains("expected string result got null"))
+      assertStringModeTypeError(err, "null")
     }
 
     test("execStringRejectsNonStringTypes") {
       // Numbers
       val (res1, out1, err1) = runMain("42", "--exec", "--string")
-      assert(res1 == 1, out1.isEmpty, err1.contains("expected string result"))
+      assert(res1 == 1, out1.isEmpty)
+      assertStringModeTypeError(err1, "number")
 
       // Booleans
       val (res2, out2, err2) = runMain("true", "--exec", "--string")
-      assert(res2 == 1, out2.isEmpty, err2.contains("expected string result"))
+      assert(res2 == 1, out2.isEmpty)
+      assertStringModeTypeError(err2, "boolean")
 
       // Arrays
       val (res3, out3, err3) = runMain("[1, 2]", "--exec", "--string")
-      assert(res3 == 1, out3.isEmpty, err3.contains("expected string result"))
+      assert(res3 == 1, out3.isEmpty)
+      assertStringModeTypeError(err3, "array")
 
       // Objects
       val (res4, out4, err4) = runMain("{a: 1}", "--exec", "--string")
-      assert(res4 == 1, out4.isEmpty, err4.contains("expected string result"))
+      assert(res4 == 1, out4.isEmpty)
+      assertStringModeTypeError(err4, "object")
     }
 
     test("multiStringOutput") {
@@ -206,6 +225,22 @@ object MainTests extends TestSuite {
       assert(barDestStr == "bar\n")
     }
 
+    test("multiStringRejectsNonStringValues") {
+      val multiDest = os.temp.dir()
+      val (res, out, err) =
+        runMain("""{"bad.txt": 42}""", "--exec", "--multi", multiDest, "--string")
+      assert(res == 1)
+      assert(out.isEmpty)
+      assertStringModeTypeError(err, "number")
+    }
+
+    test("yamlStreamStringRejectsSingleNonStringValue") {
+      val (res, out, err) = runMain("[42]", "--exec", "--yaml-stream", "--string")
+      assert(res == 1)
+      assert(out.isEmpty)
+      assertStringModeTypeError(err, "number")
+    }
+
     // -- No trailing newline behavior --
 
     test("noTrailingNewline") {
@@ -488,6 +523,17 @@ object MainTests extends TestSuite {
   def runMain(args: os.Shellable*): (Int, String, String) =
     runMainWithEnv(jsonnetPathEnv = "", args*)
 
+  private def normalizeOutput(s: String): String =
+    s.replace("\r\n", "\n").replaceAll("\n+\\z", "")
+
+  private def assertStringModeTypeError(err: String, tpe: String): Unit = {
+    assert(err.contains(s"expected string result, got: $tpe"))
+    assert(!err.contains("Internal error"))
+    assert(!err.contains("upickle.core.Abort"))
+    assert(!err.contains("\tat "))
+    assert(!err.contains("SjsonnetMainBase"))
+  }
+
   def runMainWithEnv(jsonnetPathEnv: String, args: os.Shellable*): (Int, String, String) = {
     val err = new ByteArrayOutputStream()
     val perr = new PrintStream(err, true, "UTF-8")