From 7e6e21b5128996ac04e32d1dafcbc35b625bf8f8 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Fri, 11 Sep 2026 22:08:31 +0800 Subject: [PATCH] fix: apply addition semantics to object extension Motivation: Jsonnet defines lhs { body } as syntactic sugar for lhs + { body }. Requiring an object lhs incorrectly rejected valid expressions such as "hello" { x: 1 }, which should stringify the object and concatenate it. Modification: Preserve the object-inheritance fast path. Delegate non-object operands to BinaryOp + using the already evaluated lhs, sharing RHS construction, stringification, and diagnostics without repeating lhs evaluation. Add regression tests for operand types, object bodies, evaluation order, laziness, inheritance, shared body caches, and foldl fallback, including negative goldens and 14 success assertions. Result: Object extension follows addition semantics while retaining the existing strict-mode syntax restriction. All three JVM Scala versions pass 677 tests each (2031 total). Formatting, assembly, and CLI golden checks pass. The success assertions also pass on Go Jsonnet 0.21.0 and 0.22.0. References: https://jsonnet.org/ref/spec.html#desugaring https://github.com/deltarocks/jrsonnet/commit/b5fcc2639d612b852b153474d80b41c537c18e2a --- sjsonnet/src/sjsonnet/Evaluator.scala | 19 ++- .../error.objextend_array_lhs.jsonnet | 1 + .../error.objextend_array_lhs.jsonnet.golden | 2 + .../error.objextend_number_lhs.jsonnet | 1 + .../error.objextend_number_lhs.jsonnet.golden | 2 + .../objextend_nonobject_lhs.jsonnet | 22 ++++ .../objextend_nonobject_lhs.jsonnet.golden | 1 + .../test/src/sjsonnet/EvaluatorTests.scala | 124 ++++++++++++++++++ 8 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet.golden diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index f5631d690..8776d9387 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -795,12 +795,19 @@ class Evaluator( } def visitObjExtend(e: ObjExtend)(implicit scope: ValScope): Val = { - val original = visitExpr(e.base).cast[Val.Obj] - e.ext match { - case ext: ObjBody.MemberList => visitMemberList(e.pos, ext, original) - case ext: ObjBody.ObjComp => visitObjComp(ext, original) - case o: Val.Obj => o.addSuper(e.pos, original) - case _ => Error.fail("Should not have happened", e.pos) + val base = visitExpr(e.base) + base match { + case original: Val.Obj => + e.ext match { + case ext: ObjBody.MemberList => visitMemberList(e.pos, ext, original) + case ext: ObjBody.ObjComp => visitObjComp(ext, original) + case o: Val.Obj => o.addSuper(e.pos, original) + case _ => Error.fail("Should not have happened", e.pos) + } + case _ => + // Desugar to + so stringification, RHS evaluation and errors share the same path. + // Reuse the evaluated base to avoid evaluating the original expression twice. + visitBinaryOp(BinaryOp(e.pos, base, BinaryOp.OP_+, e.ext)) } } diff --git a/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet b/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet new file mode 100644 index 000000000..3bdcb6ef3 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet @@ -0,0 +1 @@ +[1, 2] { x: 1 } diff --git a/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet.golden new file mode 100644 index 000000000..90026184a --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.objextend_array_lhs.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: Unknown binary operation: array + object + at [].(error.objextend_array_lhs.jsonnet:1:8) diff --git a/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet b/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet new file mode 100644 index 000000000..b2b05a094 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet @@ -0,0 +1 @@ +42 { x: 1 } diff --git a/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet.golden new file mode 100644 index 000000000..087dbc45c --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.objextend_number_lhs.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: Unknown binary operation: number + object + at [].(error.objextend_number_lhs.jsonnet:1:4) diff --git a/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet b/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet new file mode 100644 index 000000000..b2f881802 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet @@ -0,0 +1,22 @@ +// Per the Jsonnet spec, lhs { body } is equivalent to lhs + { body }. +local identity(x) = x; +local base = { x: 1, y: self.x }; + +std.assertEqual("hello" { x: 1 }, 'hello{"x": 1}') && +std.assertEqual(identity("hello") {}, "hello" + {}) && +std.assertEqual("" { local x = 2, x: x, y: self.x }, '{"x": 2, "y": 2}') && +std.assertEqual("" { x: 1, nested: { y: $.x } }, '{"nested": {"y": 1}, "x": 1}') && +std.assertEqual("" { hidden:: error "unused", x: 1 }, '{"x": 1}') && +std.assertEqual("" { assert self.x == 1, x: 1 }, '{"x": 1}') && +std.assertEqual("" { x+: 1, hasSuper: "x" in super }, '{"hasSuper": false, "x": 1}') && +std.assertEqual("" { [null]: error "unused" }, "" + {}) && +std.assertEqual("" { [k]: k for k in ["z", "a"] }, '{"a": "a", "z": "z"}') && +std.assertEqual("" { [k]: error "unused" for k in [] }, "" + {}) && +std.assertEqual("" { [k]: error "unused" for k in [null] }, "" + {}) && +std.assertEqual("hello" { x: 1 } { y: 2 }, 'hello{"x": 1}{"y": 2}') && +std.assertEqual(base { x: 2, z: super.x }, { x: 2, y: 2, z: 1 }) && +std.assertEqual( + std.foldl(function(acc, x) acc { x: x }, [1, 2], "hello"), + 'hello{"x": 1}{"x": 2}' +) && +true diff --git a/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet.golden new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/objextend_nonobject_lhs.jsonnet.golden @@ -0,0 +1 @@ +true diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala index 5ba83efc1..ff60c7802 100644 --- a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala +++ b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala @@ -42,6 +42,130 @@ object EvaluatorTests extends TestSuite { eval("std.objectKeysValues({a: error 'unused'})[0].key") ==> ujson.Str("a") assert(evalErr("std.objectKeysValues({a: error 'boom'})[0].value").contains("boom")) } + test("objectExtension") { + test("matchesAddition") { + val bodies = Seq( + "{}", + "{ x: 1 }", + "{ z: 0, a: [true, null, { nested: 'value' }] }", + "{ local value = 2, x: value, y: self.x + 1 }", + "{ x: 1, nested: { y: $.x } }", + "{ hidden:: error 'unused', visible: 1 }", + "{ f(x):: x + 1, value: self.f(1) }", + "{ assert self.x == 1, x: 1 }", + "{ x+: 1, hasSuper: 'x' in super }", + "{ [null]: error 'unused', ['x' + 'y']: 1 }", + "{ [key]: key for key in ['z', 'a'] }", + "{ [key]: error 'unused' for key in [] }", + "{ [key]: error 'unused' for key in [null] }" + ) + for { + body <- bodies + base <- Seq("'hello'", "(function(x) x)('prefix')", "('🙂' + '\\n\\\"')") + preserveOrder <- Seq(false, true) + } { + eval(s"($base) $body", preserveOrder = preserveOrder) ==> + eval(s"($base) + $body", preserveOrder = preserveOrder) + } + } + test("chainedExtensions") { + eval("'prefix' { x: 1 } { y: 2 }") ==> + eval("'prefix' + { x: 1 } + { y: 2 }") + eval("(function(x) x)('prefix') { x: 1 }", strict = true) ==> + ujson.Str("prefix{\"x\": 1}") + // Strict mode deliberately forbids consecutive object bodies as a syntax restriction. + assert( + evalErr("'prefix' {} {}", strict = true).contains("Adjacent object literals not allowed") + ) + } + test("invalidBase") { + for { + base <- Seq("42", "[error 'unused']", "true", "false", "null", "function(x) x") + body <- Seq("{}", "{ x: 1 }", "{ local x = 1, x: x }", "{ [k]: 1 for k in ['x'] }") + } { + evalErr(s"($base) $body").takeWhile(_ != '\n') ==> + evalErr(s"($base) + $body").takeWhile(_ != '\n') + } + } + test("errorOrder") { + for (op <- Seq("", "+")) { + assert( + evalErr(s"(error 'base first') $op { [error 'key second']: 1 }") + .startsWith("sjsonnet.Error: base first") + ) + for (base <- Seq("'prefix'", "42")) { + assert( + evalErr(s"$base $op { [error 'key first']: error 'value later' }") + .startsWith("sjsonnet.Error: key first") + ) + assert( + evalErr(s"$base $op { [k]: 1 for k in error 'source first' }") + .startsWith("sjsonnet.Error: source first") + ) + assert( + evalErr(s"$base $op { [k]: 1 for k in ['x', 'x'] }") + .contains("Duplicate key x") + ) + } + assert( + evalErr(s"42 $op { assert false: 'unused assertion', x: error 'unused value' }") + .startsWith("sjsonnet.Error: Unknown binary operation: number + object") + ) + } + } + test("materializationErrors") { + for (body <- Seq( + "{ assert false: 'assertion' }", + "{ x: error 'visible field' }", + "{ x: super.x }", + "{ f(x): x }" + )) { + evalErr(s"'prefix' $body").takeWhile(_ != '\n') ==> + evalErr(s"'prefix' + $body").takeWhile(_ != '\n') + } + } + test("evaluatedOnce") { + val body = "{ [std.trace('key', 'x')]: std.trace('value', 1) }" + val (value, traces) = evalWithTraces(s"std.trace('base', 'prefix') $body") + value ==> ujson.Str("prefix{\"x\": 1}") + traces.size ==> 3 + assert(traces(0).endsWith("base")) + assert(traces(1).endsWith("key")) + assert(traces(2).endsWith("value")) + } + test("objectBase") { + for (op <- Seq("", "+")) { + eval(s"local base = { x: 1, y: self.x }; (base $op { x: 2, z: super.x })") ==> + ujson.Obj("x" -> 2, "y" -> 2, "z" -> 1) + eval(s"local base = { x: [1] }; base $op { x+: [2] }") ==> + ujson.Obj("x" -> ujson.Arr(1, 2)) + eval(s"local base = { x: 1 }; base $op { [k]: super.x + 1 for k in ['y'] }") ==> + ujson.Obj("x" -> 1, "y" -> 2) + eval(s"local base = { assert self.x == 2, x: 1 }; base $op { x: 2 }") ==> + ujson.Obj("x" -> 2) + eval(s"local base = { x: error 'unused' }; (base $op { y: 2 }).y") ==> + ujson.Num(2) + } + } + test("foldlFallback") { + for (op <- Seq("", "+")) { + eval(s"std.foldl(function(acc, x) acc $op { x: x }, [1, 2], 'prefix')") ==> + ujson.Str("prefix{\"x\": 1}{\"x\": 2}") + eval(s"std.foldl(function(acc, x) acc $op { x: x }, [], 'prefix')") ==> + ujson.Str("prefix") + } + } + test("sharedBodyWithDifferentBaseTypes") { + def program(op: String): String = + s"""local extend(base) = base $op { z: 0, a: 1, hidden:: error 'unused' }; + |[extend('prefix'), extend({ inherited: 2 }), extend('prefix')] + |""".stripMargin + for (preserveOrder <- Seq(false, true)) { + eval(program(""), preserveOrder = preserveOrder) ==> + eval(program("+"), preserveOrder = preserveOrder) + } + } + } test("arrays") { eval("[1, [2, 3], 4][1][0]") ==> ujson.Num(2) eval("([1, 2, 3] + [4, 5, 6])[3]") ==> ujson.Num(4)