Skip to content

Address some static analysis and TODOs #12

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

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

stephentoub
Copy link
Contributor

No description provided.

@stephentoub stephentoub requested a review from Copilot March 21, 2025 02:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses several static analysis warnings and TODOs while refactoring null‐checks and async patterns across the codebase. Key changes include:

  • Replacing explicit null checks with Throw.IfNull calls and updating cancellation support in async methods.
  • Refactoring endpoint constants and async looping in SSE and server transport implementations.
  • Updating test code to use C# shorthand and proper disposal patterns.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Common/Polyfills/System/IO/StreamExtensions.cs Implements optimized async WriteAsync for Stream using array pooling.
src/Common/Polyfills/System/Threading/ForceYielding.cs Adds a custom awaitable struct for forced yielding on NET platforms.
src/ModelContextProtocol/Shared/McpJsonRpcEndpoint.cs Switches from Task.Yield() to ForceYielding for improved async behavior.
src/Common/Polyfills/System/IO/TextReaderExtensions.cs Adds cancellation support for asynchronous reading.
tests/ModelContextProtocol.TestSseServer/Program.cs Uses concise C# object initialization and ensures proper disposal of loggerFactory and infinite delay handling.
src/Common/Polyfills/System/Net/Http/HttpClientExtensions.cs Refactors null-checks to use the Throw.IfNull method.
tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs Enables a previously skipped test using the appropriate Fact attribute.
src/ModelContextProtocol/Protocol/Transport/HttpListenerSseServerTransport.cs Refactors SSE and message endpoint handling using constants.
src/ModelContextProtocol/Client/McpClientExtensions.cs Updates null-checks and utilizes shorthand object instantiation; introduces collection initializers.
[Other Files] Various files have been updated consistently to replace manual null-checks with Throw.IfNull and remove deprecated pragma directives.
Comments suppressed due to low confidence (2)

src/ModelContextProtocol/Client/McpClientExtensions.cs:35

  • The empty list initialization using [] may not be supported by all C# versions. Consider using 'new List()' to ensure compatibility.
List<ChatMessage> messages = [];

tests/ModelContextProtocol.TestSseServer/Program.cs:240

  • The collection initializer using square brackets may lead to compilation issues if not supported. Replace it with 'Contents = new List { contents }' or the appropriate list initialization syntax.
Contents = [contents]

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

@stephentoub stephentoub merged commit 33b9582 into modelcontextprotocol:main Mar 21, 2025
5 of 6 checks passed
@stephentoub stephentoub deleted the staticcleanup branch March 21, 2025 03:09
halter73 referenced this pull request in halter73/csharp-sdk Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant