diff --git a/api_parity_test.go b/api_parity_test.go index c11cf213..3448619a 100644 --- a/api_parity_test.go +++ b/api_parity_test.go @@ -18,8 +18,17 @@ import ( ) type parityDoc struct { - Methods map[string]json.RawMessage `json:"methods"` - Types map[string]parityType `json:"types"` + Methods map[string]parityMethod `json:"methods"` + Types map[string]parityType `json:"types"` +} + +type parityMethod struct { + Parameters []parityParameter `json:"parameters"` + Fields []parityParameter `json:"fields"` +} + +type parityParameter struct { + Name string `json:"name"` } type parityType struct { @@ -34,6 +43,13 @@ type parityTypeDecl struct { Expr ast.Expr } +func (method parityMethod) parameterFields() []parityParameter { + if len(method.Parameters) > 0 { + return method.Parameters + } + return method.Fields +} + func TestAPIParityMethods(t *testing.T) { doc := loadParityDoc(t) index := loadPackageIndex(t) @@ -65,6 +81,202 @@ func TestAPIParityMethods(t *testing.T) { } } +func TestAPIParityMethodParameters(t *testing.T) { + doc := loadParityDoc(t) + index := loadPackageIndex(t) + + allowedExtraParams := map[string]map[string]string{ + "approveSuggestedPost": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "banChatSenderChat": { + "until_date": "legacy compatibility field", + }, + "closeGeneralForumTopic": { + "message_thread_id": "promoted legacy forum-topic field", + }, + "copyMessage": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "copyMessages": { + "allow_paid_broadcast": "shared forward/copy compatibility field", + "business_connection_id": "shared forward/copy compatibility field", + "message_effect_id": "shared forward/copy compatibility field", + "reply_markup": "shared forward/copy compatibility field", + "reply_parameters": "shared forward/copy compatibility field", + "suggested_post_parameters": "shared forward/copy compatibility field", + }, + "declineSuggestedPost": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "deleteBusinessMessages": { + "chat_id": "promoted legacy chat field", + }, + "deleteMessage": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "deleteMessageReaction": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "deleteMessages": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "editGeneralForumTopic": { + "message_thread_id": "promoted legacy forum-topic field", + }, + "forwardMessage": { + "allow_paid_broadcast": "shared forward/copy compatibility field", + "business_connection_id": "shared forward/copy compatibility field", + "reply_markup": "shared forward/copy compatibility field", + "reply_parameters": "shared forward/copy compatibility field", + }, + "forwardMessages": { + "allow_paid_broadcast": "shared forward/copy compatibility field", + "business_connection_id": "shared forward/copy compatibility field", + "message_effect_id": "shared forward/copy compatibility field", + "reply_markup": "shared forward/copy compatibility field", + "reply_parameters": "shared forward/copy compatibility field", + "suggested_post_parameters": "shared forward/copy compatibility field", + }, + "getGameHighScores": { + "business_connection_id": "promoted edit/game compatibility field", + }, + "hideGeneralForumTopic": { + "message_thread_id": "promoted legacy forum-topic field", + }, + "reopenGeneralForumTopic": { + "message_thread_id": "promoted legacy forum-topic field", + }, + "sendChatAction": { + "allow_paid_broadcast": "promoted BaseChat field", + "direct_messages_topic_id": "promoted BaseChat field", + "disable_notification": "promoted BaseChat field", + "message_effect_id": "promoted BaseChat field", + "protect_content": "promoted BaseChat field", + "reply_markup": "promoted BaseChat field", + "reply_parameters": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "sendChecklist": { + "allow_paid_broadcast": "promoted BaseChat field", + "direct_messages_topic_id": "promoted BaseChat field", + "message_thread_id": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "sendGame": { + "direct_messages_topic_id": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "sendInvoice": { + "business_connection_id": "promoted BaseChat field", + }, + "sendMediaGroup": { + "reply_markup": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "sendPaidMedia": { + "message_effect_id": "promoted BaseChat field", + }, + "sendPhoto": { + "thumbnail": "legacy compatibility field", + }, + "sendPoll": { + "direct_messages_topic_id": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "sendVoice": { + "thumbnail": "legacy compatibility alias for older Bot API docs", + }, + "setChatPhoto": { + "allow_paid_broadcast": "promoted BaseChat field", + "business_connection_id": "promoted BaseChat field", + "direct_messages_topic_id": "promoted BaseChat field", + "disable_notification": "promoted BaseChat field", + "message_effect_id": "promoted BaseChat field", + "message_thread_id": "promoted BaseChat field", + "protect_content": "promoted BaseChat field", + "reply_markup": "promoted BaseChat field", + "reply_parameters": "promoted BaseChat field", + "suggested_post_parameters": "promoted BaseChat field", + }, + "setGameScore": { + "business_connection_id": "promoted edit/game compatibility field", + }, + "setMessageReaction": { + "business_connection_id": "implemented before current docs listed it for this method", + }, + "stopPoll": { + "inline_message_id": "legacy edit-style compatibility field", + }, + "unhideGeneralForumTopic": { + "message_thread_id": "promoted legacy forum-topic field", + }, + "unpinAllGeneralForumTopicMessages": { + "message_thread_id": "promoted legacy forum-topic field", + }, + } + + missingParams := make(map[string][]string) + extraParams := make(map[string][]string) + + for methodName, method := range doc.Methods { + implemented, ok := index.collectMethodParams(methodName) + if !ok { + continue + } + + expected := make(map[string]struct{}) + for _, parameter := range method.parameterFields() { + expected[parameter.Name] = struct{}{} + if _, exists := implemented[parameter.Name]; !exists { + missingParams[methodName] = append(missingParams[methodName], parameter.Name) + } + } + + for parameter := range implemented { + if _, exists := expected[parameter]; exists { + continue + } + if _, allowed := allowedExtraParams[methodName][parameter]; allowed { + continue + } + extraParams[methodName] = append(extraParams[methodName], parameter) + } + } + + unusedAllowed := unusedAllowedMethodParams(allowedExtraParams, index, doc) + for _, params := range missingParams { + sort.Strings(params) + } + for _, params := range extraParams { + sort.Strings(params) + } + + missingMethods := sortedMapKeys(missingParams) + extraMethods := sortedMapKeys(extraParams) + sort.Strings(unusedAllowed) + + if len(missingMethods) > 0 || len(extraMethods) > 0 || len(unusedAllowed) > 0 { + builder := strings.Builder{} + if len(missingMethods) > 0 { + builder.WriteString("methods with missing params:\n") + for _, methodName := range missingMethods { + builder.WriteString(fmt.Sprintf("- %s: %v\n", methodName, missingParams[methodName])) + } + } + if len(extraMethods) > 0 { + builder.WriteString("methods with unexpected params:\n") + for _, methodName := range extraMethods { + builder.WriteString(fmt.Sprintf("- %s: %v\n", methodName, extraParams[methodName])) + } + } + if len(unusedAllowed) > 0 { + builder.WriteString(fmt.Sprintf("allowed extra params no longer needed: %v\n", unusedAllowed)) + } + t.Fatalf("method parameter parity failed\n%s", builder.String()) + } +} + func TestAPIParityTypesAndFields(t *testing.T) { doc := loadParityDoc(t) index := loadPackageIndex(t) @@ -147,8 +359,11 @@ func TestAPIParityTypesAndFields(t *testing.T) { } type packageIndex struct { - types map[string]parityTypeDecl - methodNames map[string]struct{} + types map[string]parityTypeDecl + methodNames map[string]struct{} + methodReceivers map[string]string + paramsFuncs map[string]*ast.FuncDecl + filesFuncs map[string]*ast.FuncDecl } func loadPackageIndex(t *testing.T) *packageIndex { @@ -174,8 +389,11 @@ func loadPackageIndex(t *testing.T) *packageIndex { fset := token.NewFileSet() index := &packageIndex{ - types: make(map[string]parityTypeDecl), - methodNames: make(map[string]struct{}), + types: make(map[string]parityTypeDecl), + methodNames: make(map[string]struct{}), + methodReceivers: make(map[string]string), + paramsFuncs: make(map[string]*ast.FuncDecl), + filesFuncs: make(map[string]*ast.FuncDecl), } for _, file := range goFiles { @@ -195,30 +413,41 @@ func loadPackageIndex(t *testing.T) *packageIndex { index.types[typeSpec.Name.Name] = parityTypeDecl{Expr: typeSpec.Type} } case *ast.FuncDecl: - if current.Recv == nil || current.Name == nil || current.Name.Name != "method" { + if current.Recv == nil || current.Name == nil { continue } - if current.Body == nil || len(current.Body.List) == 0 { + receiver, ok := receiverTypeName(current.Recv) + if !ok { continue } - - for _, statement := range current.Body.List { - returnStatement, ok := statement.(*ast.ReturnStmt) - if !ok || len(returnStatement.Results) != 1 { + switch current.Name.Name { + case "method": + if current.Body == nil || len(current.Body.List) == 0 { continue } + for _, statement := range current.Body.List { + returnStatement, ok := statement.(*ast.ReturnStmt) + if !ok || len(returnStatement.Results) != 1 { + continue + } - literal, ok := returnStatement.Results[0].(*ast.BasicLit) - if !ok || literal.Kind != token.STRING { - continue - } + literal, ok := returnStatement.Results[0].(*ast.BasicLit) + if !ok || literal.Kind != token.STRING { + continue + } - methodName, err := strconv.Unquote(literal.Value) - if err != nil { - t.Fatalf("unquote method literal in %s: %v", file, err) + methodName, err := strconv.Unquote(literal.Value) + if err != nil { + t.Fatalf("unquote method literal in %s: %v", file, err) + } + index.methodNames[methodName] = struct{}{} + index.methodReceivers[methodName] = receiver + break } - index.methodNames[methodName] = struct{}{} - break + case "params": + index.paramsFuncs[receiver] = current + case "files": + index.filesFuncs[receiver] = current } } } @@ -262,6 +491,298 @@ func loadParityDoc(t *testing.T) parityDoc { return doc } +func receiverTypeName(recv *ast.FieldList) (string, bool) { + if recv == nil || len(recv.List) == 0 { + return "", false + } + return exprTypeName(recv.List[0].Type) +} + +func exprTypeName(expr ast.Expr) (string, bool) { + switch current := expr.(type) { + case *ast.Ident: + return current.Name, true + case *ast.StarExpr: + return exprTypeName(current.X) + case *ast.IndexExpr: + return exprTypeName(current.X) + case *ast.IndexListExpr: + return exprTypeName(current.X) + case *ast.SelectorExpr: + return current.Sel.Name, true + default: + return "", false + } +} + +func sortedMapKeys[V any](values map[string]V) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func unusedAllowedMethodParams(allowed map[string]map[string]string, index *packageIndex, doc parityDoc) []string { + unused := make([]string, 0) + for methodName, params := range allowed { + implemented, ok := index.collectMethodParams(methodName) + if !ok { + for param := range params { + unused = append(unused, methodName+"."+param) + } + continue + } + expected := make(map[string]struct{}) + for _, parameter := range doc.Methods[methodName].parameterFields() { + expected[parameter.Name] = struct{}{} + } + for param := range params { + if _, exists := implemented[param]; !exists { + unused = append(unused, methodName+"."+param) + continue + } + if _, exists := expected[param]; exists { + unused = append(unused, methodName+"."+param) + } + } + } + return unused +} + +func (index *packageIndex) collectMethodParams(methodName string) (map[string]struct{}, bool) { + receiver, ok := index.methodReceivers[methodName] + if !ok { + switch methodName { + case "getMe", "getWebhookInfo": + return map[string]struct{}{}, true + default: + return nil, false + } + } + + params, ok := index.collectParamsByType(receiver, map[string]bool{}) + if !ok { + params = make(map[string]struct{}) + } + files := index.collectFileParamsByType(receiver) + for name := range files { + params[name] = struct{}{} + } + return params, true +} + +func (index *packageIndex) collectParamsByType(typeName string, path map[string]bool) (map[string]struct{}, bool) { + if path[typeName] { + return nil, false + } + path[typeName] = true + defer delete(path, typeName) + + if paramsFunc, ok := index.paramsFuncs[typeName]; ok { + params := make(map[string]struct{}) + ast.Inspect(paramsFunc.Body, func(node ast.Node) bool { + switch current := node.(type) { + case *ast.CallExpr: + index.collectCallParam(typeName, current, params, path) + case *ast.IndexExpr: + if key, ok := stringIndexKey(current); ok { + params[key] = struct{}{} + } + } + return true + }) + return params, true + } + + embedded := index.embeddedTypeNames(typeName) + if len(embedded) == 0 { + return nil, false + } + + params := make(map[string]struct{}) + for _, embeddedType := range embedded { + embeddedParams, ok := index.collectParamsByType(embeddedType, path) + if !ok { + continue + } + for name := range embeddedParams { + params[name] = struct{}{} + } + } + return params, true +} + +func (index *packageIndex) collectCallParam(receiverType string, call *ast.CallExpr, params map[string]struct{}, path map[string]bool) { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + + switch selector.Sel.Name { + case "AddNonEmpty", "AddNonZero", "AddNonZero64", "AddNonZeroFloat", "AddBool", "AddBoolPtr", "AddInterface", "AddFirstValid", "paramsWithKey": + if key, ok := firstStringArg(call); ok { + params[key] = struct{}{} + } + case "params": + calledType, ok := index.paramReceiverExprType(receiverType, selector.X) + if !ok { + return + } + nested, ok := index.collectParamsByType(calledType, path) + if !ok { + return + } + for name := range nested { + params[name] = struct{}{} + } + } +} + +func (index *packageIndex) collectFileParamsByType(typeName string) map[string]struct{} { + params := make(map[string]struct{}) + filesFunc, ok := index.filesFuncs[typeName] + if !ok { + return params + } + + ast.Inspect(filesFunc.Body, func(node ast.Node) bool { + switch current := node.(type) { + case *ast.CallExpr: + selector, ok := current.Fun.(*ast.Ident) + if !ok || selector.Name != "requestFile" { + return true + } + if key, ok := firstStringArg(current); ok { + params[key] = struct{}{} + } + case *ast.KeyValueExpr: + ident, ok := current.Key.(*ast.Ident) + if !ok || ident.Name != "Name" { + return true + } + if key, ok := stringLiteralValue(current.Value); ok { + params[key] = struct{}{} + } + } + return true + }) + + return params +} + +func (index *packageIndex) paramReceiverExprType(receiverType string, expr ast.Expr) (string, bool) { + switch current := expr.(type) { + case *ast.Ident: + return receiverType, true + case *ast.SelectorExpr: + baseType, ok := index.paramReceiverExprType(receiverType, current.X) + if !ok { + return "", false + } + return index.structFieldType(baseType, current.Sel.Name) + default: + return "", false + } +} + +func (index *packageIndex) structFieldType(typeName, fieldName string) (string, bool) { + decl, ok := index.types[typeName] + if !ok { + return "", false + } + + structType, ok := index.structTypeByExpr(decl.Expr) + if !ok { + return "", false + } + + for _, field := range structType.Fields.List { + fieldType, ok := exprTypeName(field.Type) + if !ok { + continue + } + if len(field.Names) == 0 { + if fieldType == fieldName { + return fieldType, true + } + if nestedType, ok := index.structFieldType(fieldType, fieldName); ok { + return nestedType, true + } + continue + } + for _, name := range field.Names { + if name.Name == fieldName { + return fieldType, true + } + } + } + + return "", false +} + +func (index *packageIndex) embeddedTypeNames(typeName string) []string { + decl, ok := index.types[typeName] + if !ok { + return nil + } + + structType, ok := index.structTypeByExpr(decl.Expr) + if !ok { + return nil + } + + names := make([]string, 0) + for _, field := range structType.Fields.List { + if len(field.Names) != 0 { + continue + } + if name, ok := exprTypeName(field.Type); ok { + names = append(names, name) + } + } + return names +} + +func (index *packageIndex) structTypeByExpr(expr ast.Expr) (*ast.StructType, bool) { + switch current := expr.(type) { + case *ast.StructType: + return current, true + case *ast.Ident: + decl, ok := index.types[current.Name] + if !ok { + return nil, false + } + return index.structTypeByExpr(decl.Expr) + default: + return nil, false + } +} + +func firstStringArg(call *ast.CallExpr) (string, bool) { + if len(call.Args) == 0 { + return "", false + } + return stringLiteralValue(call.Args[0]) +} + +func stringIndexKey(index *ast.IndexExpr) (string, bool) { + return stringLiteralValue(index.Index) +} + +func stringLiteralValue(expr ast.Expr) (string, bool) { + literal, ok := expr.(*ast.BasicLit) + if !ok || literal.Kind != token.STRING { + return "", false + } + value, err := strconv.Unquote(literal.Value) + if err != nil { + return "", false + } + return value, true +} + func (index *packageIndex) collectJSONFields(typeName string) (map[string]struct{}, bool) { return index.collectJSONFieldsByExpr(&ast.Ident{Name: typeName}, map[string]bool{}) } diff --git a/configs.go b/configs.go index 99cd5043..605354ab 100644 --- a/configs.go +++ b/configs.go @@ -318,7 +318,7 @@ func (config MessageConfig) params() (Params, error) { return params, err } - params.AddNonEmpty("text", config.Text) + params["text"] = config.Text params.AddNonEmpty("parse_mode", config.ParseMode) err = params.AddInterface("entities", config.Entities) if err != nil { @@ -376,7 +376,7 @@ func (config SendMessageDraftConfig) params() (Params, error) { params.AddNonZero("message_thread_id", config.MessageThreadID) params.AddNonZero("draft_id", config.DraftID) - params["text"] = config.Text + params.AddNonEmpty("text", config.Text) params.AddNonEmpty("parse_mode", config.ParseMode) err = params.AddInterface("entities", config.Entities) @@ -884,6 +884,7 @@ type PaidMediaConfig struct { BaseChat StarCount int64 Media *InputPaidMedia + Payload string Caption string // optional ParseMode string // optional CaptionEntities []MessageEntity // optional @@ -897,6 +898,7 @@ func (config PaidMediaConfig) params() (Params, error) { } params.AddNonZero64("star_count", config.StarCount) + params.AddNonEmpty("payload", config.Payload) params.AddNonEmpty("caption", config.Caption) params.AddNonEmpty("parse_mode", config.ParseMode) params.AddBool("show_caption_above_media", config.ShowCaptionAboveMedia) @@ -1307,6 +1309,7 @@ type EditMessageTextConfig struct { ParseMode string Entities []MessageEntity LinkPreviewOptions LinkPreviewOptions + RichMessage InputRichMessage } func (config EditMessageTextConfig) params() (Params, error) { @@ -1315,12 +1318,18 @@ func (config EditMessageTextConfig) params() (Params, error) { return params, err } - params["text"] = config.Text + params.AddNonEmpty("text", config.Text) params.AddNonEmpty("parse_mode", config.ParseMode) err = params.AddInterface("entities", config.Entities) if err != nil { return params, err } + if config.RichMessage != (InputRichMessage{}) { + err = params.AddInterface("rich_message", config.RichMessage) + if err != nil { + return params, err + } + } err = params.AddInterface("link_preview_options", config.LinkPreviewOptions) return params, err @@ -3303,7 +3312,7 @@ func (config UploadStickerConfig) params() (Params, error) { } func (config UploadStickerConfig) files() []RequestFile { - return []RequestFile{config.Sticker} + return requestFiles(RequestFile{Name: "sticker", Data: config.Sticker.Data}) } // NewStickerSetConfig allows creating a new sticker set. @@ -3399,7 +3408,7 @@ func (config SetCustomEmojiStickerSetThumbnailConfig) params() (Params, error) { params := make(Params) params["name"] = config.Name - params.AddNonEmpty("position", config.CustomEmojiID) + params.AddNonEmpty("custom_emoji_id", config.CustomEmojiID) return params, nil } @@ -3536,7 +3545,7 @@ func (config SetStickerMaskPositionConfig) params() (Params, error) { params := make(Params) params["sticker"] = config.Sticker - err := params.AddInterface("keywords", config.MaskPosition) + err := params.AddInterface("mask_position", config.MaskPosition) return params, err } diff --git a/configs_test.go b/configs_test.go index fe566d21..fea6f2eb 100644 --- a/configs_test.go +++ b/configs_test.go @@ -1462,6 +1462,75 @@ func TestAPIParityRegressionFixes(t *testing.T) { if got := (ChatMemberCountConfig{}).method(); got != "getChatMemberCount" { t.Fatalf("expected getChatMemberCount method, got %q", got) } + + editText := EditMessageTextConfig{ + BaseEdit: BaseEdit{ + BaseChatMessage: BaseChatMessage{ + ChatConfig: ChatConfig{ChatID: 1}, + MessageID: 2, + }, + }, + RichMessage: NewInputRichMessageMarkdown("**updated**"), + } + params, err = editText.params() + if err != nil { + t.Fatalf("editMessageText params error: %v", err) + } + if !strings.Contains(params["rich_message"], `"markdown":"**updated**"`) { + t.Fatalf("expected rich_message param, got %#v", params) + } + if _, ok := params["text"]; ok { + t.Fatalf("unexpected empty text param with rich_message: %#v", params) + } + + paidPhotoMedia := NewInputMediaPhoto(FileID("paid-photo-id")) + paid := NewInputPaidMediaPhoto(&paidPhotoMedia) + paidMedia := NewPaidMedia(1, 10, &paid) + paidMedia.Payload = "paid-payload" + params, err = paidMedia.params() + if err != nil { + t.Fatalf("sendPaidMedia params error: %v", err) + } + if params["payload"] != "paid-payload" { + t.Fatalf("expected payload param, got %#v", params) + } + + customEmojiThumbnail := NewCustomEmojiStickerSetThumbnal("emoji_set", "custom-emoji-id") + params, err = customEmojiThumbnail.params() + if err != nil { + t.Fatalf("setCustomEmojiStickerSetThumbnail params error: %v", err) + } + if params["custom_emoji_id"] != "custom-emoji-id" { + t.Fatalf("expected custom_emoji_id param, got %#v", params) + } + if _, ok := params["position"]; ok { + t.Fatalf("unexpected position param in custom emoji thumbnail params") + } + + maskPosition := SetStickerMaskPositionConfig{ + Sticker: "sticker-file-id", + MaskPosition: &MaskPosition{Point: "forehead", XShift: 0.1, YShift: 0.2, Scale: 1.3}, + } + params, err = maskPosition.params() + if err != nil { + t.Fatalf("setStickerMaskPosition params error: %v", err) + } + if !strings.Contains(params["mask_position"], `"point":"forehead"`) { + t.Fatalf("expected mask_position param, got %#v", params) + } + if _, ok := params["keywords"]; ok { + t.Fatalf("unexpected keywords param in sticker mask position params") + } + + uploadSticker := UploadStickerConfig{ + UserID: 42, + Sticker: RequestFile{Name: "custom-name", Data: FileBytes{Name: "sticker.webp", Bytes: []byte("sticker")}}, + StickerFormat: "static", + } + files := uploadSticker.files() + if len(files) != 1 || files[0].Name != "sticker" { + t.Fatalf("expected uploadStickerFile sticker file field, got %+v", files) + } } func TestMediaGroupConfig(t *testing.T) {