0.9.95 fix #54 "Shared config objects"

#54 (comment)

Author: carueda

build.sbt

@@ -1,4 +1,4 @@
-lazy val tscfgVersion = setVersion("0.9.94")
+lazy val tscfgVersion = setVersion("0.9.95")
 
 organization := "com.github.carueda"
 name := "tscfg"

changelog.md

@@ -1,3 +1,9 @@
+2019-10-07 - 0.9.95
+
+- fix #54 "Shared config objects"
+  https://github.com/carueda/tscfg/issues/54#issuecomment-539096913.
+  In short, the `@define`s are traversed first in ModelBuilder.
+
 2019-09-14 - 0.9.94
 
 - resolve #54 "Shared config objects"

src/main/resources/application.conf

@@ -1 +1 @@
-tscfg.version = 0.9.94
+tscfg.version = 0.9.95

src/main/scala/tscfg/ModelBuilder.scala

@@ -2,8 +2,8 @@ package tscfg
 
 import com.typesafe.config._
 import tscfg.generators.tsConfigUtil
-import tscfg.model.{AnnType, DURATION, ObjectType, SIZE}
 import tscfg.model.durations.ms
+import tscfg.model.{DURATION, SIZE}
 
 import scala.collection.JavaConverters._
 
@@ -42,13 +42,18 @@ class ModelBuilder(assumeAllRequired: Boolean = false) {
   private val warns = collection.mutable.ArrayBuffer[Warning]()
 
   private def fromConfig(namespace: Namespace, conf: Config): model.ObjectType = {
-    // do two passes as lightbend config does not necessarily preserve member order:
-    val ot = fromConfig1(namespace, conf)
-    fromConfig2(namespace, ot)
-  }
-
-  private def fromConfig1(namespace: Namespace, conf: Config): model.ObjectType = {
     val memberStructs = getMemberStructs(conf)
+      // have the `@define`s be traversed first:
+      .sortWith { case (childStruct, s2) ⇒
+        if (childStruct.isLeaf) false
+        else if (s2.isLeaf) true
+        else {
+          val cv = conf.getValue(childStruct.name)
+          val comments = cv.origin().comments().asScala.toList
+          comments.exists(_.trim.startsWith("@define"))
+        }
+      }
+
     val members: immutable.Map[String, model.AnnType] = memberStructs.map { childStruct ⇒
       val name = childStruct.name
       val cv = conf.getValue(name)
@@ -121,35 +126,6 @@ class ModelBuilder(assumeAllRequired: Boolean = false) {
     model.ObjectType(members)
   }
 
-  private def fromConfig2(namespace: Namespace, ot: model.ObjectType): model.ObjectType = {
-    val resolvedMembers = ot.members.map { case (name, annType) ⇒
-      val modAnnType = annType.t match {
-
-        case _:model.STRING.type ⇒
-          annType.default match {
-            case Some(strValue) ⇒
-              namespace.resolveDefine(strValue) match {
-                case Some(ort) ⇒ AnnType(ort)
-                case _ ⇒ annType
-              }
-
-            case None ⇒ annType
-          }
-
-        //// the following would be part of changes to allow recursive type
-        //case ot:ObjectType ⇒
-        //  val ot2 = fromConfig2(namespace, ot)
-        //  AnnType(ot2)
-
-        case _ ⇒ annType
-      }
-
-      name → modAnnType
-    }
-
-    model.ObjectType(resolvedMembers)
-  }
-
   private case class Struct(name: String, members: mutable.HashMap[String, Struct] = mutable.HashMap.empty) {
     def isLeaf: Boolean = members.isEmpty
     // $COVERAGE-OFF$

src/main/scala/tscfg/Namespace.scala

@@ -3,21 +3,15 @@ package tscfg
 import tscfg.model.{ObjectRefType, Type}
 
 object Namespace {
-  val root: Namespace = new Namespace("", None)
-
-  def getAllDefines: Map[String, Type] = {
-    allDefines.toMap
-  }
-
-  private def addToAllDefines(defineFullPath: String, t: Type): Unit = {
-    allDefines.update(defineFullPath, t)
-  }
-
-  private val allDefines = collection.mutable.HashMap[String, Type]()
+  /** Returns a new, empty root namespace. */
+  def root: Namespace = new Namespace("", None,
+    collection.mutable.HashMap[String, Type]())
 }
 
-class Namespace private(simpleName: String, parent: Option[Namespace]) {
-  import Namespace.addToAllDefines
+class Namespace private(simpleName: String, parent: Option[Namespace],
+                        allDefines: collection.mutable.HashMap[String, Type]) {
+
+  def getAllDefines: Map[String, Type] = allDefines.toMap
 
   def getPath: Seq[String] = parent match {
     case None ⇒ Seq.empty
@@ -26,7 +20,7 @@ class Namespace private(simpleName: String, parent: Option[Namespace]) {
 
   def getPathString: String = getPath.mkString(".")
 
-  def extend(simpleName: String): Namespace = new Namespace(simpleName, Some(this))
+  def extend(simpleName: String): Namespace = new Namespace(simpleName, Some(this), allDefines)
 
   private val defineNames = collection.mutable.HashSet[String]()
 
@@ -35,13 +29,14 @@ class Namespace private(simpleName: String, parent: Option[Namespace]) {
     assert(simpleName.nonEmpty)
 
     if (defineNames.contains(simpleName)) {
-      println(s"WARN: duplicate @define '$simpleName' in namespace $getPathString. Ignoring previous entry")
+      val ns = if (getPath.nonEmpty) s"'$getPathString'" else "(root)"
+      println(s"WARN: duplicate @define '$simpleName' in namespace $ns. Ignoring previous entry")
       // TODO include in build warnings
     }
 
     defineNames.add(simpleName)
 
-    addToAllDefines(resolvedFullPath(simpleName), t)
+    allDefines.update(resolvedFullPath(simpleName), t)
   }
 
   private def resolvedFullPath(simpleName: String): String = parent match {

src/main/tscfg/example/issue54exampleD.spec.conf

@@ -0,0 +1,8 @@
+#@define
+Struct = {
+  a = int
+}
+
+exampleD {
+  test = Struct
+}

src/main/tscfg/example/issue54exampleE.spec.conf

@@ -0,0 +1,8 @@
+#@define
+Struct = {
+  a = int
+}
+
+exampleE {
+  test = Struct
+}

src/test/scala/tscfg/NamespaceSpec.scala

@@ -22,7 +22,7 @@ class NamespaceSpec extends Specification {
     "add and resolve define" in {
       root.addDefine("RootDef1", objectType)
       root.resolveDefine("RootDef1") must beSome
-      Namespace.getAllDefines.keys must_== Set("RootDef1")
+      root.getAllDefines.keys must_== Set("RootDef1")
     }
   }
 
@@ -35,7 +35,7 @@ class NamespaceSpec extends Specification {
       ns00.addDefine("n00def1", objectType)
       ns00.resolveDefine("n00def1") must beSome
 
-      Namespace.getAllDefines.keys must_== Set("RootDef1", "ns00.n00def1")
+      root.getAllDefines.keys must_== Set("RootDef1", "ns00.n00def1")
     }
 
     "resolve define in parent namespace" in {
@@ -52,7 +52,7 @@ class NamespaceSpec extends Specification {
       ns000.addDefine("n000def1", objectType)
       ns000.resolveDefine("n000def1") must beSome
 
-      Namespace.getAllDefines.keys must_== Set(
+      root.getAllDefines.keys must_== Set(
         "RootDef1", "ns00.n00def1", "ns00.ns000.n000def1"
       )
     }
@@ -77,7 +77,7 @@ class NamespaceSpec extends Specification {
 
   "all defines" should {
     "resolve" in {
-      val all = Namespace.getAllDefines
+      val all = root.getAllDefines
       all.size must_== 3
       all.get("RootDef1") must beSome(objectType)
       all.get("ns00.n00def1") must beSome(objectType)

src/test/scala/tscfg/generators/java/JavaMainSpec.scala

@@ -732,6 +732,36 @@ class JavaMainSpec extends Specification {
     }
   }
 
+  "issue 54 - shared config - exampleD" should {
+    "be handled" in {
+      val c = new JavaIssue54exampleDCfg(ConfigFactory.parseString(
+        """
+          |exampleD {
+          |  test {
+          |    a = 1
+          |  }
+          |}
+          |""".stripMargin))
+
+      c.exampleD.test.a === 1
+    }
+  }
+
+  "issue 54 - shared config - exampleE" should {
+    "be handled" in {
+      val c = new JavaIssue54exampleECfg(ConfigFactory.parseString(
+        """
+          |exampleE {
+          |  test {
+          |    a = 1
+          |  }
+          |}
+          |""".stripMargin))
+
+      c.exampleE.test.a === 1
+    }
+  }
+
 /* SKIP due to weird travis-ci issue (with the scala version, not the java one)
   "issue 54 - shared config - example2" should {
     "be handled" in {

src/test/scala/tscfg/generators/scala/ScalaMainSpec.scala

@@ -641,6 +641,36 @@ class ScalaMainSpec extends Specification {
     }
   }
 
+  "issue 54 - shared config - exampleD" should {
+    "be handled" in {
+      val c = ScalaIssue54exampleDCfg(ConfigFactory.parseString(
+        """
+          |exampleD {
+          |  test {
+          |    a = 1
+          |  }
+          |}
+          |""".stripMargin))
+
+      c.exampleD.test.a === 1
+    }
+  }
+
+  "issue 54 - shared config - exampleE" should {
+    "be handled" in {
+      val c = ScalaIssue54exampleECfg(ConfigFactory.parseString(
+        """
+          |exampleE {
+          |  test {
+          |    a = 1
+          |  }
+          |}
+          |""".stripMargin))
+
+      c.exampleE.test.a === 1
+    }
+  }
+
   // for some strange reason ScalaIssue54bCfg keeps of getting wrongly generated on travis-ci:
   //    [error] /home/travis/build/carueda/tscfg/src/test/scala/tscfg/example/ScalaIssue54bCfg.scala:8: not found: type Shared
   //    [error]     e      : Shared,