Skip to content

Commit

Permalink
Fix field visibility of object comprehension fields to inherit
Browse files Browse the repository at this point in the history
This should make C++ Jsonnet match the existing behaviour of Go-Jsonnet.

Object comprehensions do not support differing field visibility, that is
an object comprehension with a "hidden" or "forced-visible" field such as
`{[k]::1 for k in ["x"]}` is rejected with a syntax error.

However, intuitively the `{[key_expr]: value_expr for x in ...}` syntax
seems like it should behave similarly to a normal (non-comprehension) object
that uses default field visibility. Default field visibility is to 'inherit'
visibility when merging objects with the + operator, and this is the existing
behaviour of Go-Jsonnet.

Example case:

./jsonnet -e '{"one":: "base"} + {[k]: "derived" for k in ["one"]}'

Before this commit, Go-Jsonnet output:
{ }

Before this commit, C++ Jsonnet output:
{ "one": "derived" }

Bug report:
#1111
  • Loading branch information
johnbartholomew committed Mar 23, 2024
1 parent 55eafec commit 4b914fb
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/vm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ class Interpreter {

} else if (auto *obj = dynamic_cast<const HeapComprehensionObject *>(obj_)) {
for (const auto &f : obj->compValues)
r[f.first] = ObjectField::VISIBLE;
r[f.first] = ObjectField::INHERIT;
}
return r;
}
Expand Down
5 changes: 5 additions & 0 deletions test_suite/object.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ local obj = {

std.assertEqual(obj, { ["f" + x + y + z]: { x: x, y: y, z: z } for x in [1, 2, 3] for y in [1, 4, 6] if x + 2 < y for z in [true, false] }) &&

// Tests for #1111 - object comprehension fields should use "inherit" visibility.
std.assertEqual(std.objectFields({ x: 1 } + { [k]: 2 for k in ['x'] }), ['x']) &&
std.assertEqual(std.objectFields({ x:: 1 } + { [k]: 2 for k in ['x'] }), []) &&
std.assertEqual(std.objectFields({ x::: 1 } + { [k]: 2 for k in ['x'] }), ['x']) &&

std.assertEqual({ f: { foo: 7, bar: 1 } { [self.name]+: 3, name:: "foo" }, name:: "bar" },
{ f: { foo: 7, bar: 4 } }) &&

Expand Down
5 changes: 5 additions & 0 deletions test_suite/object.jsonnet.fmt.golden
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ local obj = {

std.assertEqual(obj, { ['f' + x + y + z]: { x: x, y: y, z: z } for x in [1, 2, 3] for y in [1, 4, 6] if x + 2 < y for z in [true, false] }) &&

// Tests for #1111 - object comprehension fields should use "inherit" visibility.
std.assertEqual(std.objectFields({ x: 1 } + { [k]: 2 for k in ['x'] }), ['x']) &&
std.assertEqual(std.objectFields({ x:: 1 } + { [k]: 2 for k in ['x'] }), []) &&
std.assertEqual(std.objectFields({ x::: 1 } + { [k]: 2 for k in ['x'] }), ['x']) &&

std.assertEqual({ f: { foo: 7, bar: 1 } { [self.name]+: 3, name:: 'foo' }, name:: 'bar' },
{ f: { foo: 7, bar: 4 } }) &&

Expand Down

0 comments on commit 4b914fb

Please sign in to comment.