From 525935548d3ac2e31e615d117f1984b0af09baa5 Mon Sep 17 00:00:00 2001
From: Edgar Fernandes <edgar.fernandes@databricks.com>
Date: Mon, 9 Nov 2020 12:45:56 +0100
Subject: [PATCH] [GHI-99] - Do not allow duplicate field in object in list
 list comprehension

---
 sjsonnet/src/sjsonnet/Parser.scala            | 14 +++++++++
 .../test/src/sjsonnet/EvaluatorTests.scala    |  6 ++++
 sjsonnet/test/src/sjsonnet/ParserTests.scala  | 29 ++-----------------
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/sjsonnet/src/sjsonnet/Parser.scala b/sjsonnet/src/sjsonnet/Parser.scala
index d75428ce..1975d2ff 100644
--- a/sjsonnet/src/sjsonnet/Parser.scala
+++ b/sjsonnet/src/sjsonnet/Parser.scala
@@ -291,6 +291,20 @@ 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
+       */
+      if (lhs.isInstanceOf[Expr.Str] &&
+          comps._1.isInstanceOf[Expr.ForSpec] && 
+          comps._1.asInstanceOf[Expr.ForSpec].cond.isInstanceOf[Expr.Arr] &&
+          comps._1.asInstanceOf[Expr.ForSpec].cond.asInstanceOf[Expr.Arr].value.length > 1
+      ) {
+        Fail.opaque(s"""no duplicate field: "${lhs.asInstanceOf[Expr.Str].value}" """)
+      }
       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
index 5f607068..12612625 100644
--- 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
index 579d39a2..9ef693bf 100644
--- 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 "}""""
+    }
   }
 }