Address some static analysis and TODOs (#12)

This commit is contained in:
Stephen Toub 2025-03-20 23:09:38 -04:00 • committed by GitHub
commit 33b9582199
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 132 additions and 123 deletions

View file

@ -1,3 +1,5 @@
using ModelContextProtocol.Utils;
namespace System.Collections.Generic; namespace System.Collections.Generic;
internal static class CollectionExtensions internal static class CollectionExtensions
@ -9,10 +11,7 @@ internal static class CollectionExtensions
public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue) public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue)
{ {
if (dictionary is null) Throw.IfNull(dictionary);
{
throw new ArgumentNullException(nameof(dictionary));
}
return dictionary.TryGetValue(key, out TValue? value) ? value : defaultValue; return dictionary.TryGetValue(key, out TValue? value) ? value : defaultValue;
} }

View file

@ -0,0 +1,36 @@
using ModelContextProtocol.Utils;
using System.Buffers;
using System.Runtime.InteropServices;
namespace System.IO;
internal static class StreamExtensions
{
public static ValueTask WriteAsync(this Stream stream, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken)
{
Throw.IfNull(stream);
if (MemoryMarshal.TryGetArray(buffer, out ArraySegment<byte> segment))
{
return new ValueTask(stream.WriteAsync(segment.Array, segment.Offset, segment.Count, cancellationToken));
}
else
{
return WriteAsyncCore(stream, buffer, cancellationToken);
static async ValueTask WriteAsyncCore(Stream stream, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken)
{
byte[] array = ArrayPool<byte>.Shared.Rent(buffer.Length);
try
{
buffer.Span.CopyTo(array);
await stream.WriteAsync(array, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
}
finally
{
ArrayPool<byte>.Shared.Return(array);
}
}
}
}
}

View file

@ -4,7 +4,21 @@ internal static class TextReaderExtensions
{ {
public static Task<string> ReadLineAsync(this TextReader reader, CancellationToken cancellationToken) public static Task<string> ReadLineAsync(this TextReader reader, CancellationToken cancellationToken)
{ {
_ = cancellationToken; if (cancellationToken.IsCancellationRequested)
{
return Task.FromCanceled<string>(cancellationToken);
}
return reader.ReadLineAsync(); return reader.ReadLineAsync();
} }
public static Task<string> ReadToEndAsync(this TextReader reader, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return Task.FromCanceled<string>(cancellationToken);
}
return reader.ReadToEndAsync();
}
} }

View file

@ -1,3 +1,4 @@
using ModelContextProtocol.Utils;
using System.Runtime.InteropServices; using System.Runtime.InteropServices;
namespace System.IO; namespace System.IO;
@ -6,10 +7,7 @@ internal static class TextWriterExtensions
{ {
public static async Task WriteLineAsync(this TextWriter writer, ReadOnlyMemory<char> value, CancellationToken cancellationToken) public static async Task WriteLineAsync(this TextWriter writer, ReadOnlyMemory<char> value, CancellationToken cancellationToken)
{ {
if (writer is null) Throw.IfNull(writer);
{
throw new ArgumentNullException(nameof(writer));
}
if (value.IsEmpty) if (value.IsEmpty)
{ {

View file

@ -1,13 +1,12 @@
using ModelContextProtocol.Utils;
namespace System.Net.Http; namespace System.Net.Http;
internal static class HttpClientExtensions internal static class HttpClientExtensions
{ {
public static async Task<Stream> ReadAsStreamAsync(this HttpContent content, CancellationToken cancellationToken) public static async Task<Stream> ReadAsStreamAsync(this HttpContent content, CancellationToken cancellationToken)
{ {
if (content is null) Throw.IfNull(content);
{
throw new ArgumentNullException(nameof(content));
}
cancellationToken.ThrowIfCancellationRequested(); cancellationToken.ThrowIfCancellationRequested();
return await content.ReadAsStreamAsync(); return await content.ReadAsStreamAsync();
@ -15,10 +14,7 @@ internal static class HttpClientExtensions
public static async Task<string> ReadAsStringAsync(this HttpContent content, CancellationToken cancellationToken) public static async Task<string> ReadAsStringAsync(this HttpContent content, CancellationToken cancellationToken)
{ {
if (content is null) Throw.IfNull(content);
{
throw new ArgumentNullException(nameof(content));
}
cancellationToken.ThrowIfCancellationRequested(); cancellationToken.ThrowIfCancellationRequested();
return await content.ReadAsStringAsync(); return await content.ReadAsStringAsync();

View file

@ -1,13 +1,12 @@
using ModelContextProtocol.Utils;
namespace System.Threading.Tasks; namespace System.Threading.Tasks;
internal static class CancellationTokenSourceExtensions internal static class CancellationTokenSourceExtensions
{ {
public static Task CancelAsync(this CancellationTokenSource cancellationTokenSource) public static Task CancelAsync(this CancellationTokenSource cancellationTokenSource)
{ {
if (cancellationTokenSource is null) Throw.IfNull(cancellationTokenSource);
{
throw new ArgumentNullException(nameof(cancellationTokenSource));
}
cancellationTokenSource.Cancel(); cancellationTokenSource.Cancel();
return Task.CompletedTask; return Task.CompletedTask;

View file

@ -0,0 +1,17 @@
using System.Runtime.CompilerServices;
namespace System.Threading;
/// <summary>
/// await default(ForceYielding) to provide the same behavior as
/// await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding).
/// </summary>
internal readonly struct ForceYielding : INotifyCompletion, ICriticalNotifyCompletion
{
public ForceYielding GetAwaiter() => this;
public bool IsCompleted => false;
public void OnCompleted(Action continuation) => ThreadPool.QueueUserWorkItem(a => ((Action)a!)(), continuation);
public void UnsafeOnCompleted(Action continuation) => ThreadPool.UnsafeQueueUserWorkItem(a => ((Action)a!)(), continuation);
public void GetResult() { }
}

View file

@ -1,3 +1,5 @@
using ModelContextProtocol.Utils;
namespace System.Threading.Tasks; namespace System.Threading.Tasks;
internal static class TaskExtensions internal static class TaskExtensions
@ -15,10 +17,7 @@ internal static class TaskExtensions
public static async Task WaitAsync(this Task task, TimeSpan timeout, CancellationToken cancellationToken) public static async Task WaitAsync(this Task task, TimeSpan timeout, CancellationToken cancellationToken)
{ {
if (task is null) Throw.IfNull(task);
{
throw new ArgumentNullException(nameof(task));
}
if (timeout < TimeSpan.Zero && timeout != Timeout.InfiniteTimeSpan) if (timeout < TimeSpan.Zero && timeout != Timeout.InfiniteTimeSpan)
{ {

View file

@ -6,8 +6,6 @@ using Microsoft.Extensions.AI;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Text.Json; using System.Text.Json;
#pragma warning disable CA1508 // Avoid dead conditional code
namespace ModelContextProtocol.Client; namespace ModelContextProtocol.Client;
/// <summary> /// <summary>
@ -439,7 +437,7 @@ public static class McpClientExtensions
} }
private static JsonRpcRequest CreateRequest(string method, Dictionary<string, object?>? parameters) => private static JsonRpcRequest CreateRequest(string method, Dictionary<string, object?>? parameters) =>
new JsonRpcRequest new()
{ {
Method = method, Method = method,
Params = parameters Params = parameters

View file

@ -30,10 +30,7 @@ public static partial class McpServerBuilderExtensions
/// <param name="builder">The builder instance.</param> /// <param name="builder">The builder instance.</param>
public static IMcpServerBuilder WithHttpListenerSseServerTransport(this IMcpServerBuilder builder) public static IMcpServerBuilder WithHttpListenerSseServerTransport(this IMcpServerBuilder builder)
{ {
if (builder is null) Throw.IfNull(builder);
{
throw new ArgumentNullException(nameof(builder));
}
builder.Services.AddSingleton<IServerTransport, HttpListenerSseServerTransport>(); builder.Services.AddSingleton<IServerTransport, HttpListenerSseServerTransport>();
builder.Services.AddHostedService<McpServerHostedService>(); builder.Services.AddHostedService<McpServerHostedService>();

View file

@ -2,6 +2,7 @@
using ModelContextProtocol.Protocol.Types; using ModelContextProtocol.Protocol.Types;
using ModelContextProtocol.Server; using ModelContextProtocol.Server;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
using ModelContextProtocol.Utils;
namespace ModelContextProtocol.Configuration; namespace ModelContextProtocol.Configuration;
@ -9,10 +10,7 @@ internal sealed class McpServerOptionsSetup(IOptions<McpServerHandlers> serverHa
{ {
public void Configure(McpServerOptions options) public void Configure(McpServerOptions options)
{ {
if (options is null) Throw.IfNull(options);
{
throw new ArgumentNullException(nameof(options));
}
var assemblyName = Assembly.GetEntryAssembly()?.GetName(); var assemblyName = Assembly.GetEntryAssembly()?.GetName();
options.ServerInfo = new Implementation options.ServerInfo = new Implementation

View file

@ -11,9 +11,12 @@ namespace ModelContextProtocol.Protocol.Transport;
[ExcludeFromCodeCoverage] [ExcludeFromCodeCoverage]
internal class HttpListenerServerProvider : IDisposable internal class HttpListenerServerProvider : IDisposable
{ {
private static readonly byte[] s_accepted = "Accepted"u8.ToArray();
private const string SseEndpoint = "/sse";
private const string MessageEndpoint = "/message";
private readonly int _port; private readonly int _port;
private readonly string _sseEndpoint = "/sse";
private readonly string _messageEndpoint = "/message";
private HttpListener? _listener; private HttpListener? _listener;
private CancellationTokenSource? _cts; private CancellationTokenSource? _cts;
private Func<string, CancellationToken, bool>? _messageHandler; private Func<string, CancellationToken, bool>? _messageHandler;
@ -31,7 +34,7 @@ internal class HttpListenerServerProvider : IDisposable
public Task<string> GetSseEndpointUri() public Task<string> GetSseEndpointUri()
{ {
return Task.FromResult($"http://localhost:{_port}{_sseEndpoint}"); return Task.FromResult($"http://localhost:{_port}{SseEndpoint}");
} }
public Task InitializeMessageHandler(Func<string, CancellationToken, bool> messageHandler) public Task InitializeMessageHandler(Func<string, CancellationToken, bool> messageHandler)
@ -131,12 +134,12 @@ internal class HttpListenerServerProvider : IDisposable
throw new McpServerException("Request is null"); throw new McpServerException("Request is null");
// Handle SSE connection // Handle SSE connection
if (request.HttpMethod == "GET" && request.Url?.LocalPath == _sseEndpoint) if (request.HttpMethod == "GET" && request.Url?.LocalPath == SseEndpoint)
{ {
await HandleSseConnectionAsync(context, cancellationToken).ConfigureAwait(false); await HandleSseConnectionAsync(context, cancellationToken).ConfigureAwait(false);
} }
// Handle message POST // Handle message POST
else if (request.HttpMethod == "POST" && request.Url?.LocalPath == _messageEndpoint) else if (request.HttpMethod == "POST" && request.Url?.LocalPath == MessageEndpoint)
{ {
await HandleMessageAsync(context, cancellationToken).ConfigureAwait(false); await HandleMessageAsync(context, cancellationToken).ConfigureAwait(false);
} }
@ -167,9 +170,6 @@ internal class HttpListenerServerProvider : IDisposable
response.Headers.Add("Cache-Control", "no-cache"); response.Headers.Add("Cache-Control", "no-cache");
response.Headers.Add("Connection", "keep-alive"); response.Headers.Add("Connection", "keep-alive");
// Create a unique ID for this client
var clientId = Guid.NewGuid().ToString();
// Get the output stream and create a StreamWriter // Get the output stream and create a StreamWriter
var outputStream = response.OutputStream; var outputStream = response.OutputStream;
_streamWriter = new StreamWriter(outputStream, Encoding.UTF8) { AutoFlush = true }; _streamWriter = new StreamWriter(outputStream, Encoding.UTF8) { AutoFlush = true };
@ -179,7 +179,7 @@ internal class HttpListenerServerProvider : IDisposable
{ {
// Immediately send the "endpoint" event with the POST URL // Immediately send the "endpoint" event with the POST URL
await _streamWriter.WriteLineAsync("event: endpoint").ConfigureAwait(false); await _streamWriter.WriteLineAsync("event: endpoint").ConfigureAwait(false);
await _streamWriter.WriteLineAsync($"data: {_messageEndpoint}").ConfigureAwait(false); await _streamWriter.WriteLineAsync($"data: {MessageEndpoint}").ConfigureAwait(false);
await _streamWriter.WriteLineAsync().ConfigureAwait(false); // blank line to end an SSE message await _streamWriter.WriteLineAsync().ConfigureAwait(false); // blank line to end an SSE message
await _streamWriter.FlushAsync(cancellationToken).ConfigureAwait(false); await _streamWriter.FlushAsync(cancellationToken).ConfigureAwait(false);
@ -222,10 +222,7 @@ internal class HttpListenerServerProvider : IDisposable
string requestBody; string requestBody;
using (var reader = new StreamReader(request.InputStream, request.ContentEncoding)) using (var reader = new StreamReader(request.InputStream, request.ContentEncoding))
{ {
// TODO: Add cancellation token and netstandard2.0 support (polyfill?) requestBody = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false);
#pragma warning disable CA2016 // Forward the 'CancellationToken' parameter to methods
requestBody = await reader.ReadToEndAsync().ConfigureAwait(false);
#pragma warning restore CA2016 // Forward the 'CancellationToken' parameter to methods
} }
// Process the message asynchronously // Process the message asynchronously
@ -233,12 +230,9 @@ internal class HttpListenerServerProvider : IDisposable
{ {
// Return 202 Accepted // Return 202 Accepted
response.StatusCode = 202; response.StatusCode = 202;
// Write "accepted" response // Write "accepted" response
// TODO: Use WriteAsync, add cancellation token and netstandard2.0 support (polyfill?) await response.OutputStream.WriteAsync(s_accepted, cancellationToken).ConfigureAwait(false);
byte[] buffer = Encoding.UTF8.GetBytes("Accepted");
#pragma warning disable CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'
await response.OutputStream.WriteAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
#pragma warning restore CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'
} }
else else
{ {

View file

@ -4,6 +4,7 @@ using ModelContextProtocol.Protocol.Messages;
using ModelContextProtocol.Utils.Json; using ModelContextProtocol.Utils.Json;
using ModelContextProtocol.Logging; using ModelContextProtocol.Logging;
using ModelContextProtocol.Server; using ModelContextProtocol.Server;
using ModelContextProtocol.Utils;
namespace ModelContextProtocol.Protocol.Transport; namespace ModelContextProtocol.Protocol.Transport;
@ -160,15 +161,8 @@ public sealed class HttpListenerSseServerTransport : TransportBase, IServerTrans
/// <summary>Validates the <paramref name="serverOptions"/> and extracts from it the server name to use.</summary> /// <summary>Validates the <paramref name="serverOptions"/> and extracts from it the server name to use.</summary>
private static string GetServerName(McpServerOptions serverOptions) private static string GetServerName(McpServerOptions serverOptions)
{ {
if (serverOptions is null) Throw.IfNull(serverOptions);
{ Throw.IfNull(serverOptions.ServerInfo);
throw new ArgumentNullException(nameof(serverOptions));
}
if (serverOptions.ServerInfo is null)
{
throw new ArgumentNullException($"{nameof(serverOptions)}.{nameof(serverOptions.ServerInfo)}");
}
return serverOptions.ServerInfo.Name; return serverOptions.ServerInfo.Name;
} }

View file

@ -8,8 +8,6 @@ using ModelContextProtocol.Utils.Json;
using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Abstractions;
#pragma warning disable CA2213 // Disposable fields should be disposed
namespace ModelContextProtocol.Protocol.Transport; namespace ModelContextProtocol.Protocol.Transport;
/// <summary> /// <summary>

View file

@ -8,9 +8,6 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
#pragma warning disable CA2213 // Disposable fields should be disposed
namespace ModelContextProtocol.Protocol.Transport; namespace ModelContextProtocol.Protocol.Transport;
/// <summary> /// <summary>

View file

@ -87,7 +87,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
#if NET #if NET
await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding); await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding);
#else #else
await Task.Yield(); // TODO: Fix this await default(ForceYielding);
#endif #endif
try try
{ {

View file

@ -21,7 +21,7 @@ internal static class Throw
{ {
if (arg is null || arg.AsSpan().IsWhiteSpace()) if (arg is null || arg.AsSpan().IsWhiteSpace())
{ {
ThrowArgumentNullOrWhiteSpaceException(arg); ThrowArgumentNullOrWhiteSpaceException(parameterName);
} }
} }

View file

@ -45,7 +45,7 @@ internal static class Program
GetCompletionHandler = ConfigureCompletion(), GetCompletionHandler = ConfigureCompletion(),
}; };
var loggerFactory = CreateLoggerFactory(); using var loggerFactory = CreateLoggerFactory();
await using IMcpServer server = McpServerFactory.Create(new StdioServerTransport("TestServer", loggerFactory), options, loggerFactory); await using IMcpServer server = McpServerFactory.Create(new StdioServerTransport("TestServer", loggerFactory), options, loggerFactory);
Log.Logger.Information("Server initialized."); Log.Logger.Information("Server initialized.");
@ -266,7 +266,7 @@ internal static class Program
}; };
} }
private static HashSet<string> _subscribedResources = new(); private static readonly HashSet<string> _subscribedResources = new();
private static readonly object _subscribedResourcesLock = new(); private static readonly object _subscribedResourcesLock = new();
private static ResourcesCapability ConfigureResources() private static ResourcesCapability ConfigureResources()

View file

@ -29,7 +29,7 @@ internal class Program
{ {
Console.WriteLine("Starting server..."); Console.WriteLine("Starting server...");
McpServerOptions options = new McpServerOptions() McpServerOptions options = new()
{ {
ServerInfo = new Implementation() { Name = "TestServer", Version = "1.0.0" }, ServerInfo = new Implementation() { Name = "TestServer", Version = "1.0.0" },
Capabilities = new ServerCapabilities() Capabilities = new ServerCapabilities()
@ -236,11 +236,10 @@ internal class Program
{ {
throw new McpServerException("Missing required argument 'uri'"); throw new McpServerException("Missing required argument 'uri'");
} }
ResourceContents? contents = resourceContents.FirstOrDefault(r => r.Uri == request.Params.Uri);
if (contents is null) ResourceContents? contents = resourceContents.FirstOrDefault(r => r.Uri == request.Params.Uri) ??
{
throw new McpServerException("Resource not found"); throw new McpServerException("Resource not found");
}
return Task.FromResult(new ReadResourceResult() return Task.FromResult(new ReadResourceResult()
{ {
Contents = [contents] Contents = [contents]
@ -347,7 +346,7 @@ internal class Program
}, },
}; };
var loggerFactory = CreateLoggerFactory(); using var loggerFactory = CreateLoggerFactory();
server = McpServerFactory.Create(new HttpListenerSseServerTransport("TestServer", 3001, loggerFactory), options, loggerFactory); server = McpServerFactory.Create(new HttpListenerSseServerTransport("TestServer", 3001, loggerFactory), options, loggerFactory);
Console.WriteLine("Server initialized."); Console.WriteLine("Server initialized.");
@ -357,10 +356,7 @@ internal class Program
Console.WriteLine("Server started."); Console.WriteLine("Server started.");
// Run until process is stopped by the client (parent process) // Run until process is stopped by the client (parent process)
while (true) await Task.Delay(Timeout.Infinite);
{
await Task.Delay(1000);
}
} }
const string MCP_TINY_IMAGE = const string MCP_TINY_IMAGE =

View file

@ -220,18 +220,17 @@ public class McpServerBuilderExtensionsToolsTests
Assert.Contains("'NotRegisteredTool'", exception.Message); Assert.Contains("'NotRegisteredTool'", exception.Message);
} }
// TODO: https://github.com/dotnet/extensions/issues/6124 [Fact(Skip = "https://github.com/dotnet/extensions/issues/6124")]
//[Fact] public async Task Throws_Exception_Missing_Parameter()
//public async Task Throws_Exception_Missing_Parameter() {
//{ _builder.Object.WithTools(typeof(EchoTool));
// _builder.Object.WithTools(typeof(EchoTool));
// var serviceProvider = _services.BuildServiceProvider(); var serviceProvider = _services.BuildServiceProvider();
// var options = serviceProvider.GetRequiredService<IOptions<McpServerHandlers>>().Value; var options = serviceProvider.GetRequiredService<IOptions<McpServerHandlers>>().Value;
// var exception = await Assert.ThrowsAsync<McpServerException>(async () => await options.CallToolHandler!(new(Mock.Of<IMcpServer>(), new() { Name = "Echo" }), CancellationToken.None)); var exception = await Assert.ThrowsAsync<McpServerException>(async () => await options.CallToolHandler!(new(Mock.Of<IMcpServer>(), new() { Name = "Echo" }), CancellationToken.None));
// Assert.Equal("Missing required argument 'message'.", exception.Message); Assert.Equal("Missing required argument 'message'.", exception.Message);
//} }
[Fact] [Fact]
public void Throws_Exception_For_Null_Types() public void Throws_Exception_For_Null_Types()
@ -357,26 +356,6 @@ public class McpServerBuilderExtensionsToolsTests
} }
} }
public interface IDependendService
{
}
private class EchoToolWithDi
{
public EchoToolWithDi(IDependendService service)
{
}
[McpTool, Description("Echoes the input back to the client.")]
#pragma warning disable CA1822 // Mark members as static
public Task<string> Echo(string message)
#pragma warning restore CA1822 // Mark members as static
{
return Task.FromResult("hello " + message);
}
}
public class ComplexObject public class ComplexObject
{ {
public string? Name { get; set; } public string? Name { get; set; }

Some files were not shown because too many files have changed in this diff Show more