From ea8720f218a4765b64e6313ac9f4a84d99c6e315 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Mon, 12 Jun 2023 16:50:22 -0700 Subject: [PATCH] move noDuplicateKeysInComprehension under a flag for easy migration --- sjsonnet/src-jvm-native/sjsonnet/Config.scala | 5 +++++ .../sjsonnet/SjsonnetMain.scala | 1 + sjsonnet/src/sjsonnet/Evaluator.scala | 2 +- sjsonnet/src/sjsonnet/Settings.scala | 1 + .../test/src/sjsonnet/EvaluatorTests.scala | 19 +++++++++++++++++-- sjsonnet/test/src/sjsonnet/TestUtils.scala | 19 +++++++++++++------ 6 files changed, 38 insertions(+), 9 deletions(-) diff --git a/sjsonnet/src-jvm-native/sjsonnet/Config.scala b/sjsonnet/src-jvm-native/sjsonnet/Config.scala index 3c696727..586a6b59 100644 --- a/sjsonnet/src-jvm-native/sjsonnet/Config.scala +++ b/sjsonnet/src-jvm-native/sjsonnet/Config.scala @@ -123,4 +123,9 @@ case class Config( doc = "Evaluate the given string as Jsonnet rather than treating it as a file name" ) exec: Flag = Flag(), + @arg( + name = "no-duplicate-keys-in-comprehension", + doc = "Raise an error if an object comprehension contains duplicate keys" + ) + noDuplicateKeysInComprehension: Flag = Flag(), ) diff --git a/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMain.scala b/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMain.scala index 576ad262..e06d818a 100644 --- a/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMain.scala +++ b/sjsonnet/src-jvm-native/sjsonnet/SjsonnetMain.scala @@ -215,6 +215,7 @@ object SjsonnetMain { preserveOrder = config.preserveOrder.value, strict = config.strict.value, noStaticErrors = config.noStaticErrors.value, + noDuplicateKeysInComprehension = config.noDuplicateKeysInComprehension.value, ), storePos = if (config.yamlDebug.value) currentPos = _ else null, warnLogger = warnLogger, diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index 089ce3a2..3cd5946b 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -608,7 +608,7 @@ class Evaluator(resolver: CachedResolver, s.extend(newBindings, self, null) ) }) - if (prev_length == builder.size()) { + if (prev_length == builder.size() && settings.noDuplicateKeysInComprehension) { Error.fail(s"Duplicate key ${k} in evaluated object comprehension.", e.pos); } case Val.Null(_) => // do nothing diff --git a/sjsonnet/src/sjsonnet/Settings.scala b/sjsonnet/src/sjsonnet/Settings.scala index 5c9cf8fc..acb8511c 100644 --- a/sjsonnet/src/sjsonnet/Settings.scala +++ b/sjsonnet/src/sjsonnet/Settings.scala @@ -6,6 +6,7 @@ class Settings( val preserveOrder: Boolean = false, val strict: Boolean = false, val noStaticErrors: Boolean = false, + val noDuplicateKeysInComprehension: Boolean = false, ) object Settings { diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala index 6a424fdd..24a60691 100644 --- a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala +++ b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala @@ -333,18 +333,33 @@ object EvaluatorTests extends TestSuite{ eval("""{ ["bar_" + x]: x for x in [5,12]}""") ==> ujson.Obj("bar_5" -> 5, "bar_12" -> 12) } test("givenDuplicateFieldsInListComprehension_expectFailure") { - evalErr("""{ [x]: x for x in ["A", "A"]}""") ==> + evalErr("""{ [x]: x for x in ["A", "A"]}""", noDuplicateKeysInComprehension = true) ==> """sjsonnet.Error: Duplicate key A in evaluated object comprehension. |at .(:1:3)""".stripMargin } + test("givenDuplicateFieldsInListComprehension_noflag_expectSuccess") { + eval("""{ [x]: x for x in ["A", "A"]}""", noDuplicateKeysInComprehension = false) ==> + ujson.Obj("A" -> "A") + } test("givenDuplicateFieldsInIndirectListComprehension_expectFailure") { evalErr( """local y = { a: "A" }; |local z = { a: "A" }; - |{ [x.a]: x for x in [y, z]}""".stripMargin) ==> + |{ [x.a]: x for x in [y, z]}""".stripMargin, + noDuplicateKeysInComprehension = true + ) ==> """sjsonnet.Error: Duplicate key A in evaluated object comprehension. |at .(:3:3)""".stripMargin } + test("givenDuplicateFieldsInIndirectListComprehension_noflag_expectSuccess") { + eval( + """local y = { a: "A" }; + |local z = { a: "A", "b": "B" }; + |{ [x.a]: x for x in [y, z]}""".stripMargin, + noDuplicateKeysInComprehension = false + ) ==> + ujson.Obj("A" -> ujson.Obj("a" -> "A", "b" -> "B")) + } test("functionEqualsNull") { eval("""local f(x)=null; f == null""") ==> ujson.False eval("""local f=null; f == null""") ==> ujson.True diff --git a/sjsonnet/test/src/sjsonnet/TestUtils.scala b/sjsonnet/test/src/sjsonnet/TestUtils.scala index 72028e98..1e8991b1 100644 --- a/sjsonnet/test/src/sjsonnet/TestUtils.scala +++ b/sjsonnet/test/src/sjsonnet/TestUtils.scala @@ -1,26 +1,33 @@ package sjsonnet object TestUtils { - def eval0(s: String, preserveOrder: Boolean = false, strict: Boolean = false) = { + def eval0(s: String, + preserveOrder: Boolean = false, + strict: Boolean = false, + noDuplicateKeysInComprehension: Boolean = false) = { new Interpreter( Map(), Map(), DummyPath(), Importer.empty, parseCache = new DefaultParseCache, - new Settings(preserveOrder = preserveOrder, strict = strict) + new Settings( + preserveOrder = preserveOrder, + strict = strict, + noDuplicateKeysInComprehension = noDuplicateKeysInComprehension + ) ).interpret(s, DummyPath("(memory)")) } - def eval(s: String, preserveOrder: Boolean = false, strict: Boolean = false) = { - eval0(s, preserveOrder, strict) match { + def eval(s: String, preserveOrder: Boolean = false, strict: Boolean = false, noDuplicateKeysInComprehension: Boolean = false) = { + eval0(s, preserveOrder, strict, noDuplicateKeysInComprehension) match { case Right(x) => x case Left(e) => throw new Exception(e) } } - def evalErr(s: String, preserveOrder: Boolean = false, strict: Boolean = false) = { - eval0(s, preserveOrder, strict) match{ + def evalErr(s: String, preserveOrder: Boolean = false, strict: Boolean = false, noDuplicateKeysInComprehension: Boolean = false) = { + eval0(s, preserveOrder, strict, noDuplicateKeysInComprehension) match{ case Left(err) => err.split('\n').map(_.trim).mkString("\n") // normalize inconsistent indenation on JVM vs JS case Right(r) => throw new Exception(s"Expected exception, got result: $r") }