From 21fff8a597d3ff349a28b2f42a9c2961cc35a5b9 Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Fri, 21 Aug 2026 08:41:16 -0400 Subject: [PATCH 1/2] feat: Add tools/list-return-structs Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- tools/list-return-structs/main.go | 382 ++++++++++++++++++ tools/list-return-structs/main_test.go | 250 ++++++++++++ .../testdata/github/sample.go | 70 ++++ 3 files changed, 702 insertions(+) create mode 100644 tools/list-return-structs/main.go create mode 100644 tools/list-return-structs/main_test.go create mode 100644 tools/list-return-structs/testdata/github/sample.go diff --git a/tools/list-return-structs/main.go b/tools/list-return-structs/main.go new file mode 100644 index 00000000000..d191b931464 --- /dev/null +++ b/tools/list-return-structs/main.go @@ -0,0 +1,382 @@ +// Copyright 2026 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Command list-return-structs scans the Go sources of the `github` package and +// reports the struct types that are used strictly as return values — that is, +// structs that are only ever "unmarshaled" from the GitHub API server and are +// never "marshaled" and sent back to it. +// +// A struct is considered a "return" struct if it appears in the result list of +// any function or method, or if it is (recursively) referenced as a field of +// another return struct. A struct is considered an "input" struct if it appears +// as the type of a parameter named `opts` or `body` in any function or method, +// or if it is (recursively) referenced as a field of another input struct. The +// reported set is the return structs minus the input structs. +// +// Usage: +// +// go run tools/list-return-structs/main.go [-omitempty] [dir] +// +// The optional `dir` argument defaults to `github` (the package directory, +// relative to the current working directory). With -omitempty (or --omitempty) +// the report is reduced to structs that contain at least one field whose `json` +// struct tag includes "omitempty" or "omitzero". +// +// Structs are listed one per line in a unix-standard grep-like format, e.g.: +// +// github/rate_limit.go:14:type Rate struct { +package main + +import ( + "cmp" + "errors" + "flag" + "fmt" + "go/ast" + "go/parser" + "go/token" + "log" + "os" + "path/filepath" + "slices" + "strings" +) + +func main() { + log.SetFlags(0) + + omitempty := flag.Bool("omitempty", false, "reduce the report to structs that have at least one field tagged with \"omitempty\" or \"omitzero\"") + flag.Usage = func() { + log.Print("Usage: list-return-structs [-omitempty] [dir]") + flag.PrintDefaults() + } + flag.Parse() + + dir := "github" + if flag.NArg() > 0 { + dir = flag.Arg(0) + } + + structs, err := analyze(dir, *omitempty) + if err != nil { + log.Fatalf("analyzing %s: %v", dir, err) + } + + for _, s := range structs { + fmt.Printf("%v:%v:type %v struct {\n", s.relPath, s.line, s.name) + } +} + +// structInfo describes a struct type declaration found while scanning the +// package. +type structInfo struct { + name string // the struct's type name + relPath string // display path, e.g. "github/rate_limit.go" + line int // line of the "type X struct {" declaration + structType *ast.StructType // the parsed struct type, or nil for non-struct specs +} + +// parsePackage parses every non-test .go file in dir and returns the file set +// and parsed files. baseDir is the directory whose name should be preserved as +// the leading path component in structInfo.relPath (e.g. "github"). +func parsePackage(dir string) (*token.FileSet, []*ast.File, string, error) { + absDir, err := filepath.Abs(dir) + if err != nil { + return nil, nil, "", fmt.Errorf("resolving dir: %w", err) + } + + entries, err := os.ReadDir(absDir) + if err != nil { + return nil, nil, "", fmt.Errorf("reading dir: %w", err) + } + + // relPath is computed relative to the parent of absDir so that the path + // keeps its package directory component (e.g. "github/rate_limit.go") + // regardless of how dir was specified. + baseDir := filepath.Dir(absDir) + + fset := token.NewFileSet() + var files []*ast.File + for _, entry := range entries { + if entry.IsDir() { + continue + } + name := entry.Name() + if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + path := filepath.Join(absDir, name) + file, err := parser.ParseFile(fset, path, nil, parser.ParseComments) + if err != nil { + return nil, nil, "", fmt.Errorf("parsing %v: %w", path, err) + } + files = append(files, file) + } + + if len(files) == 0 { + return nil, nil, "", fmt.Errorf("no non-test .go files found in %v", absDir) + } + + return fset, files, baseDir, nil +} + +// collectStructs builds a map of struct name -> structInfo for every struct type +// declared in the parsed files. +func collectStructs(fset *token.FileSet, files []*ast.File, baseDir string) map[string]*structInfo { + structs := map[string]*structInfo{} + + for _, file := range files { + absPath := fset.Position(file.Package).Filename + relPath, err := filepath.Rel(baseDir, absPath) + if err != nil { + relPath = filepath.Base(absPath) + } + + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.TYPE { + continue + } + for _, spec := range genDecl.Specs { + typeSpec, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + structType, ok := typeSpec.Type.(*ast.StructType) + if !ok { + continue + } + structs[typeSpec.Name.Name] = &structInfo{ + name: typeSpec.Name.Name, + relPath: relPath, + line: fset.Position(typeSpec.Pos()).Line, + structType: structType, + } + } + } + } + + return structs +} + +// collectDirectUses inspects every function and method declaration and returns +// two sets of struct names: +// - returns: struct names appearing in a result position +// - inputs: struct names appearing as the type of a parameter named `opts` or +// `body` +// +// Only names that refer to package-local structs are of interest, but this +// function returns raw names; callers should intersect with the known structs +// map (closure expansion already does so). +func collectDirectUses(files []*ast.File) (returns, inputs map[string]bool) { + returns = map[string]bool{} + inputs = map[string]bool{} + + for _, file := range files { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Type == nil { + continue + } + + // Input parameters named `opts` or `body`. + if fn.Type.Params != nil { + for _, field := range fn.Type.Params.List { + for _, name := range field.Names { + if name.Name != "opts" && name.Name != "body" { + continue + } + if base := baseStructName(field.Type); base != "" { + inputs[base] = true + } + } + } + } + + // Result types — every named struct returned counts as a return use. + if fn.Type.Results != nil { + for _, field := range fn.Type.Results.List { + if base := baseStructName(field.Type); base != "" { + returns[base] = true + } + } + } + } + } + + return returns, inputs +} + +// transitiveClosure expands a starting set of struct names to include every +// package-local struct reachable as a field type (directly, or via pointers, +// slices, arrays, maps, or generic instantiations), recursively. +func transitiveClosure(start map[string]bool, structs map[string]*structInfo) map[string]bool { + closure := map[string]bool{} + var work []string + for name := range start { + closure[name] = true + work = append(work, name) + } + + for len(work) > 0 { + name := work[0] + work = work[1:] + info, ok := structs[name] + if !ok || info.structType == nil || info.structType.Fields == nil { + continue + } + for _, field := range info.structType.Fields.List { + if base := baseStructName(field.Type); base != "" { + if structs[base] != nil && !closure[base] { + closure[base] = true + work = append(work, base) + } + } + } + } + + return closure +} + +// baseStructName reduces a type expression to the name of its underlying named +// type, unwrapping pointers, slices, arrays, maps, ellipses, and generic +// instantiations. It returns "" for types that are not (or cannot be resolved +// as) a simple package-local identifier, such as qualified names (time.Time) or +// anonymous types. +func baseStructName(typ ast.Expr) string { + switch t := typ.(type) { + case *ast.Ident: + return t.Name + case *ast.StarExpr: + return baseStructName(t.X) + case *ast.ArrayType: + return baseStructName(t.Elt) + case *ast.Ellipsis: + return baseStructName(t.Elt) + case *ast.MapType: + return baseStructName(t.Value) + case *ast.IndexExpr: + return baseStructName(t.X) + case *ast.IndexListExpr: + return baseStructName(t.X) + case *ast.ParenExpr: + return baseStructName(t.X) + } + return "" +} + +// hasJSONOmitempty reports whether the struct has at least one field whose `json` +// struct tag includes the "omitempty" or "omitzero" option. +func hasJSONOmitempty(st *ast.StructType) bool { + if st == nil || st.Fields == nil { + return false + } + for _, field := range st.Fields.List { + if field.Tag == nil { + continue + } + tag := strings.Trim(field.Tag.Value, "`") + jsonTag, ok := structTagLookup(tag, "json") + if !ok { + continue + } + // A json tag looks like `name,omitempty` or `name,omitempty,omitzero`. + for opt := range strings.SplitSeq(jsonTag, ",") { + if opt == "omitempty" || opt == "omitzero" { + return true + } + } + } + return false +} + +// structTagLookup returns the value associated with the given key in a +// struct tag string, following the reflect.StructTag lookup rules. +func structTagLookup(tag, key string) (string, bool) { + for tag != "" { + // Skip leading whitespace. + tag = strings.TrimLeft(tag, " \t") + if tag == "" { + break + } + // Scan the key up to ':'. + i := strings.Index(tag, ":") + if i < 0 { + break + } + k := tag[:i] + rest := tag[i+1:] + if rest == "" || rest[0] != '"' { + break + } + // Scan the quoted value. + v, err := unquoteStructTag(rest) + if err != nil { + break + } + tag = rest[len(v)+2:] + if k == key { + return v, true + } + } + return "", false +} + +// unquoteStructTag unquotes a single double-quoted value at the start of s, +// returning the unquoted contents. +func unquoteStructTag(s string) (string, error) { + if len(s) < 2 || s[0] != '"' { + return "", errors.New("bad quote") + } + for i := 1; i < len(s); i++ { + switch s[i] { + case '\\': + i++ // skip escaped char + case '"': + return s[1:i], nil + } + } + return "", errors.New("unterminated quote") +} + +// analyze ties the steps together: parse the package, collect structs, compute +// the return and input closures, and return the structs that are return-only, +// sorted by file path then line. When omitempty is true, only structs with at +// least one omitempty/omitzero json-tagged field are returned. +func analyze(dir string, omitempty bool) ([]*structInfo, error) { + fset, files, baseDir, err := parsePackage(dir) + if err != nil { + return nil, err + } + + structs := collectStructs(fset, files, baseDir) + returns, inputs := collectDirectUses(files) + + returnClosure := transitiveClosure(returns, structs) + inputClosure := transitiveClosure(inputs, structs) + + var result []*structInfo + for name, info := range structs { + if !returnClosure[name] { + continue + } + if inputClosure[name] { + continue + } + if omitempty && !hasJSONOmitempty(info.structType) { + continue + } + result = append(result, info) + } + + slices.SortFunc(result, func(a, b *structInfo) int { + if c := cmp.Compare(a.relPath, b.relPath); c != 0 { + return c + } + return cmp.Compare(a.line, b.line) + }) + + return result, nil +} diff --git a/tools/list-return-structs/main_test.go b/tools/list-return-structs/main_test.go new file mode 100644 index 00000000000..f8d62e01889 --- /dev/null +++ b/tools/list-return-structs/main_test.go @@ -0,0 +1,250 @@ +// Copyright 2026 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "slices" + "strconv" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +// parseExpr parses a Go type expression string, for testing baseStructName. +func parseExpr(t *testing.T, src string) ast.Expr { + t.Helper() + expr, err := parser.ParseExpr(src) + if err != nil { + t.Fatalf("parser.ParseExpr(%q): %v", src, err) + } + return expr +} + +func TestBaseStructName(t *testing.T) { + t.Parallel() + tests := []struct { + src string + want string + }{ + {"Widget", "Widget"}, + {"*Widget", "Widget"}, + {"[]*Widget", "Widget"}, + {"[...]Widget", "Widget"}, + {"map[string]*Widget", "Widget"}, + {"map[string]Widget", "Widget"}, + {"List[Widget]", "List"}, + {"Map[K, V]", "Map"}, + {"(*Widget)", "Widget"}, + // Qualified and anonymous types are not resolvable as package-local + // struct names. + {"time.Time", ""}, + {"json.RawMessage", ""}, + {"struct{ X int }", ""}, + {"int", "int"}, // base name only; callers intersect with the structs map + {"[]int", "int"}, + } + for _, tc := range tests { + t.Run(tc.src, func(t *testing.T) { + t.Parallel() + got := baseStructName(parseExpr(t, tc.src)) + if got != tc.want { + t.Errorf("baseStructName(%q) = %q, want %q", tc.src, got, tc.want) + } + }) + } +} + +func TestBaseStructNameEllipsis(t *testing.T) { + t.Parallel() + // Variadic params are the only place a standalone *ast.Ellipsis occurs. + // Parse a function type and inspect its parameter type. + expr, err := parser.ParseExpr("func(x ...*Widget)") + if err != nil { + t.Fatalf("parser.ParseExpr: %v", err) + } + fn, ok := expr.(*ast.FuncType) + if !ok { + t.Fatalf("expected *ast.FuncType, got %T", expr) + } + field := fn.Params.List[0] + if got := baseStructName(field.Type); got != "Widget" { + t.Errorf("baseStructName of variadic param = %q, want %q", got, "Widget") + } +} + +func TestStructTagLookup(t *testing.T) { + t.Parallel() + tests := []struct { + tag string + key string + want string + ok bool + }{ + {`json:"name,omitempty"`, "json", "name,omitempty", true}, + {`json:"name,omitempty"`, "xml", "", false}, + {`xml:"x" json:"name,omitzero"`, "json", "name,omitzero", true}, + {`json:"-"`, "json", "-", true}, + {`url:"page"`, "json", "", false}, + {``, "json", "", false}, + } + for _, tc := range tests { + t.Run(tc.tag, func(t *testing.T) { + t.Parallel() + got, ok := structTagLookup(tc.tag, tc.key) + if got != tc.want || ok != tc.ok { + t.Errorf("structTagLookup(%q, %q) = (%q, %v), want (%q, %v)", + tc.tag, tc.key, got, ok, tc.want, tc.ok) + } + }) + } +} + +func TestHasJSONOmitempty(t *testing.T) { + t.Parallel() + // Build a struct from source and inspect it. + src := `package p +type T struct { + A string ` + "`json:\"a\"`" + ` + B string ` + "`json:\"b,omitempty\"`" + ` + C int ` + "`json:\"c,omitzero\"`" + ` + D string ` + "`url:\"d\"`" + ` + E string +}` + file := mustParse(t, src) + st := findStruct(t, file, "T") + + if !hasJSONOmitempty(st) { + t.Error("hasJSONOmitempty(T) = false, want true (B has omitempty)") + } +} + +func TestHasJSONOmitemptyNone(t *testing.T) { + t.Parallel() + src := `package p +type T struct { + A string ` + "`json:\"a\"`" + ` + B int ` + "`url:\"b\"`" + ` + C string +}` + file := mustParse(t, src) + st := findStruct(t, file, "T") + + if hasJSONOmitempty(st) { + t.Error("hasJSONOmitempty(T) = true, want false") + } +} + +func TestAnalyze(t *testing.T) { + t.Parallel() + structs, err := analyze("testdata/github", false) + if err != nil { + t.Fatalf("analyze: %v", err) + } + + got := names(structs) + // Response is a struct returned by every method and never an input, so it + // is correctly reported; it is excluded by the -omitempty filter below. + want := []string{"Embedded", "Inner", "Response", "Widget", "WidgetSpec"} + if !cmp.Equal(got, want) { + t.Errorf("analyze omitempty=false names =\n %v\nwant\n %v", got, want) + } +} + +func TestAnalyzeOmitempty(t *testing.T) { + t.Parallel() + structs, err := analyze("testdata/github", true) + if err != nil { + t.Fatalf("analyze: %v", err) + } + + got := names(structs) + // Widget (Name omitempty) and Inner (X omitempty) qualify. WidgetSpec, + // Embedded have no omitempty/omitzero json-tagged field. + want := []string{"Inner", "Widget"} + if !cmp.Equal(got, want) { + t.Errorf("analyze omitempty=true names =\n %v\nwant\n %v", got, want) + } +} + +func TestAnalyzeFormat(t *testing.T) { + t.Parallel() + structs, err := analyze("testdata/github", false) + if err != nil { + t.Fatalf("analyze: %v", err) + } + + // The Widget struct declaration must be reported in the grep-style format + // with the package directory prefix preserved. + var found string + for _, s := range structs { + if s.name == "Widget" { + found = formatLine(s) + } + } + if found == "" { + t.Fatalf("Widget not found in results: %v", names(structs)) + } + if !strings.HasPrefix(found, "github/sample.go:") { + t.Errorf("Widget line %q does not start with package prefix", found) + } + if !strings.HasSuffix(found, ":type Widget struct {") { + t.Errorf("Widget line %q does not have expected suffix", found) + } +} + +// names returns the sorted struct names from a slice of structInfo. +func names(structs []*structInfo) []string { + out := make([]string, 0, len(structs)) + for _, s := range structs { + out = append(out, s.name) + } + slices.Sort(out) + return out +} + +// formatLine renders a structInfo the way main prints it. +func formatLine(s *structInfo) string { + return s.relPath + ":" + strconv.Itoa(s.line) + ":type " + s.name + " struct {" +} + +// mustParse parses a single source file. +func mustParse(t *testing.T, src string) *ast.File { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "src.go", src, 0) + if err != nil { + t.Fatalf("parser.ParseFile: %v", err) + } + return f +} + +// findStruct returns the StructType for the named type in the file. +func findStruct(t *testing.T, file *ast.File, name string) *ast.StructType { + t.Helper() + for _, decl := range file.Decls { + gd, ok := decl.(*ast.GenDecl) + if !ok || gd.Tok != token.TYPE { + continue + } + for _, spec := range gd.Specs { + ts, ok := spec.(*ast.TypeSpec) + if !ok || ts.Name.Name != name { + continue + } + st, ok := ts.Type.(*ast.StructType) + if !ok { + t.Fatalf("%s is not a struct type", name) + } + return st + } + } + t.Fatalf("struct %s not found", name) + return nil +} diff --git a/tools/list-return-structs/testdata/github/sample.go b/tools/list-return-structs/testdata/github/sample.go new file mode 100644 index 00000000000..572fd959281 --- /dev/null +++ b/tools/list-return-structs/testdata/github/sample.go @@ -0,0 +1,70 @@ +// Package github is a fixture used only by list-return-structs tests. +package github + +import "context" + +// Returned directly by Get and Create; never an input. Should be INCLUDED. +type Widget struct { + Name string `json:"name,omitempty"` + Spec WidgetSpec +} + +// Field of Widget (a return struct); never an input. Should be INCLUDED +// transitively. +type WidgetSpec struct { + Detail string `json:"detail"` +} + +// Field of CreateWidgetRequest (an input struct); never returned and never +// directly an opts/body param. Should be EXCLUDED transitively. +type RequestMeta struct { + Token string `json:"token,omitempty"` +} + +// Used as an opts param. Should be EXCLUDED. +type ListWidgetsOptions struct { + Page int `json:"page,omitempty"` +} + +// Used as a body param. Should be EXCLUDED. +type CreateWidgetRequest struct { + Title string `json:"title"` + Meta RequestMeta +} + +// Returned by DualThing AND used as its body. Should be EXCLUDED. +type Dual struct { + Val string `json:"val,omitempty"` +} + +// Returned by ListEmbedded. Should be INCLUDED. +type Embedded struct { + Inner Inner +} + +// Field of Embedded (a return struct). Should be INCLUDED transitively. +type Inner struct { + X int `json:"x,omitempty"` +} + +// FakeService is a receiver for the fixture methods. +type FakeService struct{} + +// Response is a placeholder return type (deliberately not a struct here). +type Response struct{} + +func (s *FakeService) Get(ctx context.Context, opts *ListWidgetsOptions) (*Widget, *Response, error) { + return nil, nil, nil +} + +func (s *FakeService) Create(ctx context.Context, body CreateWidgetRequest) (*Widget, *Response, error) { + return nil, nil, nil +} + +func (s *FakeService) DualThing(ctx context.Context, body Dual) (*Dual, *Response, error) { + return nil, nil, nil +} + +func (s *FakeService) ListEmbedded(ctx context.Context) (*Embedded, *Response, error) { + return nil, nil, nil +} From 8f453d96a6e3e6a5453e8c1cc5ee0dbb175c0895 Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Fri, 21 Aug 2026 09:17:05 -0400 Subject: [PATCH 2/2] Fix Windows backslashes Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- tools/list-return-structs/main.go | 4 ++++ tools/list-return-structs/main_test.go | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tools/list-return-structs/main.go b/tools/list-return-structs/main.go index d191b931464..2610655e709 100644 --- a/tools/list-return-structs/main.go +++ b/tools/list-return-structs/main.go @@ -133,6 +133,10 @@ func collectStructs(fset *token.FileSet, files []*ast.File, baseDir string) map[ if err != nil { relPath = filepath.Base(absPath) } + // Always report paths with forward slashes so the output matches the + // unix-standard format on every platform (filepath.Rel uses the OS + // separator, which is '\' on Windows). + relPath = filepath.ToSlash(relPath) for _, decl := range file.Decls { genDecl, ok := decl.(*ast.GenDecl) diff --git a/tools/list-return-structs/main_test.go b/tools/list-return-structs/main_test.go index f8d62e01889..3be0105c1ef 100644 --- a/tools/list-return-structs/main_test.go +++ b/tools/list-return-structs/main_test.go @@ -199,6 +199,29 @@ func TestAnalyzeFormat(t *testing.T) { } } +func TestRelPathsUseForwardSlashes(t *testing.T) { + t.Parallel() + // On Windows, filepath.Rel produces backslash separators. The tool must + // normalize them so the output always matches the unix-standard format. + // This invariant holds on every platform; Windows CI is what enforces it + // in practice. + structs, err := analyze("testdata/github", false) + if err != nil { + t.Fatalf("analyze: %v", err) + } + if len(structs) == 0 { + t.Fatal("analyze returned no structs") + } + for _, s := range structs { + if strings.ContainsRune(s.relPath, '\\') { + t.Errorf("relPath %q contains a backslash; expected forward slashes only", s.relPath) + } + if !strings.HasPrefix(s.relPath, "github/") { + t.Errorf("relPath %q does not start with the package prefix \"github/\"", s.relPath) + } + } +} + // names returns the sorted struct names from a slice of structInfo. func names(structs []*structInfo) []string { out := make([]string, 0, len(structs))