Add flag re-introduce broken assertions evaluation, as a temporary measure to help with migration (#536)

Workaround for #526

Author: stephenamar-db

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

@@ -139,6 +139,12 @@ final case class Config(
       doc = """If set, reverses the import order of specified jpaths (so that the rightmost wins)"""
     )
     reverseJpathsPriority: Flag = Flag(),
+    @arg(
+      name = "broken-assertion-logic",
+      doc =
+        "Re-enable pre-0.5.5 broken assertion logic. See https://github.com/databricks/sjsonnet/issues/526."
+    )
+    brokenAssertionLogic: Flag = Flag(),
     @arg(
       doc = "The jsonnet file you wish to evaluate",
       positional = true

sjsonnet/src/sjsonnet/Evaluator.scala

@@ -880,8 +880,8 @@ class Evaluator(
           val k2 = y.visibleKeyNames
           val k1len = k1.length
           if (k1len != k2.length) return false
-          x.triggerAllAsserts()
-          y.triggerAllAsserts()
+          x.triggerAllAsserts(settings.brokenAssertionLogic)
+          y.triggerAllAsserts(settings.brokenAssertionLogic)
           var i = 0
           while (i < k1len) {
             val k = k1(i)

sjsonnet/src/sjsonnet/Materializer.scala

@@ -22,7 +22,7 @@ abstract class Materializer {
       case Val.Str(pos, s) => storePos(pos); visitor.visitString(s, -1)
       case obj: Val.Obj    =>
         storePos(obj.pos)
-        obj.triggerAllAsserts()
+        obj.triggerAllAsserts(evaluator.settings.brokenAssertionLogic)
         val objVisitor = visitor.visitObject(obj.visibleKeyNames.length, jsonableKeys = true, -1)
         val sort = !evaluator.settings.preserveOrder
         var prevKey: String = null

sjsonnet/src/sjsonnet/Settings.scala

@@ -8,7 +8,8 @@ class Settings(
     val preserveOrder: Boolean = false,
     val strict: Boolean = false,
     val throwErrorForInvalidSets: Boolean = false,
-    val useNewEvaluator: Boolean = false
+    val useNewEvaluator: Boolean = false,
+    val brokenAssertionLogic: Boolean = false
 )
 
 object Settings {

sjsonnet/src/sjsonnet/Val.scala

@@ -284,20 +284,24 @@ object Val {
       value0
     }
 
-    def triggerAllAsserts(): Unit = {
+    def triggerAllAsserts(brokenAssertionLogic: Boolean): Unit = {
       // We need to avoid asserting the same object more than once to prevent
       // infinite recursion
       if (!asserting) {
         asserting = true
-        triggerAllAsserts(this, `super`)
+        triggerAllAsserts(this, `super`, brokenAssertionLogic)
       }
     }
 
     // As we walk up the superclass hierarchy, the `self` binding is unchanged
     // but `super` climbs up the hierarchy as well.
-    @tailrec private def triggerAllAsserts(obj: Val.Obj, sup: Val.Obj): Unit = {
+    @tailrec private def triggerAllAsserts(
+        obj: Val.Obj,
+        sup: Val.Obj,
+        brokenAssertionLogic: Boolean): Unit = {
       if (triggerAsserts != null) triggerAsserts(obj, sup)
-      if (sup != null) sup.triggerAllAsserts(obj, sup.getSuper)
+      if ((!brokenAssertionLogic || triggerAsserts == null) && sup != null)
+        sup.triggerAllAsserts(obj, sup.getSuper, brokenAssertionLogic)
     }
 
     def addSuper(pos: Position, lhs: Val.Obj): Val.Obj = {
@@ -439,7 +443,9 @@ object Val {
           case null =>
             if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey)
           case m =>
-            self.triggerAllAsserts()
+            if (!evaluator.settings.brokenAssertionLogic || !m.isInstanceOf[Val.Obj.ConstMember]) {
+              self.triggerAllAsserts(evaluator.settings.brokenAssertionLogic)
+            }
             val vv = m.invoke(self, s, pos.fileScope, evaluator)
             val v = if (s != null && m.add) {
               s.valueRaw(k, self, pos, null, null) match {

sjsonnet/test/src/sjsonnet/EvaluatorTests.scala

@@ -2,6 +2,7 @@ package sjsonnet
 
 import utest._
 import TestUtils.{eval, evalErr}
+
 object EvaluatorTests extends TestSuite {
   def allTests(useNewEvaluator: Boolean): Tests = Tests {
     test("arithmetic") {
@@ -763,6 +764,90 @@ object EvaluatorTests extends TestSuite {
         )
       }
     }
+    test("assertInheritanceWithBrokenAssertions") {
+      test - assert(
+        evalErr(
+          """{ } + {assert false}""",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - assert(
+        evalErr(
+          """{assert false} + {}""",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - assert(
+        evalErr(
+          """{assert false} + {} + {}""",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - assert(
+        evalErr(
+          """{} + {assert false} + {}""",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - assert(
+        evalErr(
+          """{} + {} + {assert false}""",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - assert(
+        evalErr(
+          "({assert false} + {f(x): x}).f(1)",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ).contains(
+          "sjsonnet.Error: Assertion failed"
+        )
+      )
+      test - {
+        val problematicStrictInheritedAssertionsSnippet =
+          """local template = { assert self.flag };
+            |{ a: template { flag: true, }, b: template { flag: false, } }
+            |""".stripMargin
+        assert(
+          evalErr(
+            problematicStrictInheritedAssertionsSnippet,
+            useNewEvaluator = useNewEvaluator,
+            brokenAssertionLogic = true
+          ).contains("sjsonnet.Error: Assertion failed")
+        )
+      }
+
+      // Behavior is different from now on.
+      test {
+        eval(
+          "{assert false} + {assert true}",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ) ==> ujson.Obj()
+        eval(
+          "({assert false} + {x: 2}).x",
+          useNewEvaluator = useNewEvaluator,
+          brokenAssertionLogic = true
+        ) ==> ujson.Num(2)
+      }
+    }
+
   }
   def tests = allTests(false).prefix("Evaluator") ++ allTests(true).prefix("NewEvaluator")
 }

sjsonnet/test/src/sjsonnet/TestUtils.scala

@@ -8,6 +8,7 @@ object TestUtils {
       preserveOrder: Boolean = false,
       strict: Boolean = false,
       useNewEvaluator: Boolean = false,
+      brokenAssertionLogic: Boolean = false,
       std: Std = new Std()): Either[String, Value] = {
     new Interpreter(
       Map(),
@@ -19,7 +20,8 @@ object TestUtils {
         preserveOrder = preserveOrder,
         strict = strict,
         throwErrorForInvalidSets = true,
-        useNewEvaluator = useNewEvaluator
+        useNewEvaluator = useNewEvaluator,
+        brokenAssertionLogic = brokenAssertionLogic
       ),
       std = std.Std
     ).interpret(s, DummyPath("(memory)"))
@@ -30,12 +32,14 @@ object TestUtils {
       preserveOrder: Boolean = false,
       strict: Boolean = false,
       useNewEvaluator: Boolean = false,
+      brokenAssertionLogic: Boolean = false,
       std: Std = new Std()): Value = {
     eval0(
       s,
       preserveOrder,
       strict,
       useNewEvaluator,
+      brokenAssertionLogic,
       std
     ) match {
       case Right(x) => x
@@ -48,12 +52,14 @@ object TestUtils {
       preserveOrder: Boolean = false,
       strict: Boolean = false,
       useNewEvaluator: Boolean = false,
+      brokenAssertionLogic: Boolean = false,
       std: Std = new Std()): String = {
     eval0(
       s,
       preserveOrder,
       strict,
       useNewEvaluator,
+      brokenAssertionLogic,
       std
     ) match {
       case Left(err) =>