Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cm: Implement json Marshal/Unmarshal for List type #266

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lxfontes
Copy link
Member

@lxfontes lxfontes commented Dec 9, 2024

Relates to #239

This will also respect json nulls as per https://pkg.go.dev/encoding/json#Unmarshaler

cm/list_test.go Outdated Show resolved Hide resolved
cm/list.go Outdated
// We override that behavior so all int types have the same serialization format.
// []uint8{1,2,3} -> [1,2,3]
// []uint32{1,2,3} -> [1,2,3]
if byteArray, ok := any(s).([]byte); ok {
Copy link
Collaborator

@ydnar ydnar Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will miss List[T] where T is a named uint8 type.

Try something like this and convert the underlying slice: https://go.dev/play/p/MSlhH5LesNN

Update: updated link.

package main

import (
	"encoding/json"
	"fmt"
	"unsafe"
)

func main() {
	j, _ := json.Marshal([]uint8{0, 1, 2, 3})
	fmt.Println(string(j))

	j, _ = json.Marshal(sliceOf([]uint8{0, 1, 2, 3}))
	fmt.Println(string(j))
}

type slice[T any] []entry[T]

func sliceOf[S ~[]E, E any](s S) slice[E] {
	return *(*slice[E])(unsafe.Pointer(&s))
}

type entry[T any] [1]T

func (v entry[T]) MarshalJSON() ([]byte, error) {
	return json.Marshal(v[0])
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! was avoiding the unsafe.Pointer route but totally forgot about type aliases.

@@ -14,3 +18,304 @@ func TestListMethods(t *testing.T) {
t.Errorf("got (%s) != want (%s)", string(got), string(want))
}
}

type listTestItem struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is all this machinery necessary?

If List[T] implements json.Marshaler and json.Unmarshaler, then the test cases can have these lists inline.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listTestItem is to test structs.

The machinery is necessary to generalize the Slice() method & carry the type information to reflect.DeepEqual. It's either this or unrolling into many smaller test functions per type.

While List implements Marshaler/Unmarshaler, accessing the underlying slice is thru typed Slice() []T ( not a common interface ). So the options are:

  • make a generic wrapper ( what I did )
  • cast it ( which requires knowing which type to cast to )

Unmarshal is where it is needed, and I used it in Marshal as well for consistency/less typing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants