diff --git a/src/ModelContextProtocol.Analyzers/Diagnostics.cs b/src/ModelContextProtocol.Analyzers/Diagnostics.cs index d8d8ead18..189e98cfb 100644 --- a/src/ModelContextProtocol.Analyzers/Diagnostics.cs +++ b/src/ModelContextProtocol.Analyzers/Diagnostics.cs @@ -36,4 +36,15 @@ internal static class Diagnostics defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true, description: "Methods with MCP attributes should be declared as partial to allow the source generator to emit Description attributes from XML documentation comments."); + + public static DiagnosticDescriptor WithHttpTransportStatelessNotSet { get; } = new( + id: "MCP003", + title: "WithHttpTransport should explicitly configure Stateless property", + messageFormat: "WithHttpTransport is called without explicitly setting HttpServerTransportOptions.Stateless. Set Stateless to true (recommended) or false for forward compatibility.", + category: "mcp", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "WithHttpTransport should explicitly set the Stateless property on HttpServerTransportOptions for forward compatibility. " + + "Stateless mode (recommended for most servers) avoids session state, memory overhead, and deployment constraints. " + + "If your server requires server-to-client requests or unsolicited notifications, set Stateless to false explicitly."); } diff --git a/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs b/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs index f615d07d8..0559fa7f3 100644 --- a/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs +++ b/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs @@ -9,4 +9,6 @@ internal static class McpAttributeNames public const string McpServerPromptAttribute = "ModelContextProtocol.Server.McpServerPromptAttribute"; public const string McpServerResourceAttribute = "ModelContextProtocol.Server.McpServerResourceAttribute"; public const string DescriptionAttribute = "System.ComponentModel.DescriptionAttribute"; + public const string HttpMcpServerBuilderExtensions = "Microsoft.Extensions.DependencyInjection.HttpMcpServerBuilderExtensions"; + public const string HttpServerTransportOptions = "ModelContextProtocol.AspNetCore.HttpServerTransportOptions"; } diff --git a/src/ModelContextProtocol.Analyzers/WithHttpTransportAnalyzer.cs b/src/ModelContextProtocol.Analyzers/WithHttpTransportAnalyzer.cs new file mode 100644 index 000000000..20005c901 --- /dev/null +++ b/src/ModelContextProtocol.Analyzers/WithHttpTransportAnalyzer.cs @@ -0,0 +1,290 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using System.Threading; + +namespace ModelContextProtocol.Analyzers; + +/// +/// Reports a warning when WithHttpTransport is called without explicitly setting +/// HttpServerTransportOptions.Stateless to or . +/// +/// +/// +/// The default value of Stateless can change with protocol revisions. Setting the property explicitly +/// protects against future changes to the default, while stateless mode remains recommended for most servers. +/// +/// +/// The analyzer detects the following patterns: +/// +/// .WithHttpTransport() — no delegate passed. +/// .WithHttpTransport(null) — null literal passed. +/// .WithHttpTransport(o => ...) — lambda that does not assign to o.Stateless. +/// .WithHttpTransport(ConfigureMethod) — method group (cannot trace into the body). +/// +/// +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class WithHttpTransportAnalyzer : DiagnosticAnalyzer +{ + private const string WithHttpTransportMethodName = "WithHttpTransport"; + private const string StatelessPropertyName = "Stateless"; + + /// + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(Diagnostics.WithHttpTransportStatelessNotSet); + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(compilationContext => + { + INamedTypeSymbol? extensionsType = compilationContext.Compilation.GetTypeByMetadataName( + McpAttributeNames.HttpMcpServerBuilderExtensions); + + if (extensionsType is null) + { + // The AspNetCore package isn't referenced; nothing to analyze. + return; + } + + INamedTypeSymbol? optionsType = compilationContext.Compilation.GetTypeByMetadataName( + McpAttributeNames.HttpServerTransportOptions); + + if (optionsType is null) + { + return; + } + + compilationContext.RegisterSyntaxNodeAction( + nodeContext => AnalyzeInvocation(nodeContext, extensionsType, optionsType), + SyntaxKind.InvocationExpression); + }); + } + + private static void AnalyzeInvocation( + SyntaxNodeAnalysisContext context, + INamedTypeSymbol extensionsType, + INamedTypeSymbol optionsType) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + // Quick syntactic check: does the method name end with "WithHttpTransport"? + string? methodName = GetMethodName(invocation); + if (methodName != WithHttpTransportMethodName) + { + return; + } + + // Resolve the method symbol to confirm it's the right one. + if (context.SemanticModel.GetSymbolInfo(invocation, context.CancellationToken).Symbol is not IMethodSymbol methodSymbol) + { + return; + } + + // Check that the method belongs to HttpMcpServerBuilderExtensions (comparing the original definition for generic cases). + IMethodSymbol originalMethod = methodSymbol.ReducedFrom ?? methodSymbol; + if (!SymbolEqualityComparer.Default.Equals(originalMethod.ContainingType, extensionsType)) + { + return; + } + + // Determine the configureOptions argument. + bool isExtensionInvocation = methodSymbol.ReducedFrom is not null; + ExpressionSyntax? configureArg = GetConfigureOptionsArgument(invocation, originalMethod, isExtensionInvocation); + + if (configureArg is null || IsNullLiteral(configureArg)) + { + // No delegate or explicit null — warn. + context.ReportDiagnostic(Diagnostic.Create( + Diagnostics.WithHttpTransportStatelessNotSet, + invocation.GetLocation())); + return; + } + + // If the argument is a lambda or anonymous method, check whether it assigns to Stateless. + if (configureArg is AnonymousFunctionExpressionSyntax lambda) + { + if (!LambdaAssignsStateless(lambda, context.SemanticModel, optionsType, context.CancellationToken)) + { + context.ReportDiagnostic(Diagnostic.Create( + Diagnostics.WithHttpTransportStatelessNotSet, + invocation.GetLocation())); + } + return; + } + + // For method groups, delegate variables, or other expressions we cannot analyze easily — warn. + context.ReportDiagnostic(Diagnostic.Create( + Diagnostics.WithHttpTransportStatelessNotSet, + invocation.GetLocation())); + } + + private static string? GetMethodName(InvocationExpressionSyntax invocation) + { + return invocation.Expression switch + { + MemberAccessExpressionSyntax memberAccess => memberAccess.Name.Identifier.Text, + MemberBindingExpressionSyntax memberBinding => memberBinding.Name.Identifier.Text, + IdentifierNameSyntax identifier => identifier.Identifier.Text, + _ => null, + }; + } + + /// + /// Extracts the configureOptions argument from the invocation, handling both positional and named arguments. + /// Returns if no argument was provided (relying on the default null). + /// + private static ExpressionSyntax? GetConfigureOptionsArgument( + InvocationExpressionSyntax invocation, + IMethodSymbol originalMethod, + bool isExtensionInvocation) + { + ArgumentListSyntax argumentList = invocation.ArgumentList; + + // If called as an extension method via member access (builder.WithHttpTransport(...)), + // the configureOptions parameter is at index 1 in the original definition but index 0 in the call. + // If called as a static method (HttpMcpServerBuilderExtensions.WithHttpTransport(builder, ...)), + // it's at index 1 in the call. + // We match by parameter name for robustness. + + foreach (ArgumentSyntax argument in argumentList.Arguments) + { + if (argument.NameColon is not null) + { + if (argument.NameColon.Name.Identifier.Text == "configureOptions") + { + return argument.Expression; + } + } + } + + // No named argument found — try positional. + // Find the parameter index of configureOptions in the original method. + int paramIndex = -1; + for (int i = 0; i < originalMethod.Parameters.Length; i++) + { + if (originalMethod.Parameters[i].Name == "configureOptions") + { + paramIndex = i; + break; + } + } + + if (paramIndex < 0) + { + return null; + } + + // Adjust index: if called as an extension method (builder.WithHttpTransport(...)), + // the 'this' parameter is implicit in the call args. + int callArgIndex = isExtensionInvocation ? paramIndex - 1 : paramIndex; + + if (callArgIndex >= 0 && callArgIndex < argumentList.Arguments.Count) + { + return argumentList.Arguments[callArgIndex].Expression; + } + + return null; + } + + private static bool IsNullLiteral(ExpressionSyntax expression) + { + // Handle both 'null' and 'default' / 'default(Action)'. + return expression.IsKind(SyntaxKind.NullLiteralExpression) || + expression.IsKind(SyntaxKind.DefaultLiteralExpression) || + expression.IsKind(SyntaxKind.DefaultExpression); + } + + /// + /// Checks whether a lambda or anonymous method assigns to the Stateless property + /// on its parameter (which should be of type HttpServerTransportOptions). + /// + private static bool LambdaAssignsStateless( + AnonymousFunctionExpressionSyntax lambda, + SemanticModel semanticModel, + INamedTypeSymbol optionsType, + CancellationToken cancellationToken) + { + // Get the parameter name used by the lambda. For simple lambdas (o => ...), + // ParenthesizedLambda (Action can have (options) => ...), or anonymous delegates. + string? parameterName = GetLambdaParameterName(lambda); + + // Walk all descendant assignment expressions looking for assignments to .Stateless. + foreach (AssignmentExpressionSyntax assignment in lambda.DescendantNodes().OfType()) + { + if (IsStatelessAssignment(assignment, parameterName, semanticModel, optionsType, cancellationToken)) + { + return true; + } + } + + return false; + } + + private static string? GetLambdaParameterName(AnonymousFunctionExpressionSyntax lambda) + { + if (lambda is SimpleLambdaExpressionSyntax simpleLambda) + { + return simpleLambda.Parameter.Identifier.Text; + } + + if (lambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda && + parenthesizedLambda.ParameterList.Parameters.Count > 0) + { + return parenthesizedLambda.ParameterList.Parameters[0].Identifier.Text; + } + + if (lambda is AnonymousMethodExpressionSyntax anonymousMethod && + anonymousMethod.ParameterList is not null && + anonymousMethod.ParameterList.Parameters.Count > 0) + { + return anonymousMethod.ParameterList.Parameters[0].Identifier.Text; + } + + return null; + } + + /// + /// Determines whether an assignment expression targets the Stateless property + /// on HttpServerTransportOptions. Uses a fast syntactic check first, then confirms + /// via the semantic model. + /// + private static bool IsStatelessAssignment( + AssignmentExpressionSyntax assignment, + string? parameterName, + SemanticModel semanticModel, + INamedTypeSymbol optionsType, + CancellationToken cancellationToken) + { + // Fast syntactic check: is the left side .Stateless? + if (assignment.Left is not MemberAccessExpressionSyntax memberAccess || + memberAccess.Name.Identifier.Text != StatelessPropertyName) + { + return false; + } + + // If we know the parameter name, check that the receiver matches. + if (parameterName is not null && + memberAccess.Expression is IdentifierNameSyntax identifier && + identifier.Identifier.Text != parameterName) + { + return false; + } + + // Confirm via semantic model that the property belongs to HttpServerTransportOptions. + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess, cancellationToken); + if (symbolInfo.Symbol is IPropertySymbol propertySymbol && + SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingType, optionsType)) + { + return true; + } + + return false; + } +} diff --git a/tests/ModelContextProtocol.Analyzers.Tests/WithHttpTransportAnalyzerTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/WithHttpTransportAnalyzerTests.cs new file mode 100644 index 000000000..6b6c23d0e --- /dev/null +++ b/tests/ModelContextProtocol.Analyzers.Tests/WithHttpTransportAnalyzerTests.cs @@ -0,0 +1,412 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using Xunit; + +namespace ModelContextProtocol.Analyzers.Tests; + +public class WithHttpTransportAnalyzerTests +{ + // Stub types that mimic the real SDK types so the analyzer can resolve them. + private const string StubTypes = """ + namespace ModelContextProtocol.Server + { + public interface IMcpServerBuilder { } + } + + namespace ModelContextProtocol.AspNetCore + { + public class HttpServerTransportOptions + { + public bool Stateless { get; set; } + public System.TimeSpan IdleTimeout { get; set; } + } + } + + namespace Microsoft.Extensions.DependencyInjection + { + using ModelContextProtocol.Server; + using ModelContextProtocol.AspNetCore; + + public static class HttpMcpServerBuilderExtensions + { + public static IMcpServerBuilder WithHttpTransport( + this IMcpServerBuilder builder, + System.Action? configureOptions = null) + { + return builder; + } + } + } + """; + + [Fact] + public void NoArguments_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void ConditionalAccess_NoArguments_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder? builder) + { + builder?.WithHttpTransport(); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void NullArgument_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(null); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void DefaultLiteral_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + using ModelContextProtocol.AspNetCore; + using System; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(default(Action)); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void SimpleLambda_StatelessTrue_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(o => o.Stateless = true); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void SimpleLambda_StatelessFalse_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(o => o.Stateless = false); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void BlockLambda_StatelessSet_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + using System; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(options => + { + options.Stateless = true; + options.IdleTimeout = TimeSpan.FromMinutes(30); + }); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void BlockLambda_StatelessNotSet_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + using System; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(options => + { + options.IdleTimeout = TimeSpan.FromMinutes(30); + }); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void MethodGroup_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + using ModelContextProtocol.AspNetCore; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(ConfigureTransport); + } + + static void ConfigureTransport(HttpServerTransportOptions options) + { + options.Stateless = true; + } + } + """); + + // Method groups cannot be traced — analyzer warns. + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void DelegateVariable_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + using ModelContextProtocol.AspNetCore; + using System; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + Action configure = o => o.Stateless = true; + builder.WithHttpTransport(configure); + } + } + """); + + // Variable references cannot be traced — analyzer warns. + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void NoWithHttpTransportCall_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + class Test + { + void DoNothing() { } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void ParenthesizedLambda_StatelessSet_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport((options) => options.Stateless = false); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void NamedArgument_NoDelegate_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(configureOptions: null); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void NamedArgument_WithStateless_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + builder.WithHttpTransport(configureOptions: o => o.Stateless = true); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void UnrelatedWithHttpTransportMethod_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + class SomeBuilder + { + public SomeBuilder WithHttpTransport() => this; + } + + class Test + { + void Configure() + { + new SomeBuilder().WithHttpTransport(); + } + } + """); + + Assert.Empty(diagnostics); + } + + [Fact] + public void StaticInvocation_NoDelegate_Reports() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + HttpMcpServerBuilderExtensions.WithHttpTransport(builder); + } + } + """); + + Assert.Single(diagnostics, d => d.Id == "MCP003"); + } + + [Fact] + public void StaticInvocation_WithStateless_DoesNotReport() + { + var diagnostics = RunAnalyzer(""" + using Microsoft.Extensions.DependencyInjection; + using ModelContextProtocol.Server; + + class Test + { + void Configure(IMcpServerBuilder builder) + { + HttpMcpServerBuilderExtensions.WithHttpTransport(builder, o => o.Stateless = true); + } + } + """); + + Assert.Empty(diagnostics); + } + + private List RunAnalyzer(string source) + { + // Combine stub types with user source. + var stubTree = CSharpSyntaxTree.ParseText(StubTypes); + var sourceTree = CSharpSyntaxTree.ParseText(source); + + var references = new List + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + }; + + var runtimePath = Path.GetDirectoryName(typeof(object).Assembly.Location)!; + references.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "System.Runtime.dll"))); + references.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "netstandard.dll"))); + + var compilation = CSharpCompilation.Create( + "TestAssembly", + [stubTree, sourceTree], + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + var analyzers = ImmutableArray.Create(new WithHttpTransportAnalyzer()); + var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); + var allDiagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); + + return allDiagnostics.Where(d => d.Id == "MCP003").ToList(); + } +}