Skip to content

Remove useless allocations; use (*Type)(nil) instead #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 19, 2025
Merged

Remove useless allocations; use (*Type)(nil) instead #545

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 38 additions & 38 deletions mcp/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,54 +8,54 @@ import (
)

// ClientRequest types
var _ ClientRequest = &PingRequest{}
var _ ClientRequest = &InitializeRequest{}
var _ ClientRequest = &CompleteRequest{}
var _ ClientRequest = &SetLevelRequest{}
var _ ClientRequest = &GetPromptRequest{}
var _ ClientRequest = &ListPromptsRequest{}
var _ ClientRequest = &ListResourcesRequest{}
var _ ClientRequest = &ReadResourceRequest{}
var _ ClientRequest = &SubscribeRequest{}
var _ ClientRequest = &UnsubscribeRequest{}
var _ ClientRequest = &CallToolRequest{}
var _ ClientRequest = &ListToolsRequest{}
var _ ClientRequest = (*PingRequest)(nil)
var _ ClientRequest = (*InitializeRequest)(nil)
var _ ClientRequest = (*CompleteRequest)(nil)
var _ ClientRequest = (*SetLevelRequest)(nil)
var _ ClientRequest = (*GetPromptRequest)(nil)
var _ ClientRequest = (*ListPromptsRequest)(nil)
var _ ClientRequest = (*ListResourcesRequest)(nil)
var _ ClientRequest = (*ReadResourceRequest)(nil)
var _ ClientRequest = (*SubscribeRequest)(nil)
var _ ClientRequest = (*UnsubscribeRequest)(nil)
var _ ClientRequest = (*CallToolRequest)(nil)
var _ ClientRequest = (*ListToolsRequest)(nil)

// ClientNotification types
var _ ClientNotification = &CancelledNotification{}
var _ ClientNotification = &ProgressNotification{}
var _ ClientNotification = &InitializedNotification{}
var _ ClientNotification = &RootsListChangedNotification{}
var _ ClientNotification = (*CancelledNotification)(nil)
var _ ClientNotification = (*ProgressNotification)(nil)
var _ ClientNotification = (*InitializedNotification)(nil)
var _ ClientNotification = (*RootsListChangedNotification)(nil)

// ClientResult types
var _ ClientResult = &EmptyResult{}
var _ ClientResult = &CreateMessageResult{}
var _ ClientResult = &ListRootsResult{}
var _ ClientResult = (*EmptyResult)(nil)
var _ ClientResult = (*CreateMessageResult)(nil)
var _ ClientResult = (*ListRootsResult)(nil)

// ServerRequest types
var _ ServerRequest = &PingRequest{}
var _ ServerRequest = &CreateMessageRequest{}
var _ ServerRequest = &ListRootsRequest{}
var _ ServerRequest = (*PingRequest)(nil)
var _ ServerRequest = (*CreateMessageRequest)(nil)
var _ ServerRequest = (*ListRootsRequest)(nil)

// ServerNotification types
var _ ServerNotification = &CancelledNotification{}
var _ ServerNotification = &ProgressNotification{}
var _ ServerNotification = &LoggingMessageNotification{}
var _ ServerNotification = &ResourceUpdatedNotification{}
var _ ServerNotification = &ResourceListChangedNotification{}
var _ ServerNotification = &ToolListChangedNotification{}
var _ ServerNotification = &PromptListChangedNotification{}
var _ ServerNotification = (*CancelledNotification)(nil)
var _ ServerNotification = (*ProgressNotification)(nil)
var _ ServerNotification = (*LoggingMessageNotification)(nil)
var _ ServerNotification = (*ResourceUpdatedNotification)(nil)
var _ ServerNotification = (*ResourceListChangedNotification)(nil)
var _ ServerNotification = (*ToolListChangedNotification)(nil)
var _ ServerNotification = (*PromptListChangedNotification)(nil)

// ServerResult types
var _ ServerResult = &EmptyResult{}
var _ ServerResult = &InitializeResult{}
var _ ServerResult = &CompleteResult{}
var _ ServerResult = &GetPromptResult{}
var _ ServerResult = &ListPromptsResult{}
var _ ServerResult = &ListResourcesResult{}
var _ ServerResult = &ReadResourceResult{}
var _ ServerResult = &CallToolResult{}
var _ ServerResult = &ListToolsResult{}
var _ ServerResult = (*EmptyResult)(nil)
var _ ServerResult = (*InitializeResult)(nil)
var _ ServerResult = (*CompleteResult)(nil)
var _ ServerResult = (*GetPromptResult)(nil)
var _ ServerResult = (*ListPromptsResult)(nil)
var _ ServerResult = (*ListResourcesResult)(nil)
var _ ServerResult = (*ReadResourceResult)(nil)
var _ ServerResult = (*CallToolResult)(nil)
var _ ServerResult = (*ListToolsResult)(nil)
Comment on lines 10 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Heads-up: These lines do not enforce interface conformance because Client/Server are aliases to any**

In mcp/types.go, the following are defined as type aliases to any:

  • ClientRequest, ClientNotification, ClientResult
  • ServerRequest, ServerNotification, ServerResult

Given that, declarations like var _ ClientRequest = (*PingRequest)(nil) always type-check and do not provide compile-time interface enforcement. If these lines are meant purely as documentation/grouping, this refactor is fine. If the intent is to enforce conformance, consider replacing the any aliases with marker interfaces and implementing them on the relevant types.

Example approach (separate follow-up PR):

  • Define marker interfaces:
    • type ClientRequest interface{ isClientRequest() }
    • type ServerRequest interface{ isServerRequest() }
    • … similarly for Notification and Result
  • Add empty method receivers on each concrete type:
    • func (*PingRequest) isClientRequest() {}
    • etc.

Alternatively, if the aim is just to avoid allocations while retaining the “catalog,” consider adding a brief comment explaining that these are non-enforcing catalogs of types.

You can verify the current aliases with this script:

🏁 Script executed:

#!/bin/bash
# Verify that Client*/Server* types are aliases to `any`
set -euo pipefail

echo "Searching for any-alias declarations for Client*/Server*…"
rg -n -A 1 -B 1 $'type\\s+(ClientRequest|ClientNotification|ClientResult|ServerRequest|ServerNotification|ServerResult)\\s+any'

echo
echo "If the above matches show aliases to 'any', these assertions do not enforce interface conformance."

Length of output: 1464

Enforce compile-time checks for Client/Server types
The aliases in mcp/types.go (lines 1110–1125)

type ClientRequest any
type ClientNotification any
…  
type ServerResult any

mean that the assertions in mcp/utils.go (lines 10–58)

var _ ClientRequest = (*PingRequest)(nil)
…
var _ ServerResult  = (*ListToolsResult)(nil)

are purely documentation and do not provide compile-time interface enforcement.

To fix this, either:

• Replace each any alias with a marker interface in mcp/types.go:

- type ClientRequest any
+ type ClientRequest interface { isClientRequest() }

(and likewise for ClientNotification, ClientResult, ServerRequest, etc.)

• Implement the empty methods on each concrete type—e.g. in mcp/utils.go:

func (*PingRequest) isClientRequest() {}
func (*InitializeRequest) isClientRequest() {}
// …and so on for every ClientRequest type

Or, if you only intended a non-enforcing catalog of types, please add a clarifying comment above the assertion block in mcp/utils.go.

🤖 Prompt for AI Agents 
In mcp/utils.go around lines 10–58 the var _ Type = (*Concrete)(nil) assertions
are no-ops because the corresponding aliases in mcp/types.go (lines ~1110–1125)
use `type ClientRequest any` (and similar), so they do not enforce compile-time
checks; choose one of three fixes: (1) change each alias in mcp/types.go into a
marker interface (e.g. type ClientRequest interface { isClientRequest() }) and
keep the current assertions; or (2) implement the empty marker methods on every
concrete type listed in mcp/utils.go (e.g. add isClientRequest methods on
PingRequest, InitializeRequest, etc., and analogous methods for
ClientNotification/ClientResult/ServerRequest/ServerNotification/ServerResult)
so the assertions become real; or (3) if enforcement is not desired, add a clear
comment above the assertion block stating these are only documentation and will
not be enforced, so reviewers know it is intentional.

Copy link
Contributor

@ezynda3 ezynda3 Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikeschinkel could you address this?

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


// Helper functions for type assertions

Expand Down