fix exception when using json output flag

[smx-smx]

fixes the following exception, which breaks usage with ksdump:

Exception in thread "main" scala.MatchError: ArrayBuffer(...)

[generalmimon]

This is really funny to me, because I fixed the exact same error before releasing 0.10 😄: 74759af

But you're right that it's now back with the latest compiler. I guess that in Scala 2.13 that the compiler got migrated to in #266 (cc @GreyCat), ArrayBuffer no longer inherits from Seq like it did in Scala 2.12.

[generalmimon]

Thanks for reporting the bug and offering a fix, but I'd rather use a general trait that all list-like collections that can occur here implement (and such a trait will also guarantee that the object actually supports the basic functionality that we need to actually dump it as an array to JSON), instead of listing each individual list type we could encounter.

Seq was such a trait in Scala 2.12 (at least for our purposes), but something similar must exist in Scala 2.13 too.

[generalmimon]

I guess that in Scala 2.13 (...), ArrayBuffer no longer inherits from Seq like it did in Scala 2.12.

Just as I thought - see Migrating a Project to Scala 2.13's Collections | Scala Documentation:

The most important changes in the Scala 2.13 collections library are:

• scala.Seq[+A] is now an alias for scala.collection.immutable.Seq[A] (instead of scala.collection.Seq[A]). Note that this also changes the type of Scala varargs methods.

Since mutable.ArrayBuffer is obviously not immutable, it's no longer a subclass of Seq.

---

So I guess the most straightforward solution would be Option 1: migrate back to scala.collection.Seq, which should restore the old behavior. The linked migration guide goes into detail how this should be done:

We recommend using import scala.collection/import scala.collection.immutable and collection.Seq/immutable.Seq.

We recommend against using import scala.collection.Seq, which shadows the automatically imported scala.Seq, because even if it’s a one-line change it causes name confusion. For code generation or macros the safest option is using the fully-qualified _root_.scala.collection.Seq.

In our case, that would be:

diff --git i/shared/src/main/scala/io/kaitai/struct/JSON.scala w/shared/src/main/scala/io/kaitai/struct/JSON.scala
index d38a0634..b86b3a25 100644
--- i/shared/src/main/scala/io/kaitai/struct/JSON.scala
+++ w/shared/src/main/scala/io/kaitai/struct/JSON.scala
@@ -1,6 +1,7 @@
 package io.kaitai.struct

 import io.kaitai.struct.translators.CommonLiterals
+import scala.collection

 /** Common trait for all objects that can be serialized as JSON. */
 trait Jsonable {
@@ -22,7 +23,7 @@ object JSON extends CommonLiterals {
       case v: Jsonable => v.toJson
       case v: Int => v.toString
       case v: String => stringToJson(v)
-      case v: Seq[_] => seqToJson(v)
+      case v: collection.Seq[_] => seqToJson(v)
       case v: Map[String, _] => mapToJson(v)
     }
   }
@@ -33,7 +34,7 @@ object JSON extends CommonLiterals {
   def stringToJson(str: String): String =
     doStringLiteral(str)

-  def seqToJson(obj: Seq[_]): String =
+  def seqToJson(obj: collection.Seq[_]): String =
     "[" + obj.map((x) => stringify(x)).mkString(",") + "]"

   def mapToJson(obj: Map[String, Any]): String = {

[GreyCat]

Thanks for your research and suggestions, @generalmimon and @smx-smx!

I believe generally approach suggested by @generalmimon makes perfect sense. My only suggestion would be actually that we need to have a test somehow checking that JSON output works at all.

[generalmimon]

My only suggestion would be actually that we need to have a test somehow checking that JSON output works at all.

Totally agree. Do you mean to add some unit tests to jvm/src/test/scala/io/kaitai/struct (https://github.com/kaitai-io/kaitai_struct_compiler/tree/master/jvm/src/test/scala/io/kaitai/struct)?

[GreyCat]

Totally agree. Do you mean to add some unit tests to jvm/src/test/scala/io/kaitai/struct (https://github.com/kaitai-io/kaitai_struct_compiler/tree/master/jvm/src/test/scala/io/kaitai/struct)?

Yep, something like that. Maybe literally running compiler end-to-end with various options just to make sure we allow various permutations of inputs and outputs.

[generalmimon]

Maybe literally running compiler end-to-end with various options just to make sure we allow various permutations of inputs and outputs.

Yeah, I was thinking about that as well, but I'm not sure how to integrate that into our infrastructure (maybe a new repo?). Right now, we're basically only testing one specific end-to-end invocation in build-formats, which is far from covering all possible settings.

[GreyCat]

We don't really have to test all possible permutations of command line arguments × all possible .ksy files. This specific problem could be identified and tested by a simple unit test: one set of compilation problems in, one JSON output formatting expected.