diff --git a/cmd/audit/extra_test.go b/cmd/audit/extra_test.go index 723a975..3bd0186 100644 --- a/cmd/audit/extra_test.go +++ b/cmd/audit/extra_test.go @@ -125,12 +125,18 @@ func TestAuditDrift_InvalidRefReturnsError(t *testing.T) { } func TestAuditDrift_SameRefNoDrift(t *testing.T) { + // Compare HEAD's IR against itself, not against the working tree: during a + // regen run the working tree legitimately carries new methods, and drift + // against HEAD is the expected outcome rather than a failure. irPath := "../../internal/spec/api.json" - cur, err := loadIR(irPath) + out, err := exec.Command("git", "show", "HEAD:"+irPath).Output() if err != nil { - t.Skip("api.json not available, skipping drift test") + t.Skip("api.json not available at HEAD, skipping drift test") } - changes, err := auditDrift(irPath, "HEAD", cur) + var head spec.API + require.NoError(t, json.Unmarshal(out, &head)) + + changes, err := auditDrift(irPath, "HEAD", &head) require.NoError(t, err) require.Empty(t, changes) } @@ -166,7 +172,7 @@ func TestAuditAny_FlagsUnknownMethodReturn(t *testing.T) { }, }, } - out := auditAny(api) + out := auditAny(api, &spec.Overrides{}) require.Len(t, out, 1) require.Contains(t, out[0], "any return: weirdMethod") } @@ -182,7 +188,7 @@ func TestAuditAny_FlagsUnknownMethodParam(t *testing.T) { }, }, } - out := auditAny(api) + out := auditAny(api, &spec.Overrides{}) require.Len(t, out, 1) require.Contains(t, out[0], "any param: weirdMethod.Thing") } diff --git a/cmd/audit/main.go b/cmd/audit/main.go index 8865e63..0a10d6f 100644 --- a/cmd/audit/main.go +++ b/cmd/audit/main.go @@ -54,7 +54,7 @@ func main() { var problems []string problems = append(problems, auditBool(api, overrides)...) - problems = append(problems, auditAny(api)...) + problems = append(problems, auditAny(api, overrides)...) driftFound := false if *checkDrift { @@ -140,8 +140,9 @@ func looksGenuinelyBool(doc string) bool { // auditAny scans the IR for any KindOneOf TypeRef that would render as // `any` in generated code (not matched by ChatID/InputFile-or-string/known -// union heuristics). Reports each occurrence with location. -func auditAny(api *spec.API) []string { +// union heuristics). Reports each occurrence with location, except the +// locations approved in overrides. +func auditAny(api *spec.API, ov *spec.Overrides) []string { var out []string isKnownUnion := func(variants []string) bool { if hasVariants(variants, "int64", "string") { @@ -166,17 +167,17 @@ func auditAny(api *spec.API) []string { } for _, t := range api.Types { for _, f := range t.Fields { - if isAny(f.Type) { + if isAny(f.Type) && !ov.IsAnyApproved(t.Name+"."+f.Name) { out = append(out, fmt.Sprintf("any field: %s.%s (variants=%v)", t.Name, f.Name, f.Type.Variants)) } } } for _, m := range api.Methods { - if isAny(m.Returns) { + if isAny(m.Returns) && !ov.IsAnyApproved(m.Name) { out = append(out, fmt.Sprintf("any return: %s (variants=%v)", m.Name, m.Returns.Variants)) } for _, p := range m.Params { - if isAny(p.Type) { + if isAny(p.Type) && !ov.IsAnyApproved(m.Name+"."+p.Name) { out = append(out, fmt.Sprintf("any param: %s.%s (variants=%v)", m.Name, p.Name, p.Type.Variants)) } } diff --git a/cmd/audit/main_test.go b/cmd/audit/main_test.go index d34f81d..e2b5b26 100644 --- a/cmd/audit/main_test.go +++ b/cmd/audit/main_test.go @@ -66,11 +66,37 @@ func TestAuditAny_FlagsUnrecognisedOneOf(t *testing.T) { }, }, } - out := auditAny(api) + out := auditAny(api, &spec.Overrides{}) require.Len(t, out, 1) require.Contains(t, out[0], "any field: Foo.Bar") } +func TestAuditAny_SkipsApprovedLocations(t *testing.T) { + api := &spec.API{ + Types: []spec.TypeDecl{ + { + Name: "Foo", + Fields: []spec.Field{ + {Name: "Bar", JSONName: "bar", Type: spec.TypeRef{Kind: spec.KindOneOf, Variants: []string{"A", "B"}}}, + }, + }, + }, + Methods: []spec.MethodDecl{ + { + Name: "doThing", + Returns: spec.TypeRef{Kind: spec.KindOneOf, Variants: []string{"A", "B"}}, + Params: []spec.Field{ + {Name: "Baz", JSONName: "baz", Type: spec.TypeRef{Kind: spec.KindOneOf, Variants: []string{"A", "B"}}}, + }, + }, + }, + } + require.Len(t, auditAny(api, &spec.Overrides{}), 3) + + ov := &spec.Overrides{ApprovedAnyLocations: []string{"Foo.Bar", "doThing", "doThing.Baz"}} + require.Empty(t, auditAny(api, ov)) +} + func TestAuditAny_SkipsChatIDShape(t *testing.T) { api := &spec.API{ Types: []spec.TypeDecl{ @@ -82,7 +108,7 @@ func TestAuditAny_SkipsChatIDShape(t *testing.T) { }, }, } - require.Empty(t, auditAny(api)) + require.Empty(t, auditAny(api, &spec.Overrides{})) } func TestAuditAny_SkipsKnownUnion(t *testing.T) { @@ -97,7 +123,7 @@ func TestAuditAny_SkipsKnownUnion(t *testing.T) { }, }, } - require.Empty(t, auditAny(api)) + require.Empty(t, auditAny(api, &spec.Overrides{})) } func TestAuditAny_SkipsReplyMarkupShape(t *testing.T) { @@ -111,7 +137,7 @@ func TestAuditAny_SkipsReplyMarkupShape(t *testing.T) { }, }, } - require.Empty(t, auditAny(api)) + require.Empty(t, auditAny(api, &spec.Overrides{})) } func TestAuditAny_SkipsInputFileShape(t *testing.T) { @@ -125,7 +151,7 @@ func TestAuditAny_SkipsInputFileShape(t *testing.T) { }, }, } - require.Empty(t, auditAny(api)) + require.Empty(t, auditAny(api, &spec.Overrides{})) } // ---- diffSignatures ------------------------------------------------------ diff --git a/cmd/scrape/method.go b/cmd/scrape/method.go index 8d16e38..ade58db 100644 --- a/cmd/scrape/method.go +++ b/cmd/scrape/method.go @@ -42,7 +42,7 @@ func extractReturn(desc string) spec.TypeRef { return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem} }}, // "an array of X of the sent messages is returned" (ForwardMessages/CopyMessages shape). - {regexp.MustCompile(`(?:[Oo]n success[,.]?\s+)?an? array of ([A-Z][A-Za-z0-9]+)(?:\s+of [^.]+?)?\s+(?:objects\s+)?(?:is|are) returned`), func(m []string) spec.TypeRef { + {regexp.MustCompile(`(?:[Oo]n success[,.]?\s+)?an? [Aa]rray of ([A-Z][A-Za-z0-9]+)(?:\s+of [^.]+?)?\s+(?:objects\s+)?(?:is|are) returned`), func(m []string) spec.TypeRef { elem := primitiveOrNamed(m[1]) return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem} }}, diff --git a/cmd/scrape/method_test.go b/cmd/scrape/method_test.go index d8997ae..f3165e7 100644 --- a/cmd/scrape/method_test.go +++ b/cmd/scrape/method_test.go @@ -29,6 +29,8 @@ func TestExtractReturn(t *testing.T) { {"On success, an array of Message objects that were sent is returned.", spec.TypeRef{Kind: spec.KindArray, ElemType: &spec.TypeRef{Kind: spec.KindNamed, Name: "Message"}}}, // ForwardMessages/CopyMessages shape: "an array of X of the sent messages is returned". {"On success, an array of MessageId of the sent messages is returned.", spec.TypeRef{Kind: spec.KindArray, ElemType: &spec.TypeRef{Kind: spec.KindNamed, Name: "MessageId"}}}, + // Bot API 10.1 recapitalised the same sentence to "an Array of X". + {"On success, an Array of MessageId of the sent messages is returned.", spec.TypeRef{Kind: spec.KindArray, ElemType: &spec.TypeRef{Kind: spec.KindNamed, Name: "MessageId"}}}, // "Returns X on success" (no article) — OwnedGifts, StarAmount, Story, MenuButton, etc. {"Returns the gifts received and owned by a managed business account. Returns OwnedGifts on success.", spec.TypeRef{Kind: spec.KindNamed, Name: "OwnedGifts"}}, {"Returns StarAmount on success.", spec.TypeRef{Kind: spec.KindNamed, Name: "StarAmount"}}, diff --git a/internal/spec/overrides.go b/internal/spec/overrides.go index f07e46e..56b89ee 100644 --- a/internal/spec/overrides.go +++ b/internal/spec/overrides.go @@ -22,6 +22,12 @@ type Overrides struct { // ApprovedBoolMethods lists methods whose returns are genuinely bool. // The audit tool ignores these. ApprovedBoolMethods []string `json:"approved_bool_methods,omitempty"` + + // ApprovedAnyLocations lists IR locations whose union renders as `any` by + // design, keyed "." for struct fields, + // "." for params and "" for returns. + // The audit tool ignores these. + ApprovedAnyLocations []string `json:"approved_any_locations,omitempty"` } // LoadOverrides reads and parses overrides.json. Returns an empty Overrides @@ -73,3 +79,17 @@ func (o *Overrides) IsBoolApproved(methodName string) bool { } return false } + +// IsAnyApproved reports whether location is on the approved `any` list. +// See ApprovedAnyLocations for the key format. +func (o *Overrides) IsAnyApproved(location string) bool { + if o == nil { + return false + } + for _, n := range o.ApprovedAnyLocations { + if n == location { + return true + } + } + return false +} diff --git a/internal/spec/overrides.json b/internal/spec/overrides.json index 19fafc7..79bca0c 100644 --- a/internal/spec/overrides.json +++ b/internal/spec/overrides.json @@ -1,6 +1,9 @@ { "method_returns": {}, "field_types": {}, + "approved_any_locations": [ + "InputRichMessageMedia.Media" + ], "approved_bool_methods": [ "setWebhook", "deleteWebhook",