From dee1b456b892ddec4ebc4b0e09754de22d8a8f7b Mon Sep 17 00:00:00 2001 From: uffejaeger <71610258+uffejaeger@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:22:20 -0300 Subject: [PATCH 1/4] Preserve aliased generated model compatibility --- docs/api-compatibility.md | 21 +- internal/generated/intercom/client.gen.go | 4 +- .../tools/filter-generated-api-diff/main.go | 244 ++++++++++++++++++ .../filter-generated-api-diff/main_test.go | 75 ++++++ internal/tools/normalize-spec/main.go | 21 ++ internal/tools/normalize-spec/main_test.go | 65 +++++ public_api_types_test.go | 20 ++ scripts/check-api-compatibility.sh | 24 ++ 8 files changed, 465 insertions(+), 9 deletions(-) create mode 100644 internal/tools/filter-generated-api-diff/main.go create mode 100644 internal/tools/filter-generated-api-diff/main_test.go create mode 100644 internal/tools/normalize-spec/main_test.go diff --git a/docs/api-compatibility.md b/docs/api-compatibility.md index f56f1f1..2345608 100644 --- a/docs/api-compatibility.md +++ b/docs/api-compatibility.md @@ -35,10 +35,10 @@ type changes must not bypass compatibility review. Go's API export data does not expand aliases into an `internal` package during a normal public-module comparison. To avoid a blind spot, the automated gate also -compares the generated model package directly. This is deliberately -conservative: it can flag a changed generated type that is not currently -reachable through a root-package alias. Maintainers should confirm reachability -during review, but must preserve any shape that is publicly exposed. +compares the generated model package directly and filters that report to types +reachable from current root-package aliases. Changes beneath those aliases are +therefore checked without treating the private generated client itself as a +second public API surface. ### Request-scoped options @@ -93,8 +93,9 @@ tool. The command fails when an incompatible compile-time change is reported. CI runs it in the quality job, and `make pre-push` runs it locally. The generated OpenAPI client is under Go's `internal/` boundary, so downstream -SDK consumers cannot import it. It is verified for reproducibility by `make -generate-check`, rather than treated as a second public API surface. +SDK consumers cannot import it. `make generate-check` verifies reproducibility, +while the compatibility gate separately checks only generated model shapes that +are exposed transitively through public aliases. The checker has a narrow source-compatibility allowance for the reviewed Contact, Ticket, and Article boundary models: API 2.16 changed Intercom's wire @@ -107,7 +108,13 @@ retains the legacy fields and maps the first parent ID when possible. `apidiff` required alias-to-struct change and its dependent methods and iterators as incompatible, so the checker permits only those exact entries. External consumer compile and response-conversion regression tests cover the preserved -contracts. No other `apidiff` finding is suppressed. +contracts. + +The generated-alias comparison also permits two identity-only findings for +`SearchRequestSchema` and `SearchRequest_Query`. Their exported fields and +method sets are unchanged, but `apidiff` reports the oapi-codegen union wrappers +as changing from each named type to itself. External consumer compile tests +cover those public aliases. No other `apidiff` finding is suppressed. The current baseline is `v0.2.0`. After publishing a release that expands the public API, maintainers advance `API_BASELINE` in `Makefile` to that release so diff --git a/internal/generated/intercom/client.gen.go b/internal/generated/intercom/client.gen.go index 10dcd61..d5f6dd2 100644 --- a/internal/generated/intercom/client.gen.go +++ b/internal/generated/intercom/client.gen.go @@ -8694,8 +8694,8 @@ type ConversationScorecardReviewedTeammateType string // ConversationSourceSchema The type of the conversation part that started this conversation. Can be Contact, Admin, Campaign, Automated or Operator initiated. type ConversationSourceSchema struct { // Attachments A list of attachments for the part. - Attachments *[]PartAttachmentSchema `json:"attachments,omitempty"` - Author *ConversationSourceAuthorSchema `json:"author,omitempty"` + Attachments *[]PartAttachmentSchema `json:"attachments,omitempty"` + Author *ConversationPartAuthorSchema `json:"author,omitempty"` // Body The message body, which may contain HTML. For Twitter, this will show a generic message regarding why the body is obscured. In webhook payloads for API version 2.15+, this field returns plain text. Body *string `json:"body,omitempty"` diff --git a/internal/tools/filter-generated-api-diff/main.go b/internal/tools/filter-generated-api-diff/main.go new file mode 100644 index 0000000..42acbea --- /dev/null +++ b/internal/tools/filter-generated-api-diff/main.go @@ -0,0 +1,244 @@ +// Command filter-generated-api-diff keeps apidiff findings for generated +// model types that are reachable through aliases in the public root package. +package main + +import ( + "bufio" + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strconv" + "strings" +) + +const generatedImportSuffix = "/internal/generated/intercom" + +func main() { + if len(os.Args) != 2 { + _, _ = fmt.Fprintln(os.Stderr, "usage: filter-generated-api-diff ") + os.Exit(2) + } + + reachable, err := reachableGeneratedTypes(os.Args[1]) + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "find reachable generated types: %v\n", err) + os.Exit(1) + } + + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Text() + if affectedType, ok := affectedGeneratedType(line); ok { + if _, reachable := reachable[affectedType]; reachable { + fmt.Println(line) + } + } + } + if err := scanner.Err(); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "read apidiff report: %v\n", err) + os.Exit(1) + } +} + +func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { + targets, err := generatedAliasTargets(moduleRoot) + if err != nil { + return nil, err + } + + declarations, err := generatedTypeDeclarations(filepath.Join(moduleRoot, "internal", "generated", "intercom")) + if err != nil { + return nil, err + } + + reachable := make(map[string]struct{}, len(targets)) + queue := make([]string, 0, len(targets)) + for target := range targets { + if _, exists := declarations[target]; exists { + queue = append(queue, target) + } + } + + for len(queue) > 0 { + name := queue[0] + queue = queue[1:] + if _, seen := reachable[name]; seen { + continue + } + reachable[name] = struct{}{} + + dependencies := make(map[string]struct{}) + collectTypeDependencies(declarations[name], dependencies) + for dependency := range dependencies { + if _, exists := declarations[dependency]; exists { + queue = append(queue, dependency) + } + } + } + + return reachable, nil +} + +func generatedAliasTargets(moduleRoot string) (map[string]struct{}, error) { + targets := make(map[string]struct{}) + entries, err := os.ReadDir(moduleRoot) + if err != nil { + return nil, err + } + + fileset := token.NewFileSet() + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + + file, err := parser.ParseFile(fileset, filepath.Join(moduleRoot, name), nil, 0) + if err != nil { + return nil, err + } + + generatedAliases := make(map[string]struct{}) + for _, spec := range file.Imports { + importPath, err := strconv.Unquote(spec.Path.Value) + if err != nil || !strings.HasSuffix(importPath, generatedImportSuffix) { + continue + } + alias := filepath.Base(importPath) + if spec.Name != nil { + alias = spec.Name.Name + } + generatedAliases[alias] = struct{}{} + } + + for _, declaration := range file.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok || general.Tok != token.TYPE { + continue + } + for _, rawSpec := range general.Specs { + typeSpec, ok := rawSpec.(*ast.TypeSpec) + if !ok || !typeSpec.Assign.IsValid() { + continue + } + selector, ok := typeSpec.Type.(*ast.SelectorExpr) + if !ok { + continue + } + packageName, ok := selector.X.(*ast.Ident) + if !ok { + continue + } + if _, generated := generatedAliases[packageName.Name]; generated { + targets[selector.Sel.Name] = struct{}{} + } + } + } + } + + return targets, nil +} + +func generatedTypeDeclarations(directory string) (map[string]ast.Expr, error) { + declarations := make(map[string]ast.Expr) + fileset := token.NewFileSet() + packages, err := parser.ParseDir(fileset, directory, func(info os.FileInfo) bool { + return strings.HasSuffix(info.Name(), ".go") && !strings.HasSuffix(info.Name(), "_test.go") + }, 0) + if err != nil { + return nil, err + } + + for _, pkg := range packages { + for _, file := range pkg.Files { + for _, declaration := range file.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok || general.Tok != token.TYPE { + continue + } + for _, rawSpec := range general.Specs { + typeSpec, ok := rawSpec.(*ast.TypeSpec) + if ok { + declarations[typeSpec.Name.Name] = typeSpec.Type + } + } + } + } + } + + return declarations, nil +} + +func collectTypeDependencies(expression ast.Expr, dependencies map[string]struct{}) { + switch expression := expression.(type) { + case *ast.Ident: + dependencies[expression.Name] = struct{}{} + case *ast.ParenExpr: + collectTypeDependencies(expression.X, dependencies) + case *ast.StarExpr: + collectTypeDependencies(expression.X, dependencies) + case *ast.ArrayType: + collectTypeDependencies(expression.Elt, dependencies) + case *ast.MapType: + collectTypeDependencies(expression.Key, dependencies) + collectTypeDependencies(expression.Value, dependencies) + case *ast.ChanType: + collectTypeDependencies(expression.Value, dependencies) + case *ast.Ellipsis: + collectTypeDependencies(expression.Elt, dependencies) + case *ast.IndexExpr: + collectTypeDependencies(expression.X, dependencies) + collectTypeDependencies(expression.Index, dependencies) + case *ast.IndexListExpr: + collectTypeDependencies(expression.X, dependencies) + for _, index := range expression.Indices { + collectTypeDependencies(index, dependencies) + } + case *ast.StructType: + collectFieldListDependencies(expression.Fields, dependencies) + case *ast.InterfaceType: + collectFieldListDependencies(expression.Methods, dependencies) + case *ast.FuncType: + collectFieldListDependencies(expression.TypeParams, dependencies) + collectFieldListDependencies(expression.Params, dependencies) + collectFieldListDependencies(expression.Results, dependencies) + } +} + +func collectFieldListDependencies(fields *ast.FieldList, dependencies map[string]struct{}) { + if fields == nil { + return + } + for _, field := range fields.List { + collectTypeDependencies(field.Type, dependencies) + } +} + +func affectedGeneratedType(line string) (string, bool) { + symbol, _, ok := strings.Cut(strings.TrimPrefix(line, "- "), ":") + if !ok { + return "", false + } + + if strings.HasPrefix(symbol, "(*") { + end := strings.Index(symbol, ").") + if end == -1 { + return "", false + } + return symbol[2:end], true + } + if strings.HasPrefix(symbol, "(") { + end := strings.Index(symbol, ").") + if end == -1 { + return "", false + } + return symbol[1:end], true + } + if before, _, ok0 := strings.Cut(symbol, "."); ok0 { + return before, true + } + return symbol, symbol != "" +} diff --git a/internal/tools/filter-generated-api-diff/main_test.go b/internal/tools/filter-generated-api-diff/main_test.go new file mode 100644 index 0000000..601072e --- /dev/null +++ b/internal/tools/filter-generated-api-diff/main_test.go @@ -0,0 +1,75 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestAffectedGeneratedType(t *testing.T) { + tests := []struct { + line string + want string + ok bool + }{ + {line: "- ConversationSourceSchema.Author: changed from A to B", want: "ConversationSourceSchema", ok: true}, + {line: "- (*ExampleSchema).Validate: removed", want: "ExampleSchema", ok: true}, + {line: "- ExampleSchema: removed", want: "ExampleSchema", ok: true}, + {line: "not an apidiff finding", ok: false}, + } + + for _, test := range tests { + got, ok := affectedGeneratedType(test.line) + if got != test.want || ok != test.ok { + t.Errorf("affectedGeneratedType(%q) = (%q, %t), want (%q, %t)", test.line, got, ok, test.want, test.ok) + } + } +} + +func TestReachableGeneratedTypes(t *testing.T) { + root := t.TempDir() + writeTestFile(t, root, "aliases.go", `package example + +import gen "example.test/sdk/internal/generated/intercom" + +type Public = gen.Root +`) + writeTestFile(t, root, "internal/generated/intercom/models.go", `package intercom + +type Root struct { + Nested *Nested +} + +type Nested struct { + Value string +} + +type Unreachable struct { + Ignored bool +} +`) + + reachable, err := reachableGeneratedTypes(root) + if err != nil { + t.Fatalf("reachableGeneratedTypes: %v", err) + } + for _, name := range []string{"Root", "Nested"} { + if _, ok := reachable[name]; !ok { + t.Errorf("%s is not reachable", name) + } + } + if _, ok := reachable["Unreachable"]; ok { + t.Error("Unreachable unexpectedly reported as reachable") + } +} + +func writeTestFile(t *testing.T, root, name, contents string) { + t.Helper() + path := filepath.Join(root, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} diff --git a/internal/tools/normalize-spec/main.go b/internal/tools/normalize-spec/main.go index 20e0073..84f83eb 100644 --- a/internal/tools/normalize-spec/main.go +++ b/internal/tools/normalize-spec/main.go @@ -30,6 +30,7 @@ func main() { patchPathParameters(&spec) patchComponentGoNames(&spec) patchConversationAttributeDiscriminator(&spec) + patchConversationSourceAuthorCompatibility(&spec) patchPropertyGoNames(&spec) output, err := yaml.Marshal(&spec) @@ -44,6 +45,26 @@ func main() { } } +// patchConversationSourceAuthorCompatibility preserves the source-compatible +// author shape exposed through the root Conversation alias. API 2.16 points the +// source author at a new, narrower schema that omits from_ai_agent and +// is_ai_answer. The historical conversation_part_author schema is wire +// compatible and retains those optional fields for existing SDK consumers. +func patchConversationSourceAuthorCompatibility(spec *yaml.Node) { + authorRef := lookup( + spec, + "components", + "schemas", + "conversation_source", + "properties", + "author", + "$ref", + ) + if scalarValue(authorRef) == "#/components/schemas/conversation_source_author" { + authorRef.Value = "#/components/schemas/conversation_part_author" + } +} + func patchPropertyGoNames(spec *yaml.Node) { schemas := lookup(spec, "components", "schemas") if schemas == nil || schemas.Kind != yaml.MappingNode { diff --git a/internal/tools/normalize-spec/main_test.go b/internal/tools/normalize-spec/main_test.go new file mode 100644 index 0000000..e8dca0d --- /dev/null +++ b/internal/tools/normalize-spec/main_test.go @@ -0,0 +1,65 @@ +package main + +import ( + "testing" + + "gopkg.in/yaml.v3" +) + +func TestPatchConversationSourceAuthorCompatibility(t *testing.T) { + var spec yaml.Node + if err := yaml.Unmarshal([]byte(` +components: + schemas: + conversation_source: + properties: + author: + $ref: '#/components/schemas/conversation_source_author' +`), &spec); err != nil { + t.Fatalf("yaml.Unmarshal: %v", err) + } + + patchConversationSourceAuthorCompatibility(&spec) + + authorRef := lookup( + &spec, + "components", + "schemas", + "conversation_source", + "properties", + "author", + "$ref", + ) + if got, want := scalarValue(authorRef), "#/components/schemas/conversation_part_author"; got != want { + t.Fatalf("author ref = %q, want %q", got, want) + } +} + +func TestPatchConversationSourceAuthorCompatibilityLeavesUnexpectedRef(t *testing.T) { + var spec yaml.Node + if err := yaml.Unmarshal([]byte(` +components: + schemas: + conversation_source: + properties: + author: + $ref: '#/components/schemas/custom_author' +`), &spec); err != nil { + t.Fatalf("yaml.Unmarshal: %v", err) + } + + patchConversationSourceAuthorCompatibility(&spec) + + authorRef := lookup( + &spec, + "components", + "schemas", + "conversation_source", + "properties", + "author", + "$ref", + ) + if got, want := scalarValue(authorRef), "#/components/schemas/custom_author"; got != want { + t.Fatalf("author ref = %q, want %q", got, want) + } +} diff --git a/public_api_types_test.go b/public_api_types_test.go index 1d66982..c78f227 100644 --- a/public_api_types_test.go +++ b/public_api_types_test.go @@ -99,6 +99,26 @@ func TestArticleParentCompatibility(t *testing.T) { _ = intercom.ArticleList{Data: &[]intercom.Article{article}} } +func TestConversationSourceAuthorCompatibility(t *testing.T) { + conversation := intercom.Conversation{} + if conversation.Source == nil || conversation.Source.Author == nil { + return + } + var _ *bool = conversation.Source.Author.FromAiAgent + var _ *bool = conversation.Source.Author.IsAiAnswer +} + +func TestSearchRequestAliasCompatibility(t *testing.T) { + var conversation intercom.ConversationSearchQuery + var ticket intercom.TicketSearchQuery + var _ interface { + MarshalJSON() ([]byte, error) + } = conversation.Query + var _ interface { + UnmarshalJSON([]byte) error + } = &ticket.Query +} + func TestConversationAttributeConstructorsAreImportable(t *testing.T) { constructors := []func() error{ func() error { diff --git a/scripts/check-api-compatibility.sh b/scripts/check-api-compatibility.sh index 9ff156c..c56f4df 100755 --- a/scripts/check-api-compatibility.sh +++ b/scripts/check-api-compatibility.sh @@ -3,6 +3,7 @@ set -euo pipefail readonly module_path="github.com/uffejaeger/intercom-go" +readonly generated_package="${module_path}/internal/generated/intercom" readonly baseline="${API_BASELINE:-v0.2.0}" readonly apidiff_version="${APIDIFF_VERSION:-v0.0.0-20260727155853-b88d891fe743}" readonly apidiff="golang.org/x/exp/cmd/apidiff@${apidiff_version}" @@ -58,6 +59,19 @@ filter_reviewed_source_compatible_changes() { done } +filter_reviewed_generated_alias_changes() { + while IFS= read -r line; do + case "${line}" in + "- SearchRequestSchema: changed from SearchRequestSchema to SearchRequestSchema" | \ + "- SearchRequest_Query: changed from SearchRequest_Query to SearchRequest_Query") + ;; + *) + printf '%s\n' "${line}" + ;; + esac + done +} + compare_api() { local label="$1" local old_export="$2" @@ -83,6 +97,11 @@ compare_api() { # response-conversion regression tests verify the preserved source contract. if [[ "${label}" == "public" ]]; then report="$(printf '%s\n' "${report}" | filter_reviewed_source_compatible_changes)" + elif [[ "${label}" == "generated-alias-model" ]]; then + report="$(printf '%s\n' "${report}" | ( + cd "${repository_root}" + go run ./internal/tools/filter-generated-api-diff "${repository_root}" + ) | filter_reviewed_generated_alias_changes)" fi sed '/^Ignoring internal package /d' "${comparison_errors}" >&2 @@ -107,14 +126,19 @@ echo "Exporting public API from ${baseline}..." ( cd "${work_dir}/baseline" go run "${apidiff}" -m -w "${work_dir}/baseline.api" "${module_path}" + go run "${apidiff}" -w "${work_dir}/baseline-generated.api" "${generated_package}" ) echo "Exporting public API from the working tree..." ( cd "${repository_root}" go run "${apidiff}" -m -w "${work_dir}/current.api" "${module_path}" + go run "${apidiff}" -w "${work_dir}/current-generated.api" "${generated_package}" ) compare_api "public" "${work_dir}/baseline.api" "${work_dir}/current.api" -m +compare_api "generated-alias-model" \ + "${work_dir}/baseline-generated.api" "${work_dir}/current-generated.api" \ + -allow-internal echo "Public API is backward compatible with ${baseline}." From 3765e7e2e90b0b4a364543cf6d2c612154ee1f96 Mon Sep 17 00:00:00 2001 From: uffejaeger <71610258+uffejaeger@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:31:46 -0300 Subject: [PATCH 2/4] Include method types in alias reachability --- .../tools/filter-generated-api-diff/main.go | 61 ++++++++++++++----- .../filter-generated-api-diff/main_test.go | 20 +++++- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/internal/tools/filter-generated-api-diff/main.go b/internal/tools/filter-generated-api-diff/main.go index 42acbea..7793b92 100644 --- a/internal/tools/filter-generated-api-diff/main.go +++ b/internal/tools/filter-generated-api-diff/main.go @@ -49,7 +49,7 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { return nil, err } - declarations, err := generatedTypeDeclarations(filepath.Join(moduleRoot, "internal", "generated", "intercom")) + typeExpressions, err := generatedTypeExpressions(filepath.Join(moduleRoot, "internal", "generated", "intercom")) if err != nil { return nil, err } @@ -57,7 +57,7 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { reachable := make(map[string]struct{}, len(targets)) queue := make([]string, 0, len(targets)) for target := range targets { - if _, exists := declarations[target]; exists { + if _, exists := typeExpressions[target]; exists { queue = append(queue, target) } } @@ -71,9 +71,11 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { reachable[name] = struct{}{} dependencies := make(map[string]struct{}) - collectTypeDependencies(declarations[name], dependencies) + for _, expression := range typeExpressions[name] { + collectTypeDependencies(expression, dependencies) + } for dependency := range dependencies { - if _, exists := declarations[dependency]; exists { + if _, exists := typeExpressions[dependency]; exists { queue = append(queue, dependency) } } @@ -142,8 +144,8 @@ func generatedAliasTargets(moduleRoot string) (map[string]struct{}, error) { return targets, nil } -func generatedTypeDeclarations(directory string) (map[string]ast.Expr, error) { - declarations := make(map[string]ast.Expr) +func generatedTypeExpressions(directory string) (map[string][]ast.Expr, error) { + typeExpressions := make(map[string][]ast.Expr) fileset := token.NewFileSet() packages, err := parser.ParseDir(fileset, directory, func(info os.FileInfo) bool { return strings.HasSuffix(info.Name(), ".go") && !strings.HasSuffix(info.Name(), "_test.go") @@ -155,21 +157,52 @@ func generatedTypeDeclarations(directory string) (map[string]ast.Expr, error) { for _, pkg := range packages { for _, file := range pkg.Files { for _, declaration := range file.Decls { - general, ok := declaration.(*ast.GenDecl) - if !ok || general.Tok != token.TYPE { - continue - } - for _, rawSpec := range general.Specs { - typeSpec, ok := rawSpec.(*ast.TypeSpec) + switch declaration := declaration.(type) { + case *ast.GenDecl: + if declaration.Tok != token.TYPE { + continue + } + for _, rawSpec := range declaration.Specs { + typeSpec, ok := rawSpec.(*ast.TypeSpec) + if ok { + typeExpressions[typeSpec.Name.Name] = append(typeExpressions[typeSpec.Name.Name], typeSpec.Type) + } + } + case *ast.FuncDecl: + receiver, ok := receiverTypeName(declaration.Recv) if ok { - declarations[typeSpec.Name.Name] = typeSpec.Type + typeExpressions[receiver] = append(typeExpressions[receiver], declaration.Type) } } } } } - return declarations, nil + return typeExpressions, nil +} + +func receiverTypeName(receiver *ast.FieldList) (string, bool) { + if receiver == nil || len(receiver.List) != 1 { + return "", false + } + + expression := receiver.List[0].Type + for { + switch typed := expression.(type) { + case *ast.Ident: + return typed.Name, true + case *ast.ParenExpr: + expression = typed.X + case *ast.StarExpr: + expression = typed.X + case *ast.IndexExpr: + expression = typed.X + case *ast.IndexListExpr: + expression = typed.X + default: + return "", false + } + } } func collectTypeDependencies(expression ast.Expr, dependencies map[string]struct{}) { diff --git a/internal/tools/filter-generated-api-diff/main_test.go b/internal/tools/filter-generated-api-diff/main_test.go index 601072e..f2b0157 100644 --- a/internal/tools/filter-generated-api-diff/main_test.go +++ b/internal/tools/filter-generated-api-diff/main_test.go @@ -44,16 +44,34 @@ type Nested struct { Value string } +type MethodResult struct { + Nested *MethodNested +} + +type MethodNested struct { + Value string +} + +type MethodParameter struct { + Value string +} + type Unreachable struct { Ignored bool } + +func (Root) Result() MethodResult { + return MethodResult{} +} + +func (*Root) Accept(MethodParameter) {} `) reachable, err := reachableGeneratedTypes(root) if err != nil { t.Fatalf("reachableGeneratedTypes: %v", err) } - for _, name := range []string{"Root", "Nested"} { + for _, name := range []string{"Root", "Nested", "MethodResult", "MethodNested", "MethodParameter"} { if _, ok := reachable[name]; !ok { t.Errorf("%s is not reachable", name) } From db68c4270be81aa70469c6437c0a03c18d2b9283 Mon Sep 17 00:00:00 2001 From: uffejaeger <71610258+uffejaeger@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:48:16 -0300 Subject: [PATCH 3/4] Include direct public generated types --- .../tools/filter-generated-api-diff/main.go | 200 ++++++++++++++++-- .../filter-generated-api-diff/main_test.go | 90 +++++++- 2 files changed, 269 insertions(+), 21 deletions(-) diff --git a/internal/tools/filter-generated-api-diff/main.go b/internal/tools/filter-generated-api-diff/main.go index 7793b92..344779c 100644 --- a/internal/tools/filter-generated-api-diff/main.go +++ b/internal/tools/filter-generated-api-diff/main.go @@ -44,7 +44,7 @@ func main() { } func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { - targets, err := generatedAliasTargets(moduleRoot) + targets, err := generatedPublicTargets(moduleRoot) if err != nil { return nil, err } @@ -84,8 +84,21 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { return reachable, nil } -func generatedAliasTargets(moduleRoot string) (map[string]struct{}, error) { - targets := make(map[string]struct{}) +type rootExpression struct { + expression ast.Expr + generatedAliases map[string]struct{} +} + +type rootReferences struct { + generated map[string]struct{} + local map[string]struct{} +} + +func generatedPublicTargets(moduleRoot string) (map[string]struct{}, error) { + rootTypes := make(map[string][]rootExpression) + publicTypes := make(map[string]struct{}) + var publicExpressions []rootExpression + entries, err := os.ReadDir(moduleRoot) if err != nil { return nil, err @@ -117,25 +130,86 @@ func generatedAliasTargets(moduleRoot string) (map[string]struct{}, error) { } for _, declaration := range file.Decls { - general, ok := declaration.(*ast.GenDecl) - if !ok || general.Tok != token.TYPE { - continue - } - for _, rawSpec := range general.Specs { - typeSpec, ok := rawSpec.(*ast.TypeSpec) - if !ok || !typeSpec.Assign.IsValid() { - continue + switch declaration := declaration.(type) { + case *ast.GenDecl: + switch declaration.Tok { + case token.TYPE: + for _, rawSpec := range declaration.Specs { + typeSpec, ok := rawSpec.(*ast.TypeSpec) + if !ok { + continue + } + rootTypes[typeSpec.Name.Name] = append(rootTypes[typeSpec.Name.Name], rootExpression{ + expression: typeSpec.Type, + generatedAliases: generatedAliases, + }) + if ast.IsExported(typeSpec.Name.Name) { + publicTypes[typeSpec.Name.Name] = struct{}{} + } + } + case token.CONST, token.VAR: + for _, rawSpec := range declaration.Specs { + valueSpec, ok := rawSpec.(*ast.ValueSpec) + if !ok || valueSpec.Type == nil || !valueSpecIsExported(valueSpec) { + continue + } + publicExpressions = append(publicExpressions, rootExpression{ + expression: valueSpec.Type, + generatedAliases: generatedAliases, + }) + } } - selector, ok := typeSpec.Type.(*ast.SelectorExpr) - if !ok { + case *ast.FuncDecl: + if !ast.IsExported(declaration.Name.Name) { continue } - packageName, ok := selector.X.(*ast.Ident) - if !ok { - continue + expression := rootExpression{ + expression: declaration.Type, + generatedAliases: generatedAliases, } - if _, generated := generatedAliases[packageName.Name]; generated { - targets[selector.Sel.Name] = struct{}{} + if receiver, ok := receiverTypeName(declaration.Recv); ok { + rootTypes[receiver] = append(rootTypes[receiver], expression) + } else { + publicExpressions = append(publicExpressions, expression) + } + } + } + } + + targets := make(map[string]struct{}) + queue := make([]string, 0, len(publicTypes)) + for name := range publicTypes { + queue = append(queue, name) + } + for _, expression := range publicExpressions { + references := collectRootReferences(expression) + for generated := range references.generated { + targets[generated] = struct{}{} + } + for local := range references.local { + if _, exists := rootTypes[local]; exists { + queue = append(queue, local) + } + } + } + + visited := make(map[string]struct{}) + for len(queue) > 0 { + name := queue[0] + queue = queue[1:] + if _, seen := visited[name]; seen { + continue + } + visited[name] = struct{}{} + + for _, expression := range rootTypes[name] { + references := collectRootReferences(expression) + for generated := range references.generated { + targets[generated] = struct{}{} + } + for local := range references.local { + if _, exists := rootTypes[local]; exists { + queue = append(queue, local) } } } @@ -144,6 +218,96 @@ func generatedAliasTargets(moduleRoot string) (map[string]struct{}, error) { return targets, nil } +func collectRootReferences(root rootExpression) rootReferences { + references := rootReferences{ + generated: make(map[string]struct{}), + local: make(map[string]struct{}), + } + collectRootExpressionReferences(root.expression, root.generatedAliases, references) + return references +} + +func collectRootExpressionReferences(expression ast.Expr, generatedAliases map[string]struct{}, references rootReferences) { + switch expression := expression.(type) { + case *ast.Ident: + references.local[expression.Name] = struct{}{} + case *ast.SelectorExpr: + packageName, ok := expression.X.(*ast.Ident) + if ok { + if _, generated := generatedAliases[packageName.Name]; generated { + references.generated[expression.Sel.Name] = struct{}{} + } + } + case *ast.ParenExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + case *ast.StarExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + case *ast.ArrayType: + collectRootExpressionReferences(expression.Elt, generatedAliases, references) + case *ast.MapType: + collectRootExpressionReferences(expression.Key, generatedAliases, references) + collectRootExpressionReferences(expression.Value, generatedAliases, references) + case *ast.ChanType: + collectRootExpressionReferences(expression.Value, generatedAliases, references) + case *ast.Ellipsis: + collectRootExpressionReferences(expression.Elt, generatedAliases, references) + case *ast.IndexExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + collectRootExpressionReferences(expression.Index, generatedAliases, references) + case *ast.IndexListExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + for _, index := range expression.Indices { + collectRootExpressionReferences(index, generatedAliases, references) + } + case *ast.StructType: + for _, field := range expression.Fields.List { + if fieldIsExported(field) { + collectRootExpressionReferences(field.Type, generatedAliases, references) + } + } + case *ast.InterfaceType: + for _, method := range expression.Methods.List { + if fieldIsExported(method) { + collectRootExpressionReferences(method.Type, generatedAliases, references) + } + } + case *ast.FuncType: + collectRootFieldListReferences(expression.TypeParams, generatedAliases, references) + collectRootFieldListReferences(expression.Params, generatedAliases, references) + collectRootFieldListReferences(expression.Results, generatedAliases, references) + } +} + +func collectRootFieldListReferences(fields *ast.FieldList, generatedAliases map[string]struct{}, references rootReferences) { + if fields == nil { + return + } + for _, field := range fields.List { + collectRootExpressionReferences(field.Type, generatedAliases, references) + } +} + +func fieldIsExported(field *ast.Field) bool { + if len(field.Names) == 0 { + return true + } + for _, name := range field.Names { + if ast.IsExported(name.Name) { + return true + } + } + return false +} + +func valueSpecIsExported(spec *ast.ValueSpec) bool { + for _, name := range spec.Names { + if ast.IsExported(name.Name) { + return true + } + } + return false +} + func generatedTypeExpressions(directory string) (map[string][]ast.Expr, error) { typeExpressions := make(map[string][]ast.Expr) fileset := token.NewFileSet() diff --git a/internal/tools/filter-generated-api-diff/main_test.go b/internal/tools/filter-generated-api-diff/main_test.go index f2b0157..3cc17e8 100644 --- a/internal/tools/filter-generated-api-diff/main_test.go +++ b/internal/tools/filter-generated-api-diff/main_test.go @@ -33,6 +33,49 @@ func TestReachableGeneratedTypes(t *testing.T) { import gen "example.test/sdk/internal/generated/intercom" type Public = gen.Root + +type PublicStruct struct { + Direct gen.Direct + ViaLocal localHelper + hidden gen.HiddenField +} + +type localHelper struct { + Exposed gen.ViaLocal + hidden gen.HiddenLocalField +} + +type Service struct{} + +func (*Service) PublicMethod(gen.PublicMethodParameter) gen.PublicMethodResult { + return gen.PublicMethodResult{} +} + +func (*Service) privateMethod(gen.PrivateMethodParameter) {} + +func PublicFunction(gen.PublicFunctionParameter) gen.PublicFunctionResult { + return gen.PublicFunctionResult{} +} + +func privateFunction(gen.PrivateFunctionParameter) {} + +type privateStruct struct { + Exposed gen.PrivateStructField +} + +type inferredLocalResult struct{} + +func (inferredLocalResult) PublicMethod() gen.InferredMethodResult { + return gen.InferredMethodResult{} +} + +func PublicLocalResult() inferredLocalResult { + return inferredLocalResult{} +} + +var PublicVariable gen.PublicVariable +const PublicConstant gen.PublicConstant = "public" +var privateVariable gen.PrivateVariable `) writeTestFile(t, root, "internal/generated/intercom/models.go", `package intercom @@ -56,6 +99,22 @@ type MethodParameter struct { Value string } +type Direct struct{} +type ViaLocal struct{} +type HiddenField struct{} +type HiddenLocalField struct{} +type PublicMethodParameter struct{} +type PublicMethodResult struct{} +type PrivateMethodParameter struct{} +type PublicFunctionParameter struct{} +type PublicFunctionResult struct{} +type PrivateFunctionParameter struct{} +type PrivateStructField struct{} +type InferredMethodResult struct{} +type PublicVariable struct{} +type PublicConstant string +type PrivateVariable struct{} + type Unreachable struct { Ignored bool } @@ -71,13 +130,38 @@ func (*Root) Accept(MethodParameter) {} if err != nil { t.Fatalf("reachableGeneratedTypes: %v", err) } - for _, name := range []string{"Root", "Nested", "MethodResult", "MethodNested", "MethodParameter"} { + for _, name := range []string{ + "Root", + "Nested", + "MethodResult", + "MethodNested", + "MethodParameter", + "Direct", + "ViaLocal", + "PublicMethodParameter", + "PublicMethodResult", + "PublicFunctionParameter", + "PublicFunctionResult", + "InferredMethodResult", + "PublicVariable", + "PublicConstant", + } { if _, ok := reachable[name]; !ok { t.Errorf("%s is not reachable", name) } } - if _, ok := reachable["Unreachable"]; ok { - t.Error("Unreachable unexpectedly reported as reachable") + for _, name := range []string{ + "HiddenField", + "HiddenLocalField", + "PrivateMethodParameter", + "PrivateFunctionParameter", + "PrivateStructField", + "PrivateVariable", + "Unreachable", + } { + if _, ok := reachable[name]; ok { + t.Errorf("%s unexpectedly reported as reachable", name) + } } } From 903d3ee5e107a436697ac3a0455a32ad91223bec Mon Sep 17 00:00:00 2001 From: uffejaeger <71610258+uffejaeger@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:41:34 +0200 Subject: [PATCH 4/4] Handle inferred public generated types --- .../tools/filter-generated-api-diff/main.go | 131 ++++++++++++++---- .../filter-generated-api-diff/main_test.go | 35 +++++ 2 files changed, 142 insertions(+), 24 deletions(-) diff --git a/internal/tools/filter-generated-api-diff/main.go b/internal/tools/filter-generated-api-diff/main.go index 344779c..f06beab 100644 --- a/internal/tools/filter-generated-api-diff/main.go +++ b/internal/tools/filter-generated-api-diff/main.go @@ -1,5 +1,5 @@ // Command filter-generated-api-diff keeps apidiff findings for generated -// model types that are reachable through aliases in the public root package. +// model types that are reachable through the public root package. package main import ( @@ -49,7 +49,7 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { return nil, err } - typeExpressions, err := generatedTypeExpressions(filepath.Join(moduleRoot, "internal", "generated", "intercom")) + symbolExpressions, typeNames, err := generatedSymbolExpressions(filepath.Join(moduleRoot, "internal", "generated", "intercom")) if err != nil { return nil, err } @@ -57,25 +57,29 @@ func reachableGeneratedTypes(moduleRoot string) (map[string]struct{}, error) { reachable := make(map[string]struct{}, len(targets)) queue := make([]string, 0, len(targets)) for target := range targets { - if _, exists := typeExpressions[target]; exists { + if _, exists := symbolExpressions[target]; exists { queue = append(queue, target) } } + visited := make(map[string]struct{}) for len(queue) > 0 { name := queue[0] queue = queue[1:] - if _, seen := reachable[name]; seen { + if _, seen := visited[name]; seen { continue } - reachable[name] = struct{}{} + visited[name] = struct{}{} + if _, isType := typeNames[name]; isType { + reachable[name] = struct{}{} + } dependencies := make(map[string]struct{}) - for _, expression := range typeExpressions[name] { + for _, expression := range symbolExpressions[name] { collectTypeDependencies(expression, dependencies) } for dependency := range dependencies { - if _, exists := typeExpressions[dependency]; exists { + if _, exists := symbolExpressions[dependency]; exists { queue = append(queue, dependency) } } @@ -150,13 +154,21 @@ func generatedPublicTargets(moduleRoot string) (map[string]struct{}, error) { case token.CONST, token.VAR: for _, rawSpec := range declaration.Specs { valueSpec, ok := rawSpec.(*ast.ValueSpec) - if !ok || valueSpec.Type == nil || !valueSpecIsExported(valueSpec) { + if !ok || !valueSpecIsExported(valueSpec) { continue } - publicExpressions = append(publicExpressions, rootExpression{ - expression: valueSpec.Type, - generatedAliases: generatedAliases, - }) + if valueSpec.Type != nil { + publicExpressions = append(publicExpressions, rootExpression{ + expression: valueSpec.Type, + generatedAliases: generatedAliases, + }) + } + for _, value := range exportedValueInitializers(valueSpec) { + publicExpressions = append(publicExpressions, rootExpression{ + expression: value, + generatedAliases: generatedAliases, + }) + } } } case *ast.FuncDecl: @@ -275,6 +287,20 @@ func collectRootExpressionReferences(expression ast.Expr, generatedAliases map[s collectRootFieldListReferences(expression.TypeParams, generatedAliases, references) collectRootFieldListReferences(expression.Params, generatedAliases, references) collectRootFieldListReferences(expression.Results, generatedAliases, references) + case *ast.CompositeLit: + if expression.Type != nil { + collectRootExpressionReferences(expression.Type, generatedAliases, references) + } + case *ast.CallExpr: + collectRootExpressionReferences(expression.Fun, generatedAliases, references) + for _, argument := range expression.Args { + collectRootExpressionReferences(argument, generatedAliases, references) + } + case *ast.UnaryExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + case *ast.BinaryExpr: + collectRootExpressionReferences(expression.X, generatedAliases, references) + collectRootExpressionReferences(expression.Y, generatedAliases, references) } } @@ -308,14 +334,35 @@ func valueSpecIsExported(spec *ast.ValueSpec) bool { return false } -func generatedTypeExpressions(directory string) (map[string][]ast.Expr, error) { - typeExpressions := make(map[string][]ast.Expr) +func exportedValueInitializers(spec *ast.ValueSpec) []ast.Expr { + if len(spec.Values) == len(spec.Names) { + values := make([]ast.Expr, 0, len(spec.Values)) + for index, name := range spec.Names { + if ast.IsExported(name.Name) { + values = append(values, spec.Values[index]) + } + } + return values + } + + // A single initializer can provide multiple values. Its result types cannot + // be paired with individual names syntactically, so inspect it when any name + // in the declaration is exported. + if len(spec.Values) == 1 && valueSpecIsExported(spec) { + return spec.Values + } + return nil +} + +func generatedSymbolExpressions(directory string) (map[string][]ast.Expr, map[string]struct{}, error) { + symbolExpressions := make(map[string][]ast.Expr) + typeNames := make(map[string]struct{}) fileset := token.NewFileSet() packages, err := parser.ParseDir(fileset, directory, func(info os.FileInfo) bool { return strings.HasSuffix(info.Name(), ".go") && !strings.HasSuffix(info.Name(), "_test.go") }, 0) if err != nil { - return nil, err + return nil, nil, err } for _, pkg := range packages { @@ -323,26 +370,48 @@ func generatedTypeExpressions(directory string) (map[string][]ast.Expr, error) { for _, declaration := range file.Decls { switch declaration := declaration.(type) { case *ast.GenDecl: - if declaration.Tok != token.TYPE { - continue - } - for _, rawSpec := range declaration.Specs { - typeSpec, ok := rawSpec.(*ast.TypeSpec) - if ok { - typeExpressions[typeSpec.Name.Name] = append(typeExpressions[typeSpec.Name.Name], typeSpec.Type) + switch declaration.Tok { + case token.TYPE: + for _, rawSpec := range declaration.Specs { + typeSpec, ok := rawSpec.(*ast.TypeSpec) + if ok { + typeNames[typeSpec.Name.Name] = struct{}{} + symbolExpressions[typeSpec.Name.Name] = append(symbolExpressions[typeSpec.Name.Name], typeSpec.Type) + } + } + case token.CONST, token.VAR: + for _, rawSpec := range declaration.Specs { + valueSpec, ok := rawSpec.(*ast.ValueSpec) + if !ok { + continue + } + for index, name := range valueSpec.Names { + if valueSpec.Type != nil { + symbolExpressions[name.Name] = append(symbolExpressions[name.Name], valueSpec.Type) + } + if len(valueSpec.Values) == len(valueSpec.Names) { + symbolExpressions[name.Name] = append(symbolExpressions[name.Name], valueSpec.Values[index]) + } else if len(valueSpec.Values) == 1 { + symbolExpressions[name.Name] = append(symbolExpressions[name.Name], valueSpec.Values[0]) + } + } } } case *ast.FuncDecl: receiver, ok := receiverTypeName(declaration.Recv) if ok { - typeExpressions[receiver] = append(typeExpressions[receiver], declaration.Type) + symbolExpressions[receiver] = append(symbolExpressions[receiver], declaration.Type) + } else if declaration.Type.Results != nil { + for _, result := range declaration.Type.Results.List { + symbolExpressions[declaration.Name.Name] = append(symbolExpressions[declaration.Name.Name], result.Type) + } } } } } } - return typeExpressions, nil + return symbolExpressions, typeNames, nil } func receiverTypeName(receiver *ast.FieldList) (string, bool) { @@ -402,6 +471,20 @@ func collectTypeDependencies(expression ast.Expr, dependencies map[string]struct collectFieldListDependencies(expression.TypeParams, dependencies) collectFieldListDependencies(expression.Params, dependencies) collectFieldListDependencies(expression.Results, dependencies) + case *ast.CompositeLit: + if expression.Type != nil { + collectTypeDependencies(expression.Type, dependencies) + } + case *ast.CallExpr: + collectTypeDependencies(expression.Fun, dependencies) + for _, argument := range expression.Args { + collectTypeDependencies(argument, dependencies) + } + case *ast.UnaryExpr: + collectTypeDependencies(expression.X, dependencies) + case *ast.BinaryExpr: + collectTypeDependencies(expression.X, dependencies) + collectTypeDependencies(expression.Y, dependencies) } } diff --git a/internal/tools/filter-generated-api-diff/main_test.go b/internal/tools/filter-generated-api-diff/main_test.go index 3cc17e8..0293bc9 100644 --- a/internal/tools/filter-generated-api-diff/main_test.go +++ b/internal/tools/filter-generated-api-diff/main_test.go @@ -75,6 +75,15 @@ func PublicLocalResult() inferredLocalResult { var PublicVariable gen.PublicVariable const PublicConstant gen.PublicConstant = "public" +var InferredPublicVariable = gen.InferredPublicVariable{} +const InferredPublicConstant = gen.InferredPublicConstant("public") +const InferredPublicConstantFromValue = gen.InferredPublicConstantValue +var InferredPublicConstructed = gen.NewInferredPublicConstructed() +var privateInferredVariable = gen.PrivateInferredVariable{} +const privateInferredConstant = gen.PrivateInferredConstant("private") +const privateInferredConstantFromValue = gen.PrivateInferredConstantValue +var privateInferredConstructed = gen.NewPrivateInferredConstructed() +var InferredPublicMixed, privateInferredMixed = gen.InferredPublicMixed{}, gen.PrivateInferredMixed{} var privateVariable gen.PrivateVariable `) writeTestFile(t, root, "internal/generated/intercom/models.go", `package intercom @@ -113,6 +122,16 @@ type PrivateStructField struct{} type InferredMethodResult struct{} type PublicVariable struct{} type PublicConstant string +type InferredPublicVariable struct{} +type InferredPublicConstant string +const InferredPublicConstantValue InferredPublicConstant = "public" +type InferredPublicConstructed struct{} +type PrivateInferredVariable struct{} +type PrivateInferredConstant string +const PrivateInferredConstantValue PrivateInferredConstant = "private" +type PrivateInferredConstructed struct{} +type InferredPublicMixed struct{} +type PrivateInferredMixed struct{} type PrivateVariable struct{} type Unreachable struct { @@ -124,6 +143,14 @@ func (Root) Result() MethodResult { } func (*Root) Accept(MethodParameter) {} + +func NewInferredPublicConstructed() InferredPublicConstructed { + return InferredPublicConstructed{} +} + +func NewPrivateInferredConstructed() PrivateInferredConstructed { + return PrivateInferredConstructed{} +} `) reachable, err := reachableGeneratedTypes(root) @@ -145,6 +172,10 @@ func (*Root) Accept(MethodParameter) {} "InferredMethodResult", "PublicVariable", "PublicConstant", + "InferredPublicVariable", + "InferredPublicConstant", + "InferredPublicConstructed", + "InferredPublicMixed", } { if _, ok := reachable[name]; !ok { t.Errorf("%s is not reachable", name) @@ -156,6 +187,10 @@ func (*Root) Accept(MethodParameter) {} "PrivateMethodParameter", "PrivateFunctionParameter", "PrivateStructField", + "PrivateInferredVariable", + "PrivateInferredConstant", + "PrivateInferredConstructed", + "PrivateInferredMixed", "PrivateVariable", "Unreachable", } {