Skip to content

Commit

Permalink
Generate getters for optional primitives (#325)
Browse files Browse the repository at this point in the history
The majority use case for accessing values of optional primitives is to
dereference it as-is or use the default/zero value if not.

This modifies code generation to include accessor functions for optional
primitive fields which return non-pointer versions of those fields.

So instead of,

    var foo string
    if s.Foo != nil {
        foo = *s.Foo
    }
    doSomething(foo)

Users can do,

    doSomething(s.GetFoo())

This should also ease migration for users coming from Apache Thrift.

Resolves #295

Depends on #324
  • Loading branch information
abhinav committed Aug 28, 2017
1 parent 45d4a33 commit 54c8987
Show file tree
Hide file tree
Showing 21 changed files with 684 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Releases
v1.6.0 (unreleased)
-------------------

- Structs now include getter functions for primitive types.
- Fields of generated struct types are now tagged with easyjson-compatible
`required` tags.
- Fixed code generation bug for default values of fields with a `typedef`
Expand Down
74 changes: 74 additions & 0 deletions gen/collision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,3 +569,77 @@ func roundTrip(t *testing.T, x thriftType, msg string) thriftType {
assert.Equal(t, x, gotX.Interface(), "FromWire: %v", msg)
return gotX.Interface().(thriftType)
}

func TestCollisionAccessors(t *testing.T) {
t.Run("UnionCollision", func(t *testing.T) {
t.Run("bool", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.UnionCollision{CollisionField: ptr.Bool(true)}
assert.True(t, u.GetCollisionField())
})
t.Run("unset", func(t *testing.T) {
var u tc.UnionCollision
assert.False(t, u.GetCollisionField())
})
})

t.Run("string", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.UnionCollision{CollisionField2: ptr.String("foo")}
assert.Equal(t, "foo", u.GetCollisionField2())
})
t.Run("unset", func(t *testing.T) {
var u tc.UnionCollision
assert.Equal(t, "", u.GetCollisionField2())
})
})
})

t.Run("UnionCollision2", func(t *testing.T) {
t.Run("bool", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.UnionCollision2{CollisionField: ptr.Bool(true)}
assert.True(t, u.GetCollisionField())
})
t.Run("unset", func(t *testing.T) {
var u tc.UnionCollision2
assert.False(t, u.GetCollisionField())
})
})

t.Run("string", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.UnionCollision2{CollisionField2: ptr.String("foo")}
assert.Equal(t, "foo", u.GetCollisionField2())
})
t.Run("unset", func(t *testing.T) {
var u tc.UnionCollision2
assert.Equal(t, "", u.GetCollisionField2())
})
})
})

t.Run("AccessorConflict", func(t *testing.T) {
t.Run("name", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.AccessorConflict{Name: ptr.String("foo")}
assert.Equal(t, "foo", u.GetName())
})
t.Run("unset", func(t *testing.T) {
u := tc.AccessorConflict{GetName2: ptr.String("bar")}
assert.Equal(t, "", u.GetName())
})
})

t.Run("get_name", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
u := tc.AccessorConflict{GetName2: ptr.String("foo")}
assert.Equal(t, "foo", u.GetGetName2())
})
t.Run("unset", func(t *testing.T) {
u := tc.AccessorConflict{Name: ptr.String("bar")}
assert.Equal(t, "", u.GetGetName2())
})
})
})
}
29 changes: 29 additions & 0 deletions gen/enum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"fmt"
"testing"

tec "go.uber.org/thriftrw/gen/testdata/enum_conflict"
te "go.uber.org/thriftrw/gen/testdata/enums"
"go.uber.org/thriftrw/wire"

Expand Down Expand Up @@ -328,3 +329,31 @@ func TestUnmarshalTextReturnsErrorOnInvalidValue(t *testing.T) {
err := v.UnmarshalText([]byte("blah"))
assert.Error(t, err)
}

func TestEnumAccessors(t *testing.T) {
t.Run("Records", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
rt := tec.RecordTypeName
r := tec.Records{RecordType: &rt}
assert.Equal(t, tec.RecordTypeName, r.GetRecordType())
})

t.Run("unset", func(t *testing.T) {
var r tec.Records
assert.Equal(t, tec.DefaultRecordType, r.GetRecordType())
})
})

t.Run("StructWithOptionalEnum", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
e := te.EnumDefaultBar
s := te.StructWithOptionalEnum{E: &e}
assert.Equal(t, te.EnumDefaultBar, s.GetE())
})

t.Run("unset", func(t *testing.T) {
var s te.StructWithOptionalEnum
assert.Equal(t, te.EnumDefaultFoo, s.GetE())
})
})
}
36 changes: 36 additions & 0 deletions gen/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ func (f fieldGroupGenerator) Generate(g Generator) error {
return err
}

if err := f.PrimitiveAccessors(g); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -440,3 +444,35 @@ func (f fieldGroupGenerator) Equals(g Generator) error {
}
`, f)
}

func (f fieldGroupGenerator) PrimitiveAccessors(g Generator) error {
fieldsAndAccessors := NewNamespace()
return g.DeclareFromTemplate(
`
<$v := newVar "v">
<$o := newVar "o">
<$name := .Name>
<range .Fields>
<$fname := goName .>
<reserveFieldOrMethod $fname>
<reserveFieldOrMethod (printf "Get%v" $fname)>
<if and (not .Required) (isPrimitiveType .Type)>
func (<$v> *<$name>) Get<$fname>() (<$o> <typeReference .Type>) {
if <$v>.<$fname> != nil {
return *<$v>.<$fname>
}
<if .Default><$o> = <constantValue .Default .Type><end>
return
}
<end>
<end>
`, f,
TemplateFunc("constantValue", ConstantValue),
TemplateFunc("reserveFieldOrMethod", func(name string) (string, error) {
// we return an empty string for the sake of the templating system
err := fieldsAndAccessors.Reserve(name)
return "", err
}),
)
}
61 changes: 61 additions & 0 deletions gen/struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
ts "go.uber.org/thriftrw/gen/testdata/structs"
td "go.uber.org/thriftrw/gen/testdata/typedefs"
tu "go.uber.org/thriftrw/gen/testdata/unions"
"go.uber.org/thriftrw/ptr"
"go.uber.org/thriftrw/wire"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1584,3 +1585,63 @@ func TestStructValidation(t *testing.T) {
}
}
}

func TestStructAccessors(t *testing.T) {
t.Run("DoesNotExistException", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
err := tx.DoesNotExistException{Error2: ptr.String("foo")}
assert.Equal(t, "foo", err.GetError2())
})
t.Run("unset", func(t *testing.T) {
var err tx.DoesNotExistException
assert.Equal(t, "", err.GetError2())
})
})
t.Run("DefaultsStruct", func(t *testing.T) {
t.Run("RequiredPrimitive", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
s := ts.DefaultsStruct{RequiredPrimitive: ptr.Int32(42)}
assert.Equal(t, int32(42), s.GetRequiredPrimitive())
})
t.Run("unset", func(t *testing.T) {
var s ts.DefaultsStruct
assert.Equal(t, int32(100), s.GetRequiredPrimitive())
})
})

t.Run("OptionalPrimitive", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
s := ts.DefaultsStruct{OptionalPrimitive: ptr.Int32(100)}
assert.Equal(t, int32(100), s.GetOptionalPrimitive())
})
t.Run("unset", func(t *testing.T) {
var s ts.DefaultsStruct
assert.Equal(t, int32(200), s.GetOptionalPrimitive())
})
})

t.Run("RequiredEnum", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
e := te.EnumDefaultFoo
s := ts.DefaultsStruct{RequiredEnum: &e}
assert.Equal(t, te.EnumDefaultFoo, s.GetRequiredEnum())
})
t.Run("unset", func(t *testing.T) {
var s ts.DefaultsStruct
assert.Equal(t, te.EnumDefaultBar, s.GetRequiredEnum())
})
})

t.Run("OptionalEnum", func(t *testing.T) {
t.Run("set", func(t *testing.T) {
e := te.EnumDefaultFoo
s := ts.DefaultsStruct{OptionalEnum: &e}
assert.Equal(t, te.EnumDefaultFoo, s.GetOptionalEnum())
})
t.Run("unset", func(t *testing.T) {
var s ts.DefaultsStruct
assert.Equal(t, te.EnumDefaultBaz, s.GetOptionalEnum())
})
})
})
}
4 changes: 2 additions & 2 deletions gen/testdata/collision/idl.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 54c8987

Please sign in to comment.