Stabilized generation order of everything related to maps by switchin…

…g from Map to SortedMap where applicable, namely: * Subtypes (ordered by type name) * Instances (ordered by instance human readable name) * Enums (ordered by numberical value) * Switch cases within attribute's switch type (ordered by expr toString rendition)
kaitai-io · Feb 29, 2024 · 5f561e1 · 5f561e1
1 parent 66ad703
commit 5f561e1
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 63 deletions.
diff --git a/jvm/src/test/scala/io/kaitai/struct/datatype/SwitchType$Test.scala b/jvm/src/test/scala/io/kaitai/struct/datatype/SwitchType$Test.scala
@@ -6,12 +6,14 @@ import io.kaitai.struct.format.ClassSpec
 import org.scalatest.funspec.AnyFunSpec
 import org.scalatest.matchers.should.Matchers._
 
+import scala.collection.immutable.SortedMap
+
 class SwitchType$Test extends AnyFunSpec {
   describe("SwitchType.parseSwitch") {
     it ("combines ints properly") {
       val t = SwitchType(
         Expressions.parse("foo"),
-        Map(
+        SortedMap(
           Expressions.parse("1") -> DataType.IntMultiType(true, DataType.Width2, Some(LittleEndian)),
           Expressions.parse("2") -> DataType.IntMultiType(false, DataType.Width4, Some(LittleEndian))
         )
@@ -28,7 +30,7 @@ class SwitchType$Test extends AnyFunSpec {
 
       val t = SwitchType(
         Expressions.parse("foo"),
-        Map(
+        SortedMap(
           Expressions.parse("1") -> ut1,
           Expressions.parse("2") -> ut2
         )

diff --git a/shared/src/main/scala/io/kaitai/struct/ClassCompiler.scala b/shared/src/main/scala/io/kaitai/struct/ClassCompiler.scala
@@ -358,7 +358,7 @@ class ClassCompiler(
   }
 
   def compileEnum(curClass: ClassSpec, enumColl: EnumSpec): Unit =
-    lang.enumDeclaration(curClass.name, enumColl.name.last, enumColl.sortedSeq)
+    lang.enumDeclaration(curClass.name, enumColl.name.last, enumColl.map.toSeq)
 
   def isUnalignedBits(dt: DataType): Boolean =
     dt match {

diff --git a/shared/src/main/scala/io/kaitai/struct/ConstructClassCompiler.scala b/shared/src/main/scala/io/kaitai/struct/ConstructClassCompiler.scala
@@ -104,7 +104,7 @@ class ConstructClassCompiler(classSpecs: ClassSpecs, topClass: ClassSpec) extend
     importList.add("import enum")
     out.puts(s"class ${types2class(enumSpec.name)}(enum.IntEnum):")
     out.inc
-    enumSpec.sortedSeq.foreach { case (id, valueSpec) =>
+    enumSpec.map.foreach { case (id, valueSpec) =>
       out.puts(s"${valueSpec.name} = ${translator.doIntLiteral(id)}")
     }
     out.dec

diff --git a/shared/src/main/scala/io/kaitai/struct/HtmlClassCompiler.scala b/shared/src/main/scala/io/kaitai/struct/HtmlClassCompiler.scala
@@ -112,7 +112,7 @@ class HtmlClassCompiler(classSpecs: ClassSpecs, topClass: ClassSpec) extends Doc
     out.puts("<th>ID</th><th>Name</th><th>Note</th>")
     out.puts("</tr>")
 
-    enumColl.sortedSeq.foreach { case (id, value) =>
+    enumColl.map.foreach { case (id, value) =>
       out.puts("<tr>")
       out.puts(s"<td>$id</td><td>${value.name}</td><td>${value.doc.summary.getOrElse("")}</td></tr>")
       out.puts("</tr>")

diff --git a/shared/src/main/scala/io/kaitai/struct/datatype/DataType.scala b/shared/src/main/scala/io/kaitai/struct/datatype/DataType.scala
@@ -5,6 +5,8 @@ import io.kaitai.struct.format._
 import io.kaitai.struct.problems.KSYParseError
 import io.kaitai.struct.translators.TypeDetector
 
+import scala.collection.immutable.SortedMap
+
 sealed trait DataType {
   /**
     * @return Data type as non-owning data type. Default implementation
@@ -228,7 +230,7 @@ object DataType {
 
   case class SwitchType(
     on: Ast.expr,
-    cases: Map[Ast.expr, DataType],
+    cases: SortedMap[Ast.expr, DataType],
     isOwning: Boolean = true,
     override val isOwningInExpr: Boolean = false
   ) extends ComplexDataType {
@@ -303,12 +305,13 @@ object DataType {
       val (_on, _cases) = fromYaml1(switchSpec, path)
 
       val on = Expressions.parse(_on)
-      val cases: Map[Ast.expr, DataType] = _cases.map { case (condition, typeName) =>
-        Expressions.parse(condition) -> DataType.fromYaml(
-          Some(typeName), path ++ List("cases"), metaDef,
-          arg
-        )
-      }
+      val cases: Map[Ast.expr, DataType] = SortedMap.from(
+        _cases.map { case (condition, typeName) =>
+          Expressions.parse(condition) -> DataType.fromYaml(
+            Some(typeName), path ++ List("cases"), metaDef,
+            arg
+          )
+        })
 
       // If we have size defined, and we don't have any "else" case already, add
       // an implicit "else" case that will at least catch everything else as
@@ -328,7 +331,7 @@ object DataType {
         }
       }
 
-      SwitchType(on, cases ++ addCases)
+      SwitchType(on, SortedMap.from(cases ++ addCases))
     }
   }
 

diff --git a/shared/src/main/scala/io/kaitai/struct/exprlang/Ast.scala b/shared/src/main/scala/io/kaitai/struct/exprlang/Ast.scala
@@ -1,6 +1,6 @@
 package io.kaitai.struct.exprlang
 
-import io.kaitai.struct.format.Identifier
+import io.kaitai.struct.format.{Identifier, InstanceIdentifier}
 
 /**
   * Loosely based on /pythonparse/shared/src/main/scala/pythonparse/
@@ -89,6 +89,12 @@ object Ast {
     /** For internal use in the compiler. It cannot appear in an AST parsed from a user-supplied string. */
     case class InternalName(id: Identifier) extends expr
     case class List(elts: Seq[expr]) extends expr
+
+    /**
+     * Implicit declaration of ordering, so expressions can be used for ordering operations, e.g.
+     * for `SortedMap.from(...)`
+     */
+    implicit val ExprIdentifierOrdering: Ordering[expr] = Ordering.by(_.toString)
   }
 
   sealed trait boolop

diff --git a/shared/src/main/scala/io/kaitai/struct/format/AttrSpec.scala b/shared/src/main/scala/io/kaitai/struct/format/AttrSpec.scala
@@ -8,6 +8,8 @@ import io.kaitai.struct.exprlang.Ast.expr
 import io.kaitai.struct.exprlang.{Ast, Expressions}
 import io.kaitai.struct.problems.KSYParseError
 
+import scala.collection.immutable.SortedMap
+
 case class ConditionalSpec(ifExpr: Option[Ast.expr], repeat: RepeatSpec)
 
 trait AttrLikeSpec extends MemberSpec {
@@ -312,6 +314,6 @@ object AttrSpec {
       }
     }
 
-    SwitchType(on, cases ++ addCases)
+    SwitchType(on, SortedMap.from(cases ++ addCases))
   }
 }
diff --git a/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala b/shared/src/main/scala/io/kaitai/struct/format/ClassSpec.scala
@@ -5,6 +5,7 @@ import io.kaitai.struct.datatype.DataType._
 import io.kaitai.struct.exprlang.Ast
 import io.kaitai.struct.problems.KSYParseError
 
+import scala.collection.immutable.SortedMap
 import scala.collection.mutable
 
 /**
@@ -36,9 +37,9 @@ case class ClassSpec(
   toStringExpr: Option[Ast.expr],
   params: List[ParamDefSpec],
   seq: List[AttrSpec],
-  types: Map[String, ClassSpec],
-  instances: Map[InstanceIdentifier, InstanceSpec],
-  enums: Map[String, EnumSpec]
+  types: SortedMap[String, ClassSpec],
+  instances: SortedMap[InstanceIdentifier, InstanceSpec],
+  enums: SortedMap[String, EnumSpec]
 ) extends ClassSpecLike with YAMLPath {
   var parentClass: ClassSpecLike = UnknownClassSpec
 
@@ -154,20 +155,20 @@ object ClassSpec {
       case Some(value) => seqFromYaml(value, path ++ List("seq"), meta)
       case None => List()
     }
-    val instances: Map[InstanceIdentifier, InstanceSpec] = srcMap.get("instances") match {
+    val instances: SortedMap[InstanceIdentifier, InstanceSpec] = srcMap.get("instances") match {
       case Some(value) => instancesFromYaml(value, path ++ List("instances"), meta)
-      case None => Map()
+      case None => SortedMap()
     }
 
     checkDupMemberIds(params ++ seq ++ instances.values)
 
-    val types: Map[String, ClassSpec] = srcMap.get("types") match {
+    val types: SortedMap[String, ClassSpec] = srcMap.get("types") match {
       case Some(value) => typesFromYaml(value, fileName, path ++ List("types"), meta)
-      case None => Map()
+      case None => SortedMap()
     }
-    val enums: Map[String, EnumSpec] = srcMap.get("enums") match {
+    val enums: SortedMap[String, EnumSpec] = srcMap.get("enums") match {
       case Some(value) => enumsFromYaml(value, path ++ List("enums"))
-      case None => Map()
+      case None => SortedMap()
     }
 
     val cs = ClassSpec(
@@ -240,31 +241,37 @@ object ClassSpec {
     }
   }
 
-  def typesFromYaml(src: Any, fileName: Option[String], path: List[String], metaDef: MetaSpec): Map[String, ClassSpec] = {
+  def typesFromYaml(src: Any, fileName: Option[String], path: List[String], metaDef: MetaSpec): SortedMap[String, ClassSpec] = {
     val srcMap = ParseUtils.asMapStr(src, path)
-    srcMap.map { case (typeName, body) =>
-      Identifier.checkIdentifierSource(typeName, "type", path ++ List(typeName))
-      typeName -> ClassSpec.fromYaml(body, fileName, path ++ List(typeName), metaDef)
-    }
+    SortedMap.from(
+      srcMap.map { case (typeName, body) =>
+        Identifier.checkIdentifierSource(typeName, "type", path ++ List(typeName))
+        typeName -> ClassSpec.fromYaml(body, fileName, path ++ List(typeName), metaDef)
+      }
+    )
   }
 
-  def instancesFromYaml(src: Any, path: List[String], metaDef: MetaSpec): Map[InstanceIdentifier, InstanceSpec] = {
+  def instancesFromYaml(src: Any, path: List[String], metaDef: MetaSpec): SortedMap[InstanceIdentifier, InstanceSpec] = {
     val srcMap = ParseUtils.asMap(src, path)
-    srcMap.map { case (key, body) =>
-      val instName = ParseUtils.asStr(key, path)
-      Identifier.checkIdentifierSource(instName, "instance", path ++ List(instName))
-      val id = InstanceIdentifier(instName)
-      id -> InstanceSpec.fromYaml(body, path ++ List(instName), metaDef, id)
-    }
+    SortedMap.from(
+      srcMap.map { case (key, body) =>
+        val instName = ParseUtils.asStr(key, path)
+        Identifier.checkIdentifierSource(instName, "instance", path ++ List(instName))
+        val id = InstanceIdentifier(instName)
+        id -> InstanceSpec.fromYaml(body, path ++ List(instName), metaDef, id)
+      }
+    )
   }
 
-  def enumsFromYaml(src: Any, path: List[String]): Map[String, EnumSpec] = {
+  def enumsFromYaml(src: Any, path: List[String]): SortedMap[String, EnumSpec] = {
     val srcMap = ParseUtils.asMap(src, path)
-    srcMap.map { case (key, body) =>
-      val enumName = ParseUtils.asStr(key, path)
-      Identifier.checkIdentifierSource(enumName, "enum", path ++ List(enumName))
-      enumName -> EnumSpec.fromYaml(body, path ++ List(enumName))
-    }
+    SortedMap.from(
+      srcMap.map { case (key, body) =>
+        val enumName = ParseUtils.asStr(key, path)
+        Identifier.checkIdentifierSource(enumName, "enum", path ++ List(enumName))
+        enumName -> EnumSpec.fromYaml(body, path ++ List(enumName))
+      }
+    )
   }
 
   def fromYaml(src: Any, fileName: Option[String]): ClassSpec = fromYaml(src, fileName, List(), MetaSpec.OPAQUE)
@@ -279,9 +286,9 @@ object ClassSpec {
       toStringExpr = None,
       params = List(),
       seq = List(),
-      types = Map(),
-      instances = Map(),
-      enums = Map()
+      types = SortedMap(),
+      instances = SortedMap(),
+      enums = SortedMap()
     )
     placeholder.name = typeName
     placeholder

diff --git a/shared/src/main/scala/io/kaitai/struct/format/EnumSpec.scala b/shared/src/main/scala/io/kaitai/struct/format/EnumSpec.scala
@@ -2,40 +2,37 @@ package io.kaitai.struct.format
 
 import io.kaitai.struct.problems.KSYParseError
 
+import scala.collection.immutable.SortedMap
 import scala.collection.mutable
 
-case class EnumSpec(path: List[String], map: Map[Long, EnumValueSpec]) extends YAMLPath {
+case class EnumSpec(path: List[String], map: SortedMap[Long, EnumValueSpec]) extends YAMLPath {
   var name = List[String]()
 
   /**
     * @return Absolute name of enum as string, components separated by
     *         double colon operator `::`
     */
   def nameAsStr = name.mkString("::")
-
-  /**
-    * Stabilize order of generated enums by sorting it by integer ID - it
-    * both looks nicer and doesn't screw diffs in generated code.
-    */
-  lazy val sortedSeq: Seq[(Long, EnumValueSpec)] = map.toSeq.sortBy(_._1)
 }
 
 object EnumSpec {
   def fromYaml(src: Any, path: List[String]): EnumSpec = {
     val srcMap = ParseUtils.asMap(src, path)
     val memberNameMap = mutable.Map[String, Long]()
-    EnumSpec(path, srcMap.map { case (id, desc) =>
-      val idLong = ParseUtils.asLong(id, path)
-      val value = EnumValueSpec.fromYaml(desc, path ++ List(idLong.toString))
+    EnumSpec(path, SortedMap.from(
+      srcMap.map { case (id, desc) =>
+        val idLong = ParseUtils.asLong(id, path)
+        val value = EnumValueSpec.fromYaml(desc, path ++ List(idLong.toString))
 
-      memberNameMap.get(value.name).foreach { (prevIdLong) =>
-        throw KSYParseError.withText(
-          s"duplicate enum member ID: '${value.name}', previously defined at /${(path ++ List(prevIdLong.toString)).mkString("/")}",
-          path ++ List(idLong.toString)
-        )
+        memberNameMap.get(value.name).foreach { (prevIdLong) =>
+          throw KSYParseError.withText(
+            s"duplicate enum member ID: '${value.name}', previously defined at /${(path ++ List(prevIdLong.toString)).mkString("/")}",
+            path ++ List(idLong.toString)
+          )
+        }
+        memberNameMap.put(value.name, idLong)
+        idLong -> value
       }
-      memberNameMap.put(value.name, idLong)
-      idLong -> value
-    })
+    ))
   }
 }
diff --git a/shared/src/main/scala/io/kaitai/struct/format/Identifier.scala b/shared/src/main/scala/io/kaitai/struct/format/Identifier.scala
@@ -80,6 +80,12 @@ object Identifier {
   val SWITCH_ON = "_on"
   val IS_LE = "_is_le"
   val SIZEOF = "_sizeof"
+
+  /**
+   * Implicit declaration of ordering, so identifiers can be used for ordering operations, e.g.
+   * for `SortedMap.from(...)`
+   */
+  implicit val InstanceIdentifierOrdering: Ordering[InstanceIdentifier] = Ordering.by(_.humanReadable)
 }
 
 case class RawIdentifier(innerId: Identifier) extends Identifier {