Merge pull request #100 from databricks/edgar/fix-duplicate-field-on-…

…list-comprehension Do not allow duplicate field in object when evaluating list list comprehension
databricks · Nov 9, 2020 · 1143556 · 1143556
2 parents 2c909ba + a893c6f
commit 1143556
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 26 deletions.
diff --git a/sjsonnet/src/sjsonnet/Parser.scala b/sjsonnet/src/sjsonnet/Parser.scala
@@ -291,6 +291,18 @@ object Parser{
         exprs(preLocals.length)
       val postLocals = exprs.drop(preLocals.length+1).takeWhile(_.isInstanceOf[Expr.Member.BindStmt])
         .map(_.asInstanceOf[Expr.Member.BindStmt])
+
+      /* 
+       * Prevent duplicate fields in list comprehension. See: https://github.com/databricks/sjsonnet/issues/99
+       * 
+       * If comps._1 is a forspec with value greater than one lhs cannot be a Expr.Str
+       * Otherwise the field value will be overriden by the multiple iterations of forspec
+       */
+      (lhs, comps) match {
+        case (Expr.Str(_, _), (Expr.ForSpec(_, _, Expr.Arr(_, values)), _)) if values.length > 1 =>  
+          Fail.opaque(s"""no duplicate field: "${lhs.asInstanceOf[Expr.Str].value}" """)
+        case _ => // do nothing
+      }
       Expr.ObjBody.ObjComp(preLocals, lhs, rhs, postLocals, comps._1, comps._2)
   }
 

diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
@@ -328,5 +328,11 @@ 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)
     }
+    test("givenNoDuplicateFieldsInListComprehension1_expectSuccess") {
+      eval("""{ ["bar"]: x for x in [-876.89]}""") ==> ujson.Obj("bar" -> -876.89)
+    }
+    test("givenNoDuplicateFieldsInListComprehension2_expectSuccess") {
+      eval("""{ ["bar_" + x]: x for x in [5,12]}""") ==> ujson.Obj("bar_5" -> 5, "bar_12" -> 12)
+    }
   }
 }
diff --git a/sjsonnet/test/src/sjsonnet/ParserTests.scala b/sjsonnet/test/src/sjsonnet/ParserTests.scala
@@ -17,35 +17,12 @@ object ParserTests extends TestSuite{
 
       parse("1 + 2 * 3") ==>
         BinaryOp(2, Num(0, 1), BinaryOp.`+`, BinaryOp(6, Num(4, 2), BinaryOp.`*`, Num(8, 3)))
-//
-//      parse("2 | 3 * 2 + 3 | 4") ==>
-//        BinaryOp(
-//          BinaryOp(
-//            Num(2),
-//            "|",
-//            BinaryOp(
-//              BinaryOp(
-//                Num(3),
-//                "*",
-//                Num(2)
-//              ),
-//              "+",
-//              Num(3)
-//            )
-//          ),
-//          "|",
-//          Num(4)
-//        )
-//    }
-//
-//    'array - {
-//      parse("[]") ==> Arr(Nil)
-//      parse("[true]") ==> Arr(Seq(True))
-//      parse("[1, 2]") ==> Arr(Seq(Num(1), Num(2)))
-//      parse("[1, [2, 3], 4]") ==> Arr(Seq(Num(1), Arr(Seq(Num(2), Num(3))), Num(4)))
     }
     test("duplicateFields") {
       parseErr("{ a: 1, a: 2 }") ==> """Expected no duplicate field: a:1:14, found "}""""
     }
+    test("givenDuplicateFieldsInListComprehension_expectError") {
+      parseErr("""{ ["bar"]: x for x in [1, 2]}""") ==> """Expected no duplicate field: "bar" :1:29, found "}""""
+    }
   }
 }