cue icon indicating copy to clipboard operation
cue copied to clipboard

Value.Len() reports not supported for type struct

Open verdverm opened this issue 4 years ago • 4 comments

What version of CUE are you using (cue version)?

0.4.0

What did you do?

exec go run len.go
cmp stdout golden.output

-- len.go --
package main

import (
	"fmt"

	"cuelang.org/go/cue/cuecontext"
)

const val = `
a: "a"
b: "b"
`

func main() {
	c := cuecontext.New()
	v := c.CompileString(val)

	k := v.IncompleteKind()
	l := v.Len()

	fmt.Printf("%v %v\n%# v\n", k, l, v)
}

-- golden.stdout --
struct 2
a: "a"
b: "b"

What did you see?

$ go run len.go 
struct _|_ // len not supported for type struct
a: "a"
b: "b"

What did you expect to see?

Per the docs (https://pkg.go.dev/cuelang.org/[email protected]/cue#Value.Len)

For structs it indicates the number of fields

$ go run len.go 
struct 2
a: "a"
b: "b"

The following does report 2

len({a:"a",b:"b"})

(https://cuelang.org/play/?id=vOymXurNiqK#cue@export@cue)

verdverm avatar Oct 29 '21 02:10 verdverm

Confirmed in e8550b89 via the following repro:

exec go mod tidy
exec go run len.go
cmp stdout golden.stdout

-- go.mod --
module mod.com

go 1.16

require cuelang.org/go v0.4.0

-- golden.stdout --
struct 2
a: "a"
b: "b"
-- len.go --
package main

import (
	"fmt"

	"cuelang.org/go/cue/cuecontext"
)

const val = `
a: "a"
b: "b"
`

func main() {
	c := cuecontext.New()
	v := c.CompileString(val)

	k := v.IncompleteKind()
	l := v.Len()

	fmt.Printf("%v %v\n%#v\n", k, l, v)
}

which gives:

> cmp stdout golden.stdout
[diff -stdout +golden.stdout]
-struct _|_ // len not supported for type struct
+struct 2
 a: "a"
 b: "b"

FAIL: /tmp/testscript078088196/repro.txt/script.txt:3: stdout and golden.stdout differ

myitcv avatar Oct 29 '21 04:10 myitcv

Arguably, supporting len({}) was/is a mistake. What does it mean?

Do we count, definitions? Hidden fields? It is a bit unclear.

mpvl avatar Oct 29 '21 13:10 mpvl

Do we count, definitions? Hidden fields? It is a bit unclear.

In the spec it's defined as:

number of distinct data fields, excluding optional

But I would tend to agree, it's effectively arbitrary that it is defined as such, and not much consolation to someone who requires the count to include optional fields. And of course that doesn't even consider hidden fields.

For that reason it feels like we should instead remove this builtin and corresponding functionality within the Value.Len() API.

myitcv avatar Oct 29 '21 14:10 myitcv

I agree that it seems ambiguous and arbitrary without also taking ...Option and probably lacks good use cases. Len could be achieved by iterating on Fields while keeping counters.

@myitcv Do you mean remove both for structs or all together?

  • len([...]) in CUE seems reasonable to keep
  • Value.Len() seems less useful if it only applies to List and bytes. For bytes, would Go's native len be sufficient (requiring something like len(Value.Bytes()) and then just have a List.Len()?

verdverm avatar Oct 29 '21 16:10 verdverm