fix(goctl): recurse into inline struct in IsTagMember (#5671)

Co-authored-by: kevin <wanjunfeng@gmail.com>

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{}
+}

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 {

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
 }
 

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"))
+}

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) {

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{}
+}