std.mergePatch should create resulting object fields at Normal visibi…

…lity, not Unhide (#255)

This PR changes the default visibility of fields in objects created by
`std.mergePatch`: previously they were at `Unhide` visibility,
equivalent to the `:::` operator, but as of this PR they are now at
`Normal` standard visibility.

Concretely, previously 

```sjsonnet
{a:: 0} + std.mergePatch({a: 1}, {})
```

would return `{a: 1}` but after this patch it returns `{a:: 1)`, i.e. it
preserves the hidden field.

It turns out that v0.20.0 of google/jsonnet and google/go-jsonnet also
differ in their behavior here. That, in turn, is due to a behavior
difference in the default visibility of fields in object comprehension
results: jsonnet marks object comprehension fields as forced-visible,
while go-jsonnet marks them as inherited visibility; see
google/jsonnet#1111.

jsonnet accepted google/jsonnet#1140 to match
go-jsonnet's behavior.

Note that sjsonnet already matches go-jsonnet's behavior for object
comprehensions: we only have a difference in `std.mergePatch` because
our current implementation explicitly hardcodes Unhide visibility.

This PR changes that mergePatch-specific behavior to match the current
go-jsonnet (and future jsonnet) behavior.

sjsonnet/src/sjsonnet/Std.scala

@@ -922,7 +922,7 @@ class Std(private val additionalNativeFunctions: Map[String, Val.Builtin] = Map.
     builtin(Range),
     builtin("mergePatch", "target", "patch"){ (pos, ev, target: Val, patch: Val) =>
       val mergePosition = pos
-      def createMember(v: => Val) = new Val.Obj.Member(false, Visibility.Unhide) {
+      def createMember(v: => Val) = new Val.Obj.Member(false, Visibility.Normal) {
         def invoke(self: Val.Obj, sup: Val.Obj, fs: FileScope, ev: EvalScope): Val = v
       }
       def recPair(l: Val, r: Val): Val = (l, r) match{

sjsonnet/test/src/sjsonnet/StdMergePatchTests.scala

@@ -91,5 +91,22 @@ object StdMergePatchTests extends TestSuite {
       // It's also lost in nested fields:
       eval("{a: {b: 1}} + std.mergePatch({a: {b +: 2}}, {})") ==> ujson.Obj("a" -> ujson.Obj("b" -> 2))
     }
+
+    test("default visibility of resulting fields") {
+      // In v0.20.0 of google/jsonnet, the following returned `{a: 1}`, not `{}`:
+      eval("{a:: 0} + std.mergePatch({a: 1}, {})") ==> ujson.Obj()
+      eval("({a:: 0} + std.mergePatch({a: 1}, {})).a") ==> ujson.Num(1)
+      // However, go-jsonnet v0.20.0 returned `{}`. This difference arose because the pure-jsonnet
+      // implementation of `std.mergePatch` used object comprehensions in its implementation and
+      // the two implementations differed in the default visibility of fields in the reuslting
+      // object. See https://github.com/google/jsonnet/issues/1111
+      // google/jsonnet merged a change to match go-jsonnet's behavior in a future version:
+      // https://github.com/google/jsonnet/pull/1140
+      // Interestingly, sjsonnet already matched go-jsonnet's behavior for object comprehensions
+      // because the following already returned `{}`; our previous behavior difference was only
+      // for mergePatch results:
+      eval("""{a:: 0} + {[a]: 1 for a in ["a"]}""") ==> ujson.Obj()
+      eval("""({a:: 0} + {[a]: 1 for a in ["a"]}).a""") ==> ujson.Num(1)
+    }
   }
 }