fix: keep default json tag when go.tag has no json key

When go.tag is set without a json: entry and gen_json_tag is enabled,
the default json tag is now preserved instead of being dropped.

Also deprecate always_gen_json_tag, which existed only as a workaround
for this behavior.

Author: xiaost

README.md

@@ -118,7 +118,6 @@ Boolean options accept `true`, `false`, or empty (empty means `true`).
 | `unescape_double_quote` | **true** | Unescape double quotes in tag literals. |
 | `gen_type_meta` | false | Generate and register type metadata for structs. |
 | `gen_json_tag` | **true** | Generate `json` struct tags. |
-| `always_gen_json_tag` | false | Generate `json` tags even when a `go.tag` annotation is present. |
 | `snake_style_json_tag` | false | Use `snake_case` style for JSON tags. |
 | `lower_camel_style_json_tag` | false | Use `lowerCamelCase` style for JSON tags. |
 | `with_reflection` | false | Generate `*-reflection.go` files with runtime type metadata for structs.<br>Required by `with_field_mask`. |

generator/golang/option.go

@@ -45,7 +45,7 @@ type Features struct {
 	EscapeDoubleInTag           bool `unescape_double_quote:"Unescape the double quotes in literals when generating go tags."`
 	GenerateTypeMeta            bool `gen_type_meta:"Generate and register type meta for structures."`
 	GenerateJSONTag             bool `gen_json_tag:"Generate struct with 'json' tag"`
-	AlwaysGenerateJSONTag       bool `always_gen_json_tag:"Always generate 'json' tag even if go.tag is provided (Disabled by default)"`
+	AlwaysGenerateJSONTag       bool `always_gen_json_tag:"Deprecated."`
 	SnakeTyleJSONTag            bool `snake_style_json_tag:"Generate snake style json tag"`
 	LowerCamelCaseJSONTag       bool `lower_camel_style_json_tag:"Generate lower camel case style json tag"`
 	WithReflection              bool `with_reflection:"Generate *-reflection.go files with runtime type metadata for structs (required by with_field_mask)."`
@@ -294,6 +294,10 @@ func (cu *CodeUtils) validateOptions() error {
 		return fmt.Errorf("always_gen_json_tag requires gen_json_tag")
 	}
 
+	if f.AlwaysGenerateJSONTag {
+		cu.Warn("always_gen_json_tag is deprecated: gen_json_tag now keeps the default json tag when go.tag has no json key")
+	}
+
 	if f.StreamX && !f.ThriftStreaming {
 		cu.Warn("streamx has no effect without thrift_streaming")
 	}

generator/golang/util.go

@@ -17,6 +17,7 @@ package golang
 import (
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"sort"
@@ -329,6 +330,7 @@ func (cu *CodeUtils) genFieldTags(f *Field, insertPoint string, extend []string)
 	tags = append(tags, extend...)
 
 	gotags := f.Annotations.Get("go.tag")
+	hasJSONInGoTag := false
 	if len(gotags) > 0 {
 		tag := gotags[0]
 		if cu.Features().EscapeDoubleInTag {
@@ -339,14 +341,15 @@ func (cu *CodeUtils) genFieldTags(f *Field, insertPoint string, extend []string)
 				return m
 			})
 		}
+		_, hasJSONInGoTag = reflect.StructTag(tag).Lookup("json")
 		tags = append(tags, tag)
 	} else {
 		if cu.Features().GenDatabaseTag {
 			tags = append(tags, fmt.Sprintf(`db:"%s"`, f.Name))
 		}
 	}
 
-	if len(gotags) == 0 && cu.Features().GenerateJSONTag || cu.Features().AlwaysGenerateJSONTag {
+	if (!hasJSONInGoTag && cu.Features().GenerateJSONTag) || cu.Features().AlwaysGenerateJSONTag {
 		id := f.Name
 		if cu.Features().SnakeTyleJSONTag {
 			id = snakify(id)

generator/golang/util_test.go

@@ -15,8 +15,10 @@
 package golang
 
 import (
-	"github.com/cloudwego/thriftgo/parser"
 	"testing"
+
+	"github.com/cloudwego/thriftgo/generator/backend"
+	"github.com/cloudwego/thriftgo/parser"
 )
 
 func TestSnakify(t *testing.T) {
@@ -222,3 +224,78 @@ func TestGenAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestGenFieldTags(t *testing.T) {
+	newField := func(annos parser.Annotations) *Field {
+		return &Field{Field: &parser.Field{
+			ID:           1,
+			Name:         "Name",
+			Requiredness: parser.FieldType_Default,
+			Annotations:  annos,
+		}}
+	}
+
+	cases := []struct {
+		desc     string
+		field    *Field
+		feats    func(f *Features)
+		expected string
+	}{
+		{
+			desc:     "no go.tag, gen_json_tag default on",
+			field:    newField(nil),
+			expected: "`thrift:\"Name,1\" json:\"Name\"`",
+		},
+		{
+			desc:     "go.tag with json: keeps go.tag json, no duplicate",
+			field:    newField(parser.Annotations{{Key: "go.tag", Values: []string{`json:"n" yaml:"n"`}}}),
+			expected: "`thrift:\"Name,1\" json:\"n\" yaml:\"n\"`",
+		},
+		{
+			desc:     "go.tag without json: still gets default json tag",
+			field:    newField(parser.Annotations{{Key: "go.tag", Values: []string{`yaml:"n"`}}}),
+			expected: "`thrift:\"Name,1\" yaml:\"n\" json:\"Name\"`",
+		},
+		{
+			desc:  "go.tag without json: but gen_json_tag off → no json tag",
+			field: newField(parser.Annotations{{Key: "go.tag", Values: []string{`yaml:"n"`}}}),
+			feats: func(f *Features) {
+				f.GenerateJSONTag = false
+			},
+			expected: "`thrift:\"Name,1\" yaml:\"n\"`",
+		},
+		{
+			desc:  "always_gen_json_tag forces json even when go.tag has json",
+			field: newField(parser.Annotations{{Key: "go.tag", Values: []string{`json:"n"`}}}),
+			feats: func(f *Features) {
+				f.AlwaysGenerateJSONTag = true
+			},
+			expected: "`thrift:\"Name,1\" json:\"n\" json:\"Name\"`",
+		},
+		{
+			// e.g. single-quoted thrift literal `'json:\"n\"'` survives the parser as raw `json:\"n\"`;
+			// the lookup must run after EscapeDoubleInTag or the json key is missed.
+			desc:     "go.tag with escaped quotes still detects json key",
+			field:    newField(parser.Annotations{{Key: "go.tag", Values: []string{`json:\"n\"`}}}),
+			expected: "`thrift:\"Name,1\" json:\"n\"`",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			cu := NewCodeUtils(backend.DummyLogFunc())
+			feats := defaultFeatures
+			if c.feats != nil {
+				c.feats(&feats)
+			}
+			cu.SetFeatures(feats)
+			got, err := cu.GenFieldTags(c.field, "")
+			if err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
+			if got != c.expected {
+				t.Fatalf("got %q, want %q", got, c.expected)
+			}
+		})
+	}
+}