cue: decode empty list as empty slice instead of slice-typed nil

[bozaro]

An empty slice and a slice-typed nil have the same behaviour in most cases, but sometimes there are differences.
For example, when marshaling in json.Marshal.
From my point of view, it is more correct to return an empty slice for an empty list instead of slice-typed nil.

https://go.dev/play/p/PfVuusup6UO

Source:

package main

import (
	"encoding/json"
	"fmt"
)

func main() {
	var a, b []interface{}
	a = []interface{}{}
	ja, _ := json.Marshal(a)
	jb, _ := json.Marshal(b)
	fmt.Println(string(ja))
	fmt.Println(string(jb))
}

Output:

[]
null

[mvdan]

Is there a github issue that this PR is fixing? I would suggest filing an issue first if not, because this is a subtle change in behavior that we need to agree is a good idea first.

[bozaro]

Is there a github issue that this PR is fixing?

I don't find related github issue, but at least two of my colleagues mentioned this problem before I encountered it.

because this is a subtle change in behavior that we need to agree is a good idea first.

I will be very surprised if the test case (current behaviour):

	{
		value: `[]`,
		dst:   new(interface{}),
		want:  ([]interface{})(nil),
	}, {
		value: `[]`,
		dst:   new([]interface{}),
		want:  []interface{}{},
	}, {
		value: `[]`,
		dst:   new([]int),
		want:  []int{},
	}

will be more preferable than (new behaviour):

	{
		value: `[]`,
		dst:   new(interface{}),
		want:  []interface{}{},
	}, {
		value: `[]`,
		dst:   new([]interface{}),
		want:  []interface{}{},
	}, {
		value: `[]`,
		dst:   new([]int),
		want:  []int{},
	}

[bozaro]

That is, now an empty list turns into a slice-typed nil only on decode to interface{} type.
In other cases we got empty slice.

[mvdan]

I see. I think I agree with that, for the sake of consistency. Most other decoders in Go, like encoding/json, also do that:

To unmarshal a JSON array into a slice, Unmarshal resets the slice length to zero and then appends each element to the slice. As a special case, to unmarshal an empty JSON array into a slice, Unmarshal replaces the slice with a new empty slice.

I'll check with Marcel to see if he disagrees or if there's a factor I'm missing.

For the future, I would still recommend filing an issue first and following the template. Discussions like this one get lost or forgotten if they happen as part of a patch.

[myitcv]

@bozaro thank you for raising this PR.

If I can just, however, emphasise @mvdan's request:

recommend filing an issue first and following the template

The issue template for the CUE project is, like the Go project, carefully designed to help with the triaging process to make it easier for everyone (including project maintainers) to understand the nature of a bug or feature request. For a bug report, the questions What did you do?, What did you expect to see?, and What did you see instead? help to illicit as many of the facts as possible in a standard form. That standard form is again designed to make exchanges from all sides as efficient and effective as possible, something that is especially important for an open source project. These motivations and others are also covered in the contribution docs.

Thanks again for contributing to the CUE project.

[bozaro]

Is there a chance to get this change in 0.6.0?

[mvdan]

We had to prioritise regressions and severe bugs like panics for v0.6.0, so I'm afraid not. We'll jump to v0.7 as soon as the final release is out, hopefully tomorrow, so I'll certainly look at this with Marcel soon. We would have of course liked to include more fixes into v0.6.0, but the more we try to include, the longer the release will take to be ready :)

[mvdan]

Looked at this with @mpvl and we agree it's a good change :) Importing to Gerrit.