Skip to content

Commit

Permalink
[ES-449270] Fix duplicate key error in object comprehension (#156)
Browse files Browse the repository at this point in the history
Fixes #121 

Please advise on checking for size twice vs calling `contains` once per
key, I haven't benchmarked this.

This might also break some existing files which rely on this behavior
without realizing it's out of spec, either with duplicate list entries
getting reduced or more complex replacements where only the last entry
matters. Not sure if we're ok with that or what other options we have.
Breaking these probably beats allowing duplicate entries in the future.
  • Loading branch information
ckolb-db authored May 30, 2023
1 parent ded8e14 commit 1b4a5c0
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
4 changes: 4 additions & 0 deletions sjsonnet/src/sjsonnet/Evaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -589,12 +589,16 @@ 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) {
def invoke(self: Val.Obj, sup: Val.Obj, fs: FileScope, ev: EvalScope): Val =
visitExpr(e.value)(
s.extend(newBindings, self, null)
)
})
if (prev_length == builder.size()) {
Error.fail(s"Duplicate key ${k} in evaluated object comprehension.", e.pos);
}
case Val.Null(_) => // do nothing
}
}
Expand Down
13 changes: 13 additions & 0 deletions sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,19 @@ object EvaluatorTests extends TestSuite{
test("givenNoDuplicateFieldsInListComprehension2_expectSuccess") {
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"]}""") ==>
"""sjsonnet.Error: Duplicate key A in evaluated object comprehension.
|at .(:1:3)""".stripMargin
}
test("givenDuplicateFieldsInIndirectListComprehension_expectFailure") {
evalErr(
"""local y = { a: "A" };
|local z = { a: "A" };
|{ [x.a]: x for x in [y, z]}""".stripMargin) ==>
"""sjsonnet.Error: Duplicate key A in evaluated object comprehension.
|at .(:3:3)""".stripMargin
}
test("functionEqualsNull") {
eval("""local f(x)=null; f == null""") ==> ujson.False
eval("""local f=null; f == null""") ==> ujson.True
Expand Down

0 comments on commit 1b4a5c0

Please # to comment.