Remove inline typeconvert when logging some thrift typedefs (#464)

#366 added fast zap logging
by generating extra `MarshalLog...` methods on generated types. For
typedefs in thrift, other types that log them will cast to the
underlying type in their `MarshalLog...` method, causing the generated
code to need to import multiple go packages generated from both direct
and indirect thrift dependencies.

This should not be necessary, as typedefs of complex types (anything
that cannot be mapped directly to a go stdlib type) get their own
`MarshalLog...` methods, which cast to underlying type when necessary.

This PR changes the logging for typedef'ed fields to:
- cast to the underlying type inline when the underlying thrift type
directly maps to a go type.
- defer to the typedef's generated `MarshalLog...` method otherwise.

This results in having more predictable importpaths (eg knowing that one
thrift import will lead to one go import in generated code) makes
implementing good bazel rules for thriftrw easier.

See internal issue GO-362 for a more elaborate example of how the
current behaviour complicates our Bazel setup.

Co-authored-by: Abhinav Gupta <abg@uber.com>

gen/generator.go

@@ -245,6 +245,8 @@ func (g *generator) TextTemplate(s string, data interface{}, opts ...TemplateOpt
 		"isNotNil": func(val interface{}) bool {
 			return val != nil
 		},
+		"zapTypedefGenerateMarshaler":     curryGenerator(g.z.zapTypedefGenerateMarshaler, g),
+		"zapTypedefHasGeneratedMarshaler": curryGenerator(g.z.zapTypedefHasGeneratedMarshaler, g),
 	}
 
 	tmpl := template.New("thriftrw").Delims("<", ">").Funcs(templateFuncs)

gen/internal/tests/thrift/typedefs.thrift

@@ -11,6 +11,8 @@ typedef string State
 
 typedef i128 UUID  // alias of struct
 
+typedef UUID MyUUID // alias of alias
+
 typedef list<Event> EventGroup  // alias fo collection
 
 struct i128 {
@@ -23,6 +25,10 @@ struct Event {
     2: optional Timestamp time  // optional typedef
 }
 
+struct TransitiveTypedefField {
+    1: required MyUUID defUUID  // required typedef of alias
+}
+
 struct DefaultPrimitiveTypedef {
     1: optional State state = "hello"
 }

gen/quick_test.go

@@ -455,6 +455,8 @@ func TestQuickSuite(t *testing.T) {
 		{Sample: td.StateMap{}, Kind: thriftTypedef},
 		{Sample: td.Timestamp(0), NoLog: true, Kind: thriftTypedef},
 		{Sample: td.UUID{}, Kind: thriftTypedef},
+		{Sample: td.MyUUID{}, Kind: thriftTypedef},
+		{Sample: td.TransitiveTypedefField{}, Kind: thriftStruct},
 		{Sample: tl.LittlePotatoe(0), NoLog: true, Kind: thriftTypedef},
 		{Sample: tl.LittlePotatoe2(0.0), NoLog: true, Kind: thriftTypedef},
 		{Sample: tul.UUID(""), NoLog: true, Kind: thriftTypedef},

gen/typedef.go

@@ -114,20 +114,18 @@ func typedef(g Generator, spec *compile.TypedefSpec) error {
 		</* We want the behavior of the underlying type for typedefs: in the case that
 				they are objects or arrays, we need to cast to the underlying object or array;
 				else, zapMarshaler in zap.go will take care of it. */>
-		<if (eq (zapEncoder .Target) "Object") ->
+		<if (zapTypedefHasGeneratedMarshaler .Target) ->
 			<$zapcore := import "go.uber.org/zap/zapcore">
 			<$enc := newVar "enc">
+			<if (eq (zapEncoder .Target) "Object") ->
 			func (<$v> <$typedefType>) MarshalLogObject(<$enc> <$zapcore>.ObjectEncoder) error {
-				return (<zapMarshaler . $v>).MarshalLogObject(<$enc>)
+				return (<zapTypedefGenerateMarshaler . $v>).MarshalLogObject(<$enc>)
 			}
-		<- end>
-
-		<if (eq (zapEncoder .Target) "Array") ->
-			<$zapcore := import "go.uber.org/zap/zapcore">
-			<$enc := newVar "enc">
+			<- else if (eq (zapEncoder .Target) "Array") ->
 			func (<$v> <$typedefType>) MarshalLogArray(<$enc> <$zapcore>.ArrayEncoder) error {
-				return (<zapMarshaler . $v>).MarshalLogArray(<$enc>)
+				return (<zapTypedefGenerateMarshaler . $v>).MarshalLogArray(<$enc>)
 			}
+			<- end>
 		<- end>
 		<- end>
 		`,