From 925f8a2bcc159eaf3b1da0f5fc695beac26e15ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E6=A1=89?= <161032298+014-code@users.noreply.github.com> Date: Sat, 1 Aug 2026 01:19:17 +0800 Subject: [PATCH] fix(goctl): recurse into inline struct in IsTagMember (#5671) Co-authored-by: kevin --- tools/goctl/api/parser/inline_tag_test.go | 69 ++++++++ tools/goctl/api/parser/parser.go | 67 +++++++ tools/goctl/api/spec/fn.go | 35 +++- tools/goctl/api/spec/fn_test.go | 167 ++++++++++++++++++ tools/goctl/pkg/parser/api/parser/analyzer.go | 67 +++++++ .../pkg/parser/api/parser/inline_tag_test.go | 69 ++++++++ 6 files changed, 469 insertions(+), 5 deletions(-) create mode 100644 tools/goctl/api/parser/inline_tag_test.go create mode 100644 tools/goctl/api/spec/fn_test.go create mode 100644 tools/goctl/pkg/parser/api/parser/inline_tag_test.go diff --git a/tools/goctl/api/parser/inline_tag_test.go b/tools/goctl/api/parser/inline_tag_test.go new file mode 100644 index 000000000..7e5afaa75 --- /dev/null +++ b/tools/goctl/api/parser/inline_tag_test.go @@ -0,0 +1,69 @@ +package parser + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/zeromicro/go-zero/tools/goctl/api/spec" +) + +const inlineTagAPI = ` +syntax = "v1" + +type ( + Auth { + Token string ` + "`header:\"Authorization\"`" + ` + } + Middle { + Auth + } + PointerRequest { + *Auth + } + NestedRequest { + Middle + } + RecursiveRequest { + Token string ` + "`header:\"X-Token\"`" + ` + *RecursiveRequest + } +) + +service test-api { + @handler Pointer + get /pointer (PointerRequest) + + @handler Nested + get /nested (NestedRequest) + + @handler Recursive + get /recursive (RecursiveRequest) +} +` + +func TestParseContentResolvesInlineTypesForTagLookup(t *testing.T) { + apiSpec, err := ParseContent(inlineTagAPI) + require.NoError(t, err) + + for _, name := range []string{"PointerRequest", "NestedRequest", "RecursiveRequest"} { + t.Run(name, func(t *testing.T) { + tp := findStructByName(t, apiSpec.Types, name) + require.NotEmpty(t, tp.GetTagMembers("header")) + require.Empty(t, tp.GetTagMembers("path")) + }) + } +} + +func findStructByName(t *testing.T, types []spec.Type, name string) spec.DefineStruct { + t.Helper() + for _, tp := range types { + if tp.Name() == name { + defined, ok := tp.(spec.DefineStruct) + require.True(t, ok) + return defined + } + } + + t.Fatalf("type %s not found", name) + return spec.DefineStruct{} +} diff --git a/tools/goctl/api/parser/parser.go b/tools/goctl/api/parser/parser.go index a41c0044b..baf1d0f52 100644 --- a/tools/goctl/api/parser/parser.go +++ b/tools/goctl/api/parser/parser.go @@ -145,6 +145,17 @@ func (p parser) fillTypes() error { case spec.DefineStruct: var members []spec.Member for _, member := range v.Members { + if member.IsInline { + tp, err := p.resolveInlineType(member.Type, map[string]bool{v.RawName: true}) + if err != nil { + return err + } + + member.Type = tp + members = append(members, member) + continue + } + switch v := member.Type.(type) { case spec.DefineStruct: tp, err := p.findDefinedType(v.RawName) @@ -167,6 +178,62 @@ func (p parser) fillTypes() error { return nil } +func (p parser) resolveInlineType(tp spec.Type, resolving map[string]bool) (spec.Type, error) { + switch v := tp.(type) { + case spec.DefineStruct: + if resolving[v.RawName] { + return v, nil + } + + tp, err := p.findDefinedType(v.RawName) + if err != nil { + return nil, err + } + + defined, ok := (*tp).(spec.DefineStruct) + if !ok { + return nil, fmt.Errorf("type %s is not a struct", v.RawName) + } + + resolving[v.RawName] = true + defer delete(resolving, v.RawName) + for i := range defined.Members { + if !defined.Members[i].IsInline { + continue + } + + resolved, err := p.resolveInlineType(defined.Members[i].Type, resolving) + if err != nil { + return nil, err + } + defined.Members[i].Type = resolved + } + return defined, nil + case spec.NestedStruct: + for i := range v.Members { + if !v.Members[i].IsInline { + continue + } + + resolved, err := p.resolveInlineType(v.Members[i].Type, resolving) + if err != nil { + return nil, err + } + v.Members[i].Type = resolved + } + return v, nil + case spec.PointerType: + resolved, err := p.resolveInlineType(v.Type, resolving) + if err != nil { + return nil, err + } + v.Type = resolved + return v, nil + default: + return tp, nil + } +} + func (p parser) findDefinedType(name string) (*spec.Type, error) { for _, item := range p.spec.Types { if _, ok := item.(spec.DefineStruct); ok { diff --git a/tools/goctl/api/spec/fn.go b/tools/goctl/api/spec/fn.go index 9555411b1..c0b2dfad8 100644 --- a/tools/goctl/api/spec/fn.go +++ b/tools/goctl/api/spec/fn.go @@ -139,18 +139,43 @@ func (m Member) IsFormMember() bool { return false } -// IsTagMember returns true if contains given tag +// IsTagMember returns true if the member contains the given tag. +// For inline members, it recursively checks the members of the referenced +// struct, since inline members themselves carry no tag and any matching tag +// must live on one of their children. This avoids spuriously reporting the +// presence of a tag (e.g. `header`) for an inline struct whose children do +// not actually use that tag. See go-zero #4800. func (m Member) IsTagMember(tagKey string) bool { - if m.IsInline { - return true - } - tags := m.Tags() for _, tag := range tags { if tag.Key == tagKey { return true } } + if m.IsInline { + return typeContainsTag(m.Type, tagKey) + } + return false +} + +func typeContainsTag(tp Type, tagKey string) bool { + var members []Member + switch v := tp.(type) { + case DefineStruct: + members = v.Members + case NestedStruct: + members = v.Members + case PointerType: + return typeContainsTag(v.Type, tagKey) + default: + return false + } + + for _, child := range members { + if child.IsTagMember(tagKey) { + return true + } + } return false } diff --git a/tools/goctl/api/spec/fn_test.go b/tools/goctl/api/spec/fn_test.go new file mode 100644 index 000000000..affbb19e5 --- /dev/null +++ b/tools/goctl/api/spec/fn_test.go @@ -0,0 +1,167 @@ +package spec + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMember_IsTagMember(t *testing.T) { + t.Run("non-inline member with matching tag returns true", func(t *testing.T) { + m := Member{Tag: `header:"Authorization"`} + assert.True(t, m.IsTagMember("header")) + }) + + t.Run("non-inline member without matching tag returns false", func(t *testing.T) { + m := Member{Tag: `json:"username"`} + assert.False(t, m.IsTagMember("header")) + assert.False(t, m.IsTagMember("path")) + assert.False(t, m.IsTagMember("form")) + }) + + t.Run("non-inline member without any tag returns false", func(t *testing.T) { + m := Member{} + assert.False(t, m.IsTagMember("header")) + }) + + t.Run("inline struct without matching child tag returns false (#4800)", func(t *testing.T) { + m := Member{ + Name: "Pagination", + IsInline: true, + Type: DefineStruct{ + RawName: "Pagination", + Members: []Member{ + {Name: "Page", Tag: `json:"page"`}, + {Name: "PageSize", Tag: `json:"pageSize"`}, + }, + }, + } + assert.False(t, m.IsTagMember("header")) + assert.False(t, m.IsTagMember("path")) + assert.False(t, m.IsTagMember("form")) + }) + + t.Run("inline struct whose child has matching tag returns true", func(t *testing.T) { + m := Member{ + Name: "Auth", + IsInline: true, + Type: DefineStruct{ + RawName: "Auth", + Members: []Member{ + {Name: "Token", Tag: `header:"Authorization"`}, + }, + }, + } + assert.True(t, m.IsTagMember("header")) + }) + + t.Run("nested inline structs are recursed", func(t *testing.T) { + m := Member{ + Name: "Outer", + IsInline: true, + Type: DefineStruct{ + RawName: "Outer", + Members: []Member{ + { + Name: "Inner", + IsInline: true, + Type: DefineStruct{ + RawName: "Inner", + Members: []Member{ + {Name: "Token", Tag: `header:"X-Token"`}, + }, + }, + }, + }, + }, + } + assert.True(t, m.IsTagMember("header")) + }) + + t.Run("nested inline structs without matching child return false", func(t *testing.T) { + m := Member{ + Name: "Outer", + IsInline: true, + Type: DefineStruct{ + RawName: "Outer", + Members: []Member{ + { + Name: "Inner", + IsInline: true, + Type: DefineStruct{ + RawName: "Inner", + Members: []Member{ + {Name: "Page", Tag: `json:"page"`}, + }, + }, + }, + }, + }, + } + assert.False(t, m.IsTagMember("header")) + }) + + t.Run("inline NestedStruct whose child has matching tag returns true", func(t *testing.T) { + m := Member{ + Name: "Auth", + IsInline: true, + Type: NestedStruct{ + RawName: "Auth", + Members: []Member{ + {Name: "Token", Tag: `header:"Authorization"`}, + }, + }, + } + assert.True(t, m.IsTagMember("header")) + }) + + t.Run("inline PointerType whose child has matching tag returns true", func(t *testing.T) { + m := Member{ + Name: "Auth", + IsInline: true, + Type: PointerType{ + RawName: "*Auth", + Type: DefineStruct{ + RawName: "Auth", + Members: []Member{ + {Name: "Token", Tag: `header:"Authorization"`}, + }, + }, + }, + } + assert.True(t, m.IsTagMember("header")) + assert.False(t, m.IsTagMember("path")) + }) + + t.Run("empty inline struct returns false", func(t *testing.T) { + m := Member{ + Name: "Empty", + IsInline: true, + Type: DefineStruct{RawName: "Empty"}, + } + assert.False(t, m.IsTagMember("header")) + }) +} + +func TestDefineStruct_GetTagMembers_InlineRegression(t *testing.T) { + s := DefineStruct{ + RawName: "QueryUserListReq", + Members: []Member{ + {Name: "Username", Tag: `json:"username,optional"`}, + { + Name: "Pagination", + IsInline: true, + Type: DefineStruct{ + RawName: "Pagination", + Members: []Member{ + {Name: "Page", Tag: `json:"page"`}, + {Name: "PageSize", Tag: `json:"pageSize"`}, + }, + }, + }, + }, + } + assert.Empty(t, s.GetTagMembers("header"), + "inline struct without header-tagged children must not match header (#4800)") + assert.Empty(t, s.GetTagMembers("path")) +} diff --git a/tools/goctl/pkg/parser/api/parser/analyzer.go b/tools/goctl/pkg/parser/api/parser/analyzer.go index b668a8ad9..2ca3e74ab 100644 --- a/tools/goctl/pkg/parser/api/parser/analyzer.go +++ b/tools/goctl/pkg/parser/api/parser/analyzer.go @@ -349,6 +349,17 @@ func (a *Analyzer) fillTypes() error { case spec.DefineStruct: var members []spec.Member for _, member := range v.Members { + if member.IsInline { + tp, err := a.resolveInlineType(member.Type, map[string]bool{v.RawName: true}) + if err != nil { + return err + } + + member.Type = tp + members = append(members, member) + continue + } + switch v := member.Type.(type) { case spec.DefineStruct: tp, err := a.findDefinedType(v.RawName) @@ -371,6 +382,62 @@ func (a *Analyzer) fillTypes() error { return nil } +func (a *Analyzer) resolveInlineType(tp spec.Type, resolving map[string]bool) (spec.Type, error) { + switch v := tp.(type) { + case spec.DefineStruct: + if resolving[v.RawName] { + return v, nil + } + + tp, err := a.findDefinedType(v.RawName) + if err != nil { + return nil, err + } + + defined, ok := tp.(spec.DefineStruct) + if !ok { + return nil, fmt.Errorf("type %s is not a struct", v.RawName) + } + + resolving[v.RawName] = true + defer delete(resolving, v.RawName) + for i := range defined.Members { + if !defined.Members[i].IsInline { + continue + } + + resolved, err := a.resolveInlineType(defined.Members[i].Type, resolving) + if err != nil { + return nil, err + } + defined.Members[i].Type = resolved + } + return defined, nil + case spec.NestedStruct: + for i := range v.Members { + if !v.Members[i].IsInline { + continue + } + + resolved, err := a.resolveInlineType(v.Members[i].Type, resolving) + if err != nil { + return nil, err + } + v.Members[i].Type = resolved + } + return v, nil + case spec.PointerType: + resolved, err := a.resolveInlineType(v.Type, resolving) + if err != nil { + return nil, err + } + v.Type = resolved + return v, nil + default: + return tp, nil + } +} + func (a *Analyzer) fillTypeExpr(expr *ast.TypeExpr) error { head, _ := expr.CommentGroup() switch val := expr.DataType.(type) { diff --git a/tools/goctl/pkg/parser/api/parser/inline_tag_test.go b/tools/goctl/pkg/parser/api/parser/inline_tag_test.go new file mode 100644 index 000000000..6e455361a --- /dev/null +++ b/tools/goctl/pkg/parser/api/parser/inline_tag_test.go @@ -0,0 +1,69 @@ +package parser + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/zeromicro/go-zero/tools/goctl/api/spec" +) + +const inlineTagAPI = ` +syntax = "v1" + +type ( + Auth { + Token string ` + "`header:\"Authorization\"`" + ` + } + Middle { + Auth + } + PointerRequest { + *Auth + } + NestedRequest { + Middle + } + RecursiveRequest { + Token string ` + "`header:\"X-Token\"`" + ` + *RecursiveRequest + } +) + +service test-api { + @handler Pointer + get /pointer (PointerRequest) + + @handler Nested + get /nested (NestedRequest) + + @handler Recursive + get /recursive (RecursiveRequest) +} +` + +func TestParseResolvesInlineTypesForTagLookup(t *testing.T) { + apiSpec, err := Parse("inline.api", inlineTagAPI) + require.NoError(t, err) + + for _, name := range []string{"PointerRequest", "NestedRequest", "RecursiveRequest"} { + t.Run(name, func(t *testing.T) { + tp := findStructByName(t, apiSpec.Types, name) + require.NotEmpty(t, tp.GetTagMembers("header")) + require.Empty(t, tp.GetTagMembers("path")) + }) + } +} + +func findStructByName(t *testing.T, types []spec.Type, name string) spec.DefineStruct { + t.Helper() + for _, tp := range types { + if tp.Name() == name { + defined, ok := tp.(spec.DefineStruct) + require.True(t, ok) + return defined + } + } + + t.Fatalf("type %s not found", name) + return spec.DefineStruct{} +}