Merge pull request #447 from thriftrw/syrie/release

Release v1.24.0 (#447)

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [1.24.0] - 2020-06-18
+### Added
+- Generate `Default_*` methods that construct Thrift structs with defined 
+  default values pre-populated.  
+
+### Changed  
+- gen: Redefine Options.Plugin as a struct usable outside go.uber.org/thriftrw.
+
+### Fixed
+- Serializing generated Thrift objects no longer mutates them with default
+  values for unspecified fields.
+
 ## [1.23.0] - 2020-03-31
 ### Added
 - Support opting out of the `omitempty` JSON option by adding `!omitempty` to the
@@ -330,6 +342,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 - Initial release.
 
+[1.24.0]: https://github.com/thriftrw/thriftrw-go/compare/v1.23.0...v1.24.0
 [1.23.0]: https://github.com/thriftrw/thriftrw-go/compare/v1.22.0...v1.23.0
 [1.22.0]: https://github.com/thriftrw/thriftrw-go/compare/v1.21.0...v1.22.0
 [1.21.0]: https://github.com/thriftrw/thriftrw-go/compare/v1.20.2...v1.21.0

gen/collision_test.go

@@ -97,7 +97,7 @@ func TestStruct(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		roundTrip(t, tt.x, tt.desc)
+		roundTrip(t, tt.x, tt.x, tt.desc)
 	}
 }
 
@@ -433,15 +433,16 @@ func TestConstant(t *testing.T) {
 }
 
 func TestWithDefault(t *testing.T) {
-	a := &tc.WithDefault{
+	withDefault := &tc.WithDefault{
 		Pouet: &tc.StructCollision2{
 			CollisionField2: "false indeed",
 		},
 	}
-	b := &tc.WithDefault{}
+	noDefault := &tc.WithDefault{}
 
-	a = roundTrip(t, a, "WithDefault{filled in}").(*tc.WithDefault)
-	b = roundTrip(t, b, "WithDefault{}").(*tc.WithDefault)
+	// expect FromWire values to have defaults and be equivalent
+	a := roundTrip(t, withDefault, withDefault, "WithDefault{filled in}").(*tc.WithDefault)
+	b := roundTrip(t, noDefault, withDefault, "WithDefault{}").(*tc.WithDefault)
 	if a != nil && b != nil {
 		require.Equal(t, a, b)
 	}
@@ -531,9 +532,9 @@ func TestMyEnum2Equals(t *testing.T) {
 	}
 }
 
-func roundTrip(t *testing.T, x thriftType, msg string) thriftType {
-	v, err := x.ToWire()
-	if !assert.NoError(t, err, "failed to serialize: %v", x) {
+func roundTrip(t *testing.T, give thriftType, want thriftType, msg string) thriftType {
+	v, err := give.ToWire()
+	if !assert.NoError(t, err, "failed to serialize: %v", give) {
 		return nil
 	}
 
@@ -548,11 +549,11 @@ func roundTrip(t *testing.T, x thriftType, msg string) thriftType {
 	}
 
 	if !assert.True(
-		t, wire.ValuesAreEqual(newV, v), "%v: deserialize(serialize(%v.ToWire())) != %v", msg, x, v) {
+		t, wire.ValuesAreEqual(newV, v), "%v: deserialize(serialize(%v.ToWire())) != %v", msg, give, v) {
 		return nil
 	}
 
-	xType := reflect.TypeOf(x)
+	xType := reflect.TypeOf(give)
 	if xType.Kind() == reflect.Ptr {
 		xType = xType.Elem()
 	}
@@ -562,7 +563,7 @@ func roundTrip(t *testing.T, x thriftType, msg string) thriftType {
 		return nil
 	}
 
-	assert.Equal(t, x, gotX, "FromWire: %v", msg)
+	assert.Equal(t, want, gotX, "FromWire: %v", msg)
 	return gotX
 }
 

gen/field.go

@@ -83,6 +83,10 @@ func (f fieldGroupGenerator) Generate(g Generator) error {
 		return err
 	}
 
+	if err := f.DefineDefaultConstructor(g); err != nil {
+		return err
+	}
+
 	if err := f.ToWire(g); err != nil {
 		return err
 	}
@@ -244,6 +248,35 @@ func (f *fieldGroupGenerator) declFieldName(fs *compile.FieldSpec) (string, erro
 	return name, nil
 }
 
+func (f fieldGroupGenerator) DefineDefaultConstructor(g Generator) error {
+	var hasDefaults bool
+	for _, f := range f.Fields {
+		if f.Default != nil {
+			hasDefaults = true
+			break
+		}
+	}
+	if !hasDefaults {
+		return nil
+	}
+	return g.DeclareFromTemplate(
+		`
+		// Default_<.Name> constructs a new <.Name> struct,
+		// pre-populating any fields with defined default values. 
+		func Default_<.Name>() *<.Name> {
+			<- $v := newVar "v" ->
+			var v <.Name>
+			<- range .Fields ->
+				<- $fname := goName . ->
+				<- if .Default>
+					<$v>.<$fname> = <constantValuePtr .Default .Type>
+				<- end ->
+			<end>
+			return &v
+		}
+		`, f, TemplateFunc("constantValuePtr", ConstantValuePtr))
+}
+
 func (f fieldGroupGenerator) ToWire(g Generator) error {
 	return g.DeclareFromTemplate(
 		`
@@ -297,14 +330,17 @@ func (f fieldGroupGenerator) ToWire(g Generator) error {
 						<$i>++
 				<- else ->
 					<- if .Default ->
-						if <$f> == nil {
-							<$f> = <constantValuePtr .Default .Type>
+						<- $fval := printf "%s%s" $v $fname ->
+						<$fval> := <$f>
+						if <$fval> == nil {
+							<$fval> = <constantValuePtr .Default .Type>
 						}
 						{
+							<$wVal>, err = <toWirePtr .Type $fval>
 					<- else ->
 						if <$f> != nil {
-					<- end>
 							<$wVal>, err = <toWirePtr .Type $f>
+					<- end>
 							if err != nil {
 								return <$wVal>, err
 							}

gen/generate.go

@@ -30,10 +30,16 @@ import (
 
 	"go.uber.org/thriftrw/compile"
 	"go.uber.org/thriftrw/internal/plugin"
+	"go.uber.org/thriftrw/plugin/api"
 
 	"go.uber.org/multierr"
 )
 
+// CodeGenerator lists possible code generators for a plugin.
+type CodeGenerator struct {
+	ServiceGenerator api.ServiceGenerator
+}
+
 // Options controls how code gets generated.
 type Options struct {
 	// OutputDir is the directory into which all generated code is written.
@@ -60,7 +66,7 @@ type Options struct {
 	NoVersionCheck bool
 
 	// Code generation plugin
-	Plugin plugin.Handle
+	Plugin CodeGenerator
 
 	// Do not generate types.go
 	NoTypes bool
@@ -133,20 +139,18 @@ func Generate(m *compile.Module, o *Options) error {
 		}
 	}
 
-	plug := o.Plugin
+	plug := o.Plugin.ServiceGenerator
 	if plug == nil {
-		plug = plugin.EmptyHandle
+		plug = plugin.EmptyServiceGenerator
 	}
 
-	if sgen := plug.ServiceGenerator(); sgen != nil {
-		res, err := sgen.Generate(genBuilder.Build())
-		if err != nil {
-			return err
-		}
+	res, err := plug.Generate(genBuilder.Build())
+	if err != nil {
+		return err
+	}
 
-		if err := mergeFiles(files, res.Files); err != nil {
-			return err
-		}
+	if err := mergeFiles(files, res.Files); err != nil {
+		return err
 	}
 
 	for relPath, contents := range files {

gen/generate_test.go

@@ -290,9 +290,12 @@ func TestGenerate(t *testing.T) {
 			require.NoError(t, err)
 			defer os.RemoveAll(outputDir)
 
-			var p plugin.Handle
+			var p CodeGenerator
 			if tt.getPlugin != nil {
-				p = tt.getPlugin(mockCtrl)
+				handle := tt.getPlugin(mockCtrl)
+				p = CodeGenerator{
+					ServiceGenerator: handle.ServiceGenerator(),
+				}
 			}
 
 			err = Generate(module, &Options{