diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index c7683b18..ae1bd415 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -425,12 +425,39 @@ object Val { var i = chain.length - 1 while (i >= 0) { val s = chain(i) - val filteredExcludedKeys = if (s.excludedKeys != null) { + var members = s.getValue0 + var filteredExcludedKeys = if (s.excludedKeys != null) { Util.intersect(s.excludedKeys, keysInThisChain) } else null + + // If this object has excluded keys that the LHS provides as visible, + // re-introduce them with synthetic members that delegate to the LHS. + // This ensures `lhs + removeKey(rhs, k)` preserves lhs's visible key `k`. + if (filteredExcludedKeys != null) { + val iter = filteredExcludedKeys.iterator() + while (iter.hasNext) { + val key = iter.next() + if (lhs.containsVisibleKey(key)) { + if (members eq s.getValue0) { + members = new util.LinkedHashMap[String, Obj.Member](s.getValue0) + } + val capturedKey = key + members.put( + key, + new Obj.Member(false, Visibility.Normal, deprecatedSkipAsserts = true) { + def invoke(self: Val.Obj, sup: Val.Obj, fs: FileScope, ev: EvalScope): Val = + lhs.value(capturedKey, pos, self)(ev) + } + ) + iter.remove() + } + } + if (filteredExcludedKeys.isEmpty) filteredExcludedKeys = null + } + current = new Val.Obj( s.pos, - s.getValue0, + members, false, s.triggerAsserts, current, @@ -498,30 +525,17 @@ object Val { val chain = builder.result() val chainLength = chain.length - // Collect all excluded keys, reusing the set directly when only one source has exclusions - var exclusionSet: java.util.Set[String] = null - var multipleExclusions = false - for (s <- chain) { - val keys = s.excludedKeys - if (Util.isNotEmpty(keys)) { - if (exclusionSet == null) { - exclusionSet = keys - } else { - if (!multipleExclusions) { - val merged = new util.HashSet[String](exclusionSet.size + keys.size) - merged.addAll(exclusionSet) - exclusionSet = merged - multipleExclusions = true - } - exclusionSet.asInstanceOf[util.HashSet[String]].addAll(keys) - } - } - } - - // Iterate root-first (reverse of collection order) and populate the mapping + // Iterate root-first (reverse of collection order) and populate the mapping. + // Each object's excludedKeys removes keys gathered from lower levels (closer to root), + // but objects above it in the chain can re-introduce those keys via their own members. var i = chainLength - 1 while (i >= 0) { - gatherKeysForSingle(chain(i), exclusionSet, mapping) + val s = chain(i) + if (Util.isNotEmpty(s.excludedKeys)) { + val iter = s.excludedKeys.iterator() + while (iter.hasNext) mapping.remove(iter.next()) + } + gatherKeysForSingle(s, null, mapping) i -= 1 } } diff --git a/sjsonnet/test/src/sjsonnet/StdObjectRemoveKeyTests.scala b/sjsonnet/test/src/sjsonnet/StdObjectRemoveKeyTests.scala new file mode 100644 index 00000000..eb277250 --- /dev/null +++ b/sjsonnet/test/src/sjsonnet/StdObjectRemoveKeyTests.scala @@ -0,0 +1,75 @@ +package sjsonnet + +import utest._ +import TestUtils.eval + +object StdObjectRemoveKeyTests extends TestSuite { + + def tests: Tests = Tests { + + test("basic removal") { + eval("std.objectRemoveKey({a: 1, b: 2}, 'a')") ==> ujson.Obj("b" -> 2) + eval("std.objectRemoveKey({a: 1, b: 2}, 'b')") ==> ujson.Obj("a" -> 1) + eval("std.objectRemoveKey({a: 1}, 'a')") ==> ujson.Obj() + } + + test("removing non-existent key is a no-op") { + eval("std.objectRemoveKey({a: 1}, 'b')") ==> ujson.Obj("a" -> 1) + } + + test("re-adding removed key via object inheritance") { + eval("""std.objectRemoveKey({foo: "bar"}, "foo") + {foo: "bar2"}""") ==> + ujson.Obj("foo" -> "bar2") + + eval("""std.objectRemoveKey({a: 1, b: 2}, "a") + {a: 3}""") ==> + ujson.Obj("b" -> 2, "a" -> 3) + + eval("""std.objectRemoveKey({a: 1, b: 2, c: 3}, "b") + {b: 99, d: 4}""") ==> + ujson.Obj("a" -> 1, "c" -> 3, "b" -> 99, "d" -> 4) + } + + test("double removal then re-add") { + eval( + """std.objectRemoveKey(std.objectRemoveKey({a: 1, b: 2, c: 3}, "a"), "b") + {a: 10}""" + ) ==> ujson.Obj("c" -> 3, "a" -> 10) + } + + test("remove then re-add then remove again") { + eval( + """std.objectRemoveKey(std.objectRemoveKey({foo: 1, bar: 2}, "foo") + {foo: 3}, "foo")""" + ) ==> ujson.Obj("bar" -> 2) + } + + test("internal super chain preserved after removal") { + eval("std.objectRemoveKey({a: 1} + {b: super.a}, 'a')") ==> ujson.Obj("b" -> 1) + } + + test("external inheritance preserved after removal") { + eval("{a: 1} + std.objectRemoveKey({b: super.a}, 'a')") ==> + ujson.Obj("a" -> 1, "b" -> 1) + } + + test("LHS key preserved when RHS removes it") { + eval("""{a: 1} + std.objectRemoveKey({a: 2}, "a")""") ==> + ujson.Obj("a" -> 1) + + eval("""{a: 1} + std.objectRemoveKey({a: 2, b: 3}, "a")""") ==> + ujson.Obj("a" -> 1, "b" -> 3) + + eval("""{a: 10} + std.objectRemoveKey({a: 1} + {b: super.a}, "a")""") ==> + ujson.Obj("a" -> 10, "b" -> 1) + } + + test("containsKey and objectFields reflect re-added key") { + eval( + """local r = std.objectRemoveKey({a: 1}, "a") + {a: 2}; + |std.objectHas(r, "a")""".stripMargin + ) ==> ujson.True + + eval( + """local r = std.objectRemoveKey({a: 1}, "a") + {a: 2}; + |std.objectFields(r)""".stripMargin + ) ==> ujson.Arr("a") + } + } +}