-
Notifications
You must be signed in to change notification settings - Fork 14
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
wit/bindgen,cm: marshal & unmarshal for record types #225
Conversation
Signed-off-by: Lucas Fontes <[email protected]>
Amazing. This looks great so far. Thanks! |
@@ -679,13 +679,13 @@ func (g *generator) recordRep(file *gen.File, dir wit.Direction, r *wit.Record, | |||
exported := len(goName) == 0 || token.IsExported(goName) | |||
var b strings.Builder | |||
b.WriteString("struct {\n") | |||
stringio.Write(&b, "_ ", file.Import(g.opts.cmPackage), ".HostLayout") | |||
stringio.Write(&b, "_ ", file.Import(g.opts.cmPackage), ".HostLayout", "`json:\"-\"`") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget the space before the backtick, or just have a single string with HostLayout and the tag.
@@ -881,6 +899,39 @@ func (g *generator) variantRep(file *gen.File, dir wit.Direction, t *wit.TypeDef | |||
stringio.Write(&b, "return ", stringsName, "[v.Tag()]\n") | |||
b.WriteString("}\n\n") | |||
|
|||
file.Import("encoding/json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to capture the name of the import. It might not be json.
file.Import("encoding/json") | ||
b.WriteString(formatDocComments("MarshalJSON implements [json.Marshaler].", true)) | ||
stringio.Write(&b, "func (v ", goName, ") MarshalJSON() ([]byte, error) {\n") | ||
stringio.Write(&b, "ret := make(map[string]any)\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there might be a better way to do this.
Possibly by putting MarshalJSON and UnmarshalJSON in package cm, and generate only the minimal mapping from case name to case type.
@@ -9,6 +12,20 @@ type List[T any] struct { | |||
list[T] | |||
} | |||
|
|||
func (l List[T]) MarshalJSON() ([]byte, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods should be on the inner list[T] struct.
@@ -8,6 +12,27 @@ type Option[T any] struct { | |||
option[T] | |||
} | |||
|
|||
// MarshalJSON implements the json.Marshaler interface for [Option]. | |||
func (o Option[T]) MarshalJSON() ([]byte, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods should be on the inner struct.
|
||
// UnmarshalJSON unmarshals the Option from JSON. | ||
func (o *Option[T]) UnmarshalJSON(buf []byte) error { | ||
if len(buf) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also check if buf == null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 the null
story here is interesting. will push it up as part of the breaking this PR into many
going to close this PR and move over to #239 |
Adds
json
struct tags to Records, based on wit names.Also adds
MarshalJSON
&UnmarshalJSON
to:Example, encoding a complex record type:
and filling it up:
Without this Pull Request:
With this Pull Request:
Tuple handling
Tuples are encoded as json arrays with explicit
null
s.Variant handling
Variants are encoded as json dictionaries, so they can carry the variant data.
Option handling
Options rely on explicit
null
for the "None" case.If this direction seems reasonable I will add:
cm.Result