Fix +: being erroneously interpreted as : (#155)

Addresses issue #154. When using +: within an object comprehension, sjsonnet will interpret it as a : instead. This creates an inconsistency with the official implementation of jsonnet. This commit fixes said issue by making sure that + are correctly parsed and evaluated within object comprehension. Co-authored-by: Li Haoyi <[email protected]>
databricks · May 30, 2023 · a71e998 · a71e998
1 parent f4bed42
commit a71e998
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 8 deletions.
diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala
@@ -590,7 +590,7 @@ class Evaluator(resolver: CachedResolver,
         visitExpr(e.key)(s) match {
           case Val.Str(_, k) =>
             val prev_length = builder.size()
-            builder.put(k, new Val.Obj.Member(false, Visibility.Normal) {
+            builder.put(k, new Val.Obj.Member(e.plus, Visibility.Normal) {
               def invoke(self: Val.Obj, sup: Val.Obj, fs: FileScope, ev: EvalScope): Val =
                 visitExpr(e.value)(
                   s.extend(newBindings, self, null)
@@ -686,4 +686,4 @@ class Evaluator(resolver: CachedResolver,
 object Evaluator {
   val emptyStringArray = new Array[String](0)
   val emptyLazyArray = new Array[Lazy](0)
-}
+}
diff --git a/sjsonnet/src/sjsonnet/Expr.scala b/sjsonnet/src/sjsonnet/Expr.scala
@@ -161,6 +161,7 @@ object Expr{
                        preLocals: Array[Bind],
                        key: Expr,
                        value: Expr,
+                       plus: Boolean,
                        postLocals: Array[Bind],
                        first: ForSpec,
                        rest: List[CompSpec]) extends ObjBody {

diff --git a/sjsonnet/src/sjsonnet/ExprTransform.scala b/sjsonnet/src/sjsonnet/ExprTransform.scala
@@ -151,15 +151,15 @@ abstract class ExprTransform {
         if((x2 eq x) && (y2 eq y)) expr
         else ObjExtend(superPos, x2, y2.asInstanceOf[ObjBody])
 
-      case ObjBody.ObjComp(pos, p, k, v, o, f, r) =>
+      case ObjBody.ObjComp(pos, p, k, v, pl, o, f, r) =>
         val p2 = transformBinds(p)
         val k2 = transform(k)
         val v2 = transform(v)
         val o2 = transformBinds(o)
         val f2 = transform(f).asInstanceOf[ForSpec]
         val r2 = transformList(r).asInstanceOf[List[CompSpec]]
         if((p2 eq p) && (k2 eq k) && (v2 eq v) && (o2 eq o) && (f2 eq f) && (r2 eq r)) expr
-        else ObjBody.ObjComp(pos, p2, k2, v2, o2, f2, r2)
+        else ObjBody.ObjComp(pos, p2, k2, v2, pl, o2, f2, r2)
 
       case Slice(pos, v, x, y, z) =>
         val v2 = transform(v)

diff --git a/sjsonnet/src/sjsonnet/Parser.scala b/sjsonnet/src/sjsonnet/Parser.scala
@@ -332,7 +332,7 @@ class Parser(val currentFile: Path) {
       val preLocals = exprs
         .takeWhile(_.isInstanceOf[Expr.Bind])
         .map(_.asInstanceOf[Expr.Bind])
-      val Expr.Member.Field(offset, Expr.FieldName.Dyn(lhs), _, null, Visibility.Normal, rhs) =
+      val Expr.Member.Field(offset, Expr.FieldName.Dyn(lhs), plus, null, Visibility.Normal, rhs) =
         exprs(preLocals.length)
       val postLocals = exprs.drop(preLocals.length+1).takeWhile(_.isInstanceOf[Expr.Bind])
         .map(_.asInstanceOf[Expr.Bind])
@@ -350,7 +350,7 @@ class Parser(val currentFile: Path) {
           Fail.opaque(s"""no duplicate field: "${lhs.asInstanceOf[Val.Str].value}" """)
         case _ => // do nothing
       }
-      Expr.ObjBody.ObjComp(pos, preLocals.toArray, lhs, rhs, postLocals.toArray, comps._1, comps._2.toList)
+      Expr.ObjBody.ObjComp(pos, preLocals.toArray, lhs, rhs, plus, postLocals.toArray, comps._1, comps._2.toList)
   }
 
   def member[_: P]: P[Expr.Member] = P( objlocal | "assert" ~~ assertStmt | field )

diff --git a/sjsonnet/src/sjsonnet/ScopedExprTransform.scala b/sjsonnet/src/sjsonnet/ScopedExprTransform.scala
@@ -34,14 +34,14 @@ class ScopedExprTransform extends ExprTransform {
     case Function(pos, params, body) =>
       nestedNames(params.names)(rec(e))
 
-    case ObjComp(pos, preLocals, key, value, postLocals, first, rest) =>
+    case ObjComp(pos, preLocals, key, value, plus, postLocals, first, rest) =>
       val (f2 :: r2, (k2, (pre2, post2, v2))) = compSpecs(first :: rest, { () =>
         (transform(key), nestedBindings(dynamicExpr, dynamicExpr, preLocals ++ postLocals) {
           (transformBinds(preLocals), transformBinds(postLocals), transform(value))
         })
       })
       if((f2 eq first) && (k2 eq key) && (v2 eq value) && (pre2 eq preLocals) && (post2 eq postLocals) && (r2, rest).zipped.forall(_ eq _)) e
-      else ObjComp(pos, pre2, k2, v2, post2, f2.asInstanceOf[ForSpec], r2)
+      else ObjComp(pos, pre2, k2, v2, plus, post2, f2.asInstanceOf[ForSpec], r2)
 
     case Comp(pos, value, first, rest) =>
       val (f2 :: r2, v2) = compSpecs(first :: rest.toList, () => transform(value))

diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
@@ -331,6 +331,7 @@ object EvaluatorTests extends TestSuite{
 
       eval("{ ['foo']+: x for x in  []}", false) ==> ujson.Obj()
       eval("{ ['foo']+: x for x in  [1]}", false) ==> ujson.Obj("foo" -> 1)
+      eval("{ ['foo']+: [x] for x in [1]} + { ['foo']+: [x] for x in [2]}", false) ==> ujson.Obj("foo" -> ujson.Arr(1,2))
     }
     test("givenNoDuplicateFieldsInListComprehension1_expectSuccess") {
       eval("""{ ["bar"]: x for x in [-876.89]}""") ==> ujson.Obj("bar" -> -876.89)