Skip to content

Commit

Permalink
Merge pull request #182 from grafana/or-null-case
Browse files Browse the repository at this point in the history
gocode: Generate correct go types in unions will `null`
  • Loading branch information
sam boyer committed Jul 12, 2023
2 parents f114418 + e78b342 commit 80c1181
Show file tree
Hide file tree
Showing 6 changed files with 515 additions and 12 deletions.
31 changes: 22 additions & 9 deletions assignable.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,22 @@ func assignable(sch cue.Value, T interface{}) error {
var check, checkstruct, checklist, checkscalar checkfn

check = func(gval, sval cue.Value, p cue.Path) {
// At least for now, we have to deal with these unhelpful *null
// appearing in the encoding of pointer types.
gval = stripLeadNull(gval)

sk, gk := sval.IncompleteKind(), gval.IncompleteKind()
schemaHadNull, goHadNull := sk&cue.NullKind != 0, gk&cue.NullKind != 0
sk &^= cue.NullKind
gk &^= cue.NullKind

ogval := gval
if goHadNull {
// At least for now, we have to deal with these unhelpful *null
// appearing in the encoding of pointer types.
//
// We can't do the same for the schema side, because this null stripper
// relies on the fact that all actual Go type declarations will come across
// as a single value, without any disjunctions.
gval = stripLeadNull(gval)
}

// strict equality _might_ be too restrictive? But it's better to start there
if sk != gk && gk != cue.TopKind {
errs[p.String()] = fmt.Errorf("%s: is kind %s in schema, but kind %s in Go type", p, sk, gk)
Expand All @@ -104,10 +115,12 @@ func assignable(sch cue.Value, T interface{}) error {
switch sk {
case cue.ListKind:
checklist(gval, sval, p)
case cue.FloatKind, cue.IntKind, cue.StringKind, cue.BytesKind, cue.BoolKind:
checkscalar(gval, sval, p)
case cue.NumberKind:
checkscalar(gval, sval, p)
case cue.FloatKind, cue.IntKind, cue.StringKind, cue.BytesKind, cue.BoolKind, cue.NumberKind:
if schemaHadNull {
checkscalar(ogval, sval, p)
} else {
checkscalar(gval, sval, p)
}
case cue.StructKind:
checkstruct(gval, sval, p)
case cue.NullKind:
Expand Down Expand Up @@ -270,7 +283,7 @@ func assignable(sch cue.Value, T interface{}) error {
func stripLeadNull(v cue.Value) cue.Value {
if op, vals := v.Expr(); op == cue.OrOp {
// Walk over the vals, because there may be more than one null (e.g. omitempty +
// slice/map type)
// slice/map type).
for i := 0; i < len(vals); i++ {
if vals[i].Null() != nil {
return vals[i]
Expand Down
16 changes: 16 additions & 0 deletions assignable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,22 @@ func TestAssignable(t *testing.T) {
}
`, strconv.IntSize, strconv.IntSize),
},
"or-null": {
T: &struct {
Both *string `json:"both,omitempty"`
NoOmit *string `json:"noOmit"`
// This case is the ugly ambiguous one - is the user saying that an empty string
// should be serialized as an absent field, but a nil pointer be serialized as
// null? WAAAAAAAT
Optional *string `json:"optional"`
}{},
cue: `typ: {
both: string | null
noOmit: string | null
optional?: string | null
}
`,
},
}

for name, tst := range tt {
Expand Down
1 change: 1 addition & 0 deletions encoding/gocode/gocode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"cuelang.org/go/cue/cuecontext"
copenapi "cuelang.org/go/encoding/openapi"

"github.com/grafana/thema"
"github.com/grafana/thema/encoding/openapi"
"github.com/grafana/thema/internal/txtartest/bindlin"
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.19
// contains openapi encoder fixes. commented out because impact for
// most thema CLI uses is minimal, and others (e.g. grafana/grafana)
// can hoist this themselves if they need it
replace cuelang.org/go => github.com/sdboyer/cue v0.5.0-beta.2.0.20221218111347-341999f48bdb
replace cuelang.org/go => github.com/sdboyer/cue v0.5.0-beta.2.0.20230712135403-bdc4772ae055

require (
cuelang.org/go v0.5.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/schollz/closestmatch v2.1.0+incompatible/go.mod h1:RtP1ddjLong6gTkbtmuhtR2uUrrJOpYzYRvbcPAid+g=
github.com/sdboyer/cue v0.5.0-beta.2.0.20221218111347-341999f48bdb h1:X6XJsprVDQnlG4vT5TVb+cRlGMU78L/IKej8Q6SDFGY=
github.com/sdboyer/cue v0.5.0-beta.2.0.20221218111347-341999f48bdb/go.mod h1:okjJBHFQFer+a41sAe2SaGm1glWS8oEb6CmJvn5Zdws=
github.com/sdboyer/cue v0.5.0-beta.2.0.20230712135403-bdc4772ae055 h1:ilCOPMIGucstzXKgCVzAT8g6tu+Ejyk0J1HhXZN2xm0=
github.com/sdboyer/cue v0.5.0-beta.2.0.20230712135403-bdc4772ae055/go.mod h1:okjJBHFQFer+a41sAe2SaGm1glWS8oEb6CmJvn5Zdws=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8=
github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I=
Expand Down
Loading

0 comments on commit 80c1181

Please sign in to comment.