mirror of
https://github.com/lukaszraczylo/go-telegram.git
synced 2026-07-22 08:49:11 +00:00
fix(codegen): parse recapitalised Array-of return sentence
Telegram rewrote the forwardMessages and copyMessages return sentence from "an array of MessageId" to "an Array of MessageId". The pattern that handles the "... of the sent messages is returned" shape matched lowercase only, so both methods fell back to bool and the weekly regen job failed at the audit step. Add approved_any_locations to overrides.json so a union that renders as `any` by design can be approved as data instead of a code change. Approve InputRichMessageMedia.Media, a five-variant InputMedia union added in Bot API 10.2 that has no declared parent type. Make the drift test compare HEAD against itself. It compared the working tree against HEAD, so it failed whenever a regen run added a method.
This commit is contained in:
+11
-5
@@ -125,12 +125,18 @@ func TestAuditDrift_InvalidRefReturnsError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestAuditDrift_SameRefNoDrift(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"
|
irPath := "../../internal/spec/api.json"
|
||||||
cur, err := loadIR(irPath)
|
out, err := exec.Command("git", "show", "HEAD:"+irPath).Output()
|
||||||
if err != nil {
|
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.NoError(t, err)
|
||||||
require.Empty(t, changes)
|
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.Len(t, out, 1)
|
||||||
require.Contains(t, out[0], "any return: weirdMethod")
|
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.Len(t, out, 1)
|
||||||
require.Contains(t, out[0], "any param: weirdMethod.Thing")
|
require.Contains(t, out[0], "any param: weirdMethod.Thing")
|
||||||
}
|
}
|
||||||
|
|||||||
+7
-6
@@ -54,7 +54,7 @@ func main() {
|
|||||||
var problems []string
|
var problems []string
|
||||||
|
|
||||||
problems = append(problems, auditBool(api, overrides)...)
|
problems = append(problems, auditBool(api, overrides)...)
|
||||||
problems = append(problems, auditAny(api)...)
|
problems = append(problems, auditAny(api, overrides)...)
|
||||||
|
|
||||||
driftFound := false
|
driftFound := false
|
||||||
if *checkDrift {
|
if *checkDrift {
|
||||||
@@ -140,8 +140,9 @@ func looksGenuinelyBool(doc string) bool {
|
|||||||
|
|
||||||
// auditAny scans the IR for any KindOneOf TypeRef that would render as
|
// auditAny scans the IR for any KindOneOf TypeRef that would render as
|
||||||
// `any` in generated code (not matched by ChatID/InputFile-or-string/known
|
// `any` in generated code (not matched by ChatID/InputFile-or-string/known
|
||||||
// union heuristics). Reports each occurrence with location.
|
// union heuristics). Reports each occurrence with location, except the
|
||||||
func auditAny(api *spec.API) []string {
|
// locations approved in overrides.
|
||||||
|
func auditAny(api *spec.API, ov *spec.Overrides) []string {
|
||||||
var out []string
|
var out []string
|
||||||
isKnownUnion := func(variants []string) bool {
|
isKnownUnion := func(variants []string) bool {
|
||||||
if hasVariants(variants, "int64", "string") {
|
if hasVariants(variants, "int64", "string") {
|
||||||
@@ -166,17 +167,17 @@ func auditAny(api *spec.API) []string {
|
|||||||
}
|
}
|
||||||
for _, t := range api.Types {
|
for _, t := range api.Types {
|
||||||
for _, f := range t.Fields {
|
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))
|
out = append(out, fmt.Sprintf("any field: %s.%s (variants=%v)", t.Name, f.Name, f.Type.Variants))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
for _, m := range api.Methods {
|
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))
|
out = append(out, fmt.Sprintf("any return: %s (variants=%v)", m.Name, m.Returns.Variants))
|
||||||
}
|
}
|
||||||
for _, p := range m.Params {
|
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))
|
out = append(out, fmt.Sprintf("any param: %s.%s (variants=%v)", m.Name, p.Name, p.Type.Variants))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+31
-5
@@ -66,11 +66,37 @@ func TestAuditAny_FlagsUnrecognisedOneOf(t *testing.T) {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
out := auditAny(api)
|
out := auditAny(api, &spec.Overrides{})
|
||||||
require.Len(t, out, 1)
|
require.Len(t, out, 1)
|
||||||
require.Contains(t, out[0], "any field: Foo.Bar")
|
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) {
|
func TestAuditAny_SkipsChatIDShape(t *testing.T) {
|
||||||
api := &spec.API{
|
api := &spec.API{
|
||||||
Types: []spec.TypeDecl{
|
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) {
|
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) {
|
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) {
|
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 ------------------------------------------------------
|
// ---- diffSignatures ------------------------------------------------------
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ func extractReturn(desc string) spec.TypeRef {
|
|||||||
return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem}
|
return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem}
|
||||||
}},
|
}},
|
||||||
// "an array of X of the sent messages is returned" (ForwardMessages/CopyMessages shape).
|
// "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])
|
elem := primitiveOrNamed(m[1])
|
||||||
return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem}
|
return spec.TypeRef{Kind: spec.KindArray, ElemType: &elem}
|
||||||
}},
|
}},
|
||||||
|
|||||||
@@ -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"}}},
|
{"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".
|
// 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"}}},
|
{"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 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 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"}},
|
{"Returns StarAmount on success.", spec.TypeRef{Kind: spec.KindNamed, Name: "StarAmount"}},
|
||||||
|
|||||||
@@ -22,6 +22,12 @@ type Overrides struct {
|
|||||||
// ApprovedBoolMethods lists methods whose returns are genuinely bool.
|
// ApprovedBoolMethods lists methods whose returns are genuinely bool.
|
||||||
// The audit tool ignores these.
|
// The audit tool ignores these.
|
||||||
ApprovedBoolMethods []string `json:"approved_bool_methods,omitempty"`
|
ApprovedBoolMethods []string `json:"approved_bool_methods,omitempty"`
|
||||||
|
|
||||||
|
// ApprovedAnyLocations lists IR locations whose union renders as `any` by
|
||||||
|
// design, keyed "<TypeName>.<FieldName>" for struct fields,
|
||||||
|
// "<methodName>.<ParamName>" for params and "<methodName>" for returns.
|
||||||
|
// The audit tool ignores these.
|
||||||
|
ApprovedAnyLocations []string `json:"approved_any_locations,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// LoadOverrides reads and parses overrides.json. Returns an empty Overrides
|
// LoadOverrides reads and parses overrides.json. Returns an empty Overrides
|
||||||
@@ -73,3 +79,17 @@ func (o *Overrides) IsBoolApproved(methodName string) bool {
|
|||||||
}
|
}
|
||||||
return false
|
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
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
{
|
{
|
||||||
"method_returns": {},
|
"method_returns": {},
|
||||||
"field_types": {},
|
"field_types": {},
|
||||||
|
"approved_any_locations": [
|
||||||
|
"InputRichMessageMedia.Media"
|
||||||
|
],
|
||||||
"approved_bool_methods": [
|
"approved_bool_methods": [
|
||||||
"setWebhook",
|
"setWebhook",
|
||||||
"deleteWebhook",
|
"deleteWebhook",
|
||||||
|
|||||||
Reference in New Issue
Block a user