-
Notifications
You must be signed in to change notification settings - Fork 586
Add CS1066 suppressor for MCP server methods with optional parameters #1110
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.
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
Conversation
Add a DiagnosticSuppressor that suppresses CS1066 warnings for methods marked with [McpServerTool], [McpServerPrompt], or [McpServerResource] attributes. This allows users to specify default parameter values in their implementing declarations without compiler warnings. The XmlToDescriptionGenerator already copies these defaults to the generated defining declaration, making them functional for callers and reflection. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
- Add semantic model caching per syntax tree to avoid redundant calls - Remove unnecessary try-catch block in test helper - Fix test assertions to use correct diagnostic checking pattern Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
stephentoub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…symbol - Extract shared McpAttributeNames class with constants used by both XmlToDescriptionGenerator and CS1066Suppressor - Update CS1066Suppressor to match attributes using SymbolEqualityComparer instead of string comparison - Cache attribute symbols per compilation for efficiency Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Update XmlToDescriptionGeneratorTests.RunGenerator to: - Run the CS1066Suppressor alongside the generator - Check for unsuppressed CS1066 warnings in addition to generator errors - Fail tests if any CS1066 warnings are not properly suppressed This ensures the generator tests will fail if the suppressor doesn't properly handle CS1066 warnings for partial methods with default parameter values. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
- Run CS1066Suppressor alongside the generator in tests - Check for unsuppressed CS1066 warnings and fail tests if any are found - Generator errors cause test failures - Generator warnings are allowed (some tests like MCP001 invalid XML expect warnings) Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
|
@copilot, please update all of the source generator tests to validate that no unexpected diagnostics of any verbosity level are emitted. |
- Changed RunGenerator to accept expected diagnostic IDs as parameters - Tests now fail if any unexpected diagnostics (of any verbosity) are emitted - Updated 8 tests that expect specific diagnostics (MCP001, MCP002) to pass those diagnostic IDs as expected - All 48 analyzer tests pass Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Done in f811455. Updated |
Summary
This PR adds a
DiagnosticSuppressorthat suppresses CS1066 compiler warnings for MCP server methods (tools, prompts, resources) that have optional parameters with default values.Problem
When users define MCP tool methods as partial methods with optional parameters:
The XmlToDescriptionGenerator creates a defining declaration (no body) with the same defaults. The compiler then warns with CS1066 that the defaults on the user's implementing declaration are redundant. This creates noisy CI logs with many warnings.
Solution
Added a
DiagnosticSuppressor(CS1066Suppressor.cs) that:[McpServerTool],[McpServerPrompt], or[McpServerResource]attributes using symbol comparisonAlso added
McpAttributeNames.csto share attribute name constants between the generator and suppressor.Tests
Updated the existing
XmlToDescriptionGeneratorTests.RunGeneratormethod to:Updated 8 tests that expect specific diagnostics (MCP001, MCP002) to pass those diagnostic IDs as expected.
Added 5 new tests in
CS1066SuppressorTests.cs:Suppressor_WithMcpServerToolAttribute_SuppressesCS1066Suppressor_WithMcpServerPromptAttribute_SuppressesCS1066Suppressor_WithMcpServerResourceAttribute_SuppressesCS1066Suppressor_WithoutMcpAttribute_DoesNotSuppressCS1066Suppressor_WithMultipleParameters_SuppressesAllCS1066All 48 analyzer tests pass.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.