From a4f6bc8647483d3aebf120ce9df11a8489215209 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 19:58:11 +0000 Subject: [PATCH 1/7] Initial plan From 00d647a1eafe96e74b2124f40f8796c1964554f7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 20:32:03 +0000 Subject: [PATCH 2/7] Add CS1066 suppressor for MCP server methods with optional parameters 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> --- .../CS1066Suppressor.cs | 118 ++++++++++ .../CS1066SuppressorTests.cs | 205 ++++++++++++++++++ 2 files changed, 323 insertions(+) create mode 100644 src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs create mode 100644 tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs diff --git a/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs new file mode 100644 index 00000000..6a50d800 --- /dev/null +++ b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs @@ -0,0 +1,118 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using System.Linq; + +namespace ModelContextProtocol.Analyzers; + +/// +/// Suppresses CS1066 warnings for MCP server methods that have optional parameters. +/// +/// +/// +/// CS1066 is issued when a partial method's implementing declaration has default parameter values. +/// For partial methods, only the defining declaration's defaults are used by callers, +/// making the implementing declaration's defaults redundant. +/// +/// +/// However, for MCP tool, prompt, and resource methods, users often want to specify default values +/// in their implementing declaration for documentation purposes. The XmlToDescriptionGenerator +/// automatically copies these defaults to the generated defining declaration, making them functional. +/// +/// +/// This suppressor suppresses CS1066 for methods marked with [McpServerTool], [McpServerPrompt], +/// or [McpServerResource] attributes, allowing users to specify defaults in their code without warnings. +/// +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class CS1066Suppressor : DiagnosticSuppressor +{ + private static readonly SuppressionDescriptor McpToolSuppression = new( + id: "MCP_CS1066_TOOL", + suppressedDiagnosticId: "CS1066", + justification: "Default values on MCP tool method implementing declarations are copied to the generated defining declaration by the source generator."); + + private static readonly SuppressionDescriptor McpPromptSuppression = new( + id: "MCP_CS1066_PROMPT", + suppressedDiagnosticId: "CS1066", + justification: "Default values on MCP prompt method implementing declarations are copied to the generated defining declaration by the source generator."); + + private static readonly SuppressionDescriptor McpResourceSuppression = new( + id: "MCP_CS1066_RESOURCE", + suppressedDiagnosticId: "CS1066", + justification: "Default values on MCP resource method implementing declarations are copied to the generated defining declaration by the source generator."); + + /// + public override ImmutableArray SupportedSuppressions => + ImmutableArray.Create(McpToolSuppression, McpPromptSuppression, McpResourceSuppression); + + /// + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + foreach (Diagnostic diagnostic in context.ReportedDiagnostics) + { + Location? location = diagnostic.Location; + SyntaxTree? tree = location.SourceTree; + if (tree is null) + { + continue; + } + + SyntaxNode root = tree.GetRoot(context.CancellationToken); + SyntaxNode? node = root.FindNode(location.SourceSpan); + + // Find the containing method declaration + MethodDeclarationSyntax? method = node.FirstAncestorOrSelf(); + if (method is null) + { + continue; + } + + // Check for MCP attributes + SuppressionDescriptor? suppression = GetSuppressionForMethod(method, context); + if (suppression is not null) + { + context.ReportSuppression(Suppression.Create(suppression, diagnostic)); + } + } + } + + private static SuppressionDescriptor? GetSuppressionForMethod(MethodDeclarationSyntax method, SuppressionAnalysisContext context) + { + SemanticModel? semanticModel = context.GetSemanticModel(method.SyntaxTree); + IMethodSymbol? methodSymbol = semanticModel.GetDeclaredSymbol(method, context.CancellationToken); + + if (methodSymbol is null) + { + return null; + } + + foreach (AttributeData attribute in methodSymbol.GetAttributes()) + { + string? fullName = attribute.AttributeClass?.ToDisplayString(); + if (fullName is null) + { + continue; + } + + if (fullName == "ModelContextProtocol.Server.McpServerToolAttribute") + { + return McpToolSuppression; + } + + if (fullName == "ModelContextProtocol.Server.McpServerPromptAttribute") + { + return McpPromptSuppression; + } + + if (fullName == "ModelContextProtocol.Server.McpServerResourceAttribute") + { + return McpResourceSuppression; + } + } + + return null; + } +} diff --git a/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs new file mode 100644 index 00000000..e0589aba --- /dev/null +++ b/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs @@ -0,0 +1,205 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using Xunit; + +namespace ModelContextProtocol.Analyzers.Tests; + +public class CS1066SuppressorTests +{ + [Fact] + public void Suppressor_WithMcpServerToolAttribute_SuppressesCS1066() + { + var result = RunSuppressor(""" + using ModelContextProtocol.Server; + + namespace Test; + + [McpServerToolType] + public partial class TestTools + { + [McpServerTool] + public partial string TestMethod(string input = "default"); + } + + public partial class TestTools + { + public partial string TestMethod(string input = "default") + { + return input; + } + } + """); + + // CS1066 should be suppressed + Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); + Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + } + + [Fact] + public void Suppressor_WithMcpServerPromptAttribute_SuppressesCS1066() + { + var result = RunSuppressor(""" + using ModelContextProtocol.Server; + + namespace Test; + + [McpServerPromptType] + public partial class TestPrompts + { + [McpServerPrompt] + public partial string TestPrompt(string input = "default"); + } + + public partial class TestPrompts + { + public partial string TestPrompt(string input = "default") + { + return input; + } + } + """); + + // CS1066 should be suppressed + Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); + Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + } + + [Fact] + public void Suppressor_WithMcpServerResourceAttribute_SuppressesCS1066() + { + var result = RunSuppressor(""" + using ModelContextProtocol.Server; + + namespace Test; + + [McpServerResourceType] + public partial class TestResources + { + [McpServerResource("test://resource")] + public partial string TestResource(string input = "default"); + } + + public partial class TestResources + { + public partial string TestResource(string input = "default") + { + return input; + } + } + """); + + // CS1066 should be suppressed + Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); + Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + } + + [Fact] + public void Suppressor_WithoutMcpAttribute_DoesNotSuppressCS1066() + { + var result = RunSuppressor(""" + namespace Test; + + public partial class TestTools + { + public partial string TestMethod(string input = "default"); + } + + public partial class TestTools + { + public partial string TestMethod(string input = "default") + { + return input; + } + } + """); + + // CS1066 should NOT be suppressed (no MCP attribute) + Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && !d.IsSuppressed); + Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && d.IsSuppressed)); + } + + [Fact] + public void Suppressor_WithMultipleParameters_SuppressesAllCS1066() + { + var result = RunSuppressor(""" + using ModelContextProtocol.Server; + + namespace Test; + + [McpServerToolType] + public partial class TestTools + { + [McpServerTool] + public partial string TestMethod(string input = "default", int count = 42, bool flag = false); + } + + public partial class TestTools + { + public partial string TestMethod(string input = "default", int count = 42, bool flag = false) + { + return input; + } + } + """); + + // All CS1066 warnings should be suppressed + var cs1066Diagnostics = result.Diagnostics.Where(d => d.Id == "CS1066").ToList(); + Assert.Equal(3, cs1066Diagnostics.Count); // Three parameters with defaults + Assert.All(cs1066Diagnostics, d => Assert.True(d.IsSuppressed)); + } + + private SuppressorResult RunSuppressor(string source) + { + var syntaxTree = CSharpSyntaxTree.ParseText(source); + + // Get reference assemblies + List referenceList = + [ + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + MetadataReference.CreateFromFile(typeof(System.ComponentModel.DescriptionAttribute).Assembly.Location), + ]; + + // Add all necessary runtime assemblies + var runtimePath = Path.GetDirectoryName(typeof(object).Assembly.Location)!; + referenceList.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "System.Runtime.dll"))); + referenceList.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "netstandard.dll"))); + + // Add ModelContextProtocol.Core + try + { + var coreAssemblyPath = Path.Combine(AppContext.BaseDirectory, "ModelContextProtocol.Core.dll"); + if (File.Exists(coreAssemblyPath)) + { + referenceList.Add(MetadataReference.CreateFromFile(coreAssemblyPath)); + } + } + catch + { + // If we can't find it, the compilation will fail with appropriate errors + } + + var compilation = CSharpCompilation.Create( + "TestAssembly", + [syntaxTree], + referenceList, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + // Run the suppressor + var analyzers = ImmutableArray.Create(new CS1066Suppressor()); + + var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); + var diagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); + + return new SuppressorResult + { + Diagnostics = diagnostics.ToList() + }; + } + + private class SuppressorResult + { + public List Diagnostics { get; set; } = []; + } +} From 009399b02d18839dfbfed9c03e31962fa5f12f22 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 20:43:39 +0000 Subject: [PATCH 3/7] Address code review feedback for CS1066 suppressor - 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> --- .../CS1066Suppressor.cs | 20 ++++- .../CS1066SuppressorTests.cs | 83 ++++++++++++------- 2 files changed, 71 insertions(+), 32 deletions(-) diff --git a/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs index 6a50d800..d5c46370 100644 --- a/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs +++ b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs @@ -2,8 +2,10 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; namespace ModelContextProtocol.Analyzers; @@ -51,6 +53,9 @@ public sealed class CS1066Suppressor : DiagnosticSuppressor /// public override void ReportSuppressions(SuppressionAnalysisContext context) { + // Cache semantic models per syntax tree to avoid redundant calls + Dictionary? semanticModelCache = null; + foreach (Diagnostic diagnostic in context.ReportedDiagnostics) { Location? location = diagnostic.Location; @@ -70,8 +75,16 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) continue; } + // Get or cache the semantic model for this tree + semanticModelCache ??= new Dictionary(); + if (!semanticModelCache.TryGetValue(tree, out SemanticModel? semanticModel)) + { + semanticModel = context.GetSemanticModel(tree); + semanticModelCache[tree] = semanticModel; + } + // Check for MCP attributes - SuppressionDescriptor? suppression = GetSuppressionForMethod(method, context); + SuppressionDescriptor? suppression = GetSuppressionForMethod(method, semanticModel, context.CancellationToken); if (suppression is not null) { context.ReportSuppression(Suppression.Create(suppression, diagnostic)); @@ -79,10 +92,9 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) } } - private static SuppressionDescriptor? GetSuppressionForMethod(MethodDeclarationSyntax method, SuppressionAnalysisContext context) + private static SuppressionDescriptor? GetSuppressionForMethod(MethodDeclarationSyntax method, SemanticModel semanticModel, CancellationToken cancellationToken) { - SemanticModel? semanticModel = context.GetSemanticModel(method.SyntaxTree); - IMethodSymbol? methodSymbol = semanticModel.GetDeclaredSymbol(method, context.CancellationToken); + IMethodSymbol? methodSymbol = semanticModel.GetDeclaredSymbol(method, cancellationToken); if (methodSymbol is null) { diff --git a/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs index e0589aba..57eaaf41 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/CS1066SuppressorTests.cs @@ -32,9 +32,16 @@ public partial string TestMethod(string input = "default") } """); - // CS1066 should be suppressed - Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); - Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + // Check we have the CS1066 diagnostics from compiler + var cs1066FromCompiler = result.CompilerDiagnostics.Where(d => d.Id == "CS1066").ToList(); + + // CS1066 should be suppressed in the final diagnostics + var unsuppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed).ToList(); + var suppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && d.IsSuppressed).ToList(); + + Assert.True(cs1066FromCompiler.Count > 0 || suppressedCs1066.Count > 0, + $"Expected CS1066 diagnostics. Compiler diagnostics: {string.Join(", ", result.CompilerDiagnostics.Select(d => d.Id))}"); + Assert.Empty(unsuppressedCs1066); } [Fact] @@ -61,9 +68,16 @@ public partial string TestPrompt(string input = "default") } """); - // CS1066 should be suppressed - Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); - Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + // Check we have the CS1066 diagnostics from compiler + var cs1066FromCompiler = result.CompilerDiagnostics.Where(d => d.Id == "CS1066").ToList(); + + // CS1066 should be suppressed in the final diagnostics + var unsuppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed).ToList(); + var suppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && d.IsSuppressed).ToList(); + + Assert.True(cs1066FromCompiler.Count > 0 || suppressedCs1066.Count > 0, + $"Expected CS1066 diagnostics. Compiler diagnostics: {string.Join(", ", result.CompilerDiagnostics.Select(d => d.Id))}"); + Assert.Empty(unsuppressedCs1066); } [Fact] @@ -90,9 +104,16 @@ public partial string TestResource(string input = "default") } """); - // CS1066 should be suppressed - Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed)); - Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); + // Check we have the CS1066 diagnostics from compiler + var cs1066FromCompiler = result.CompilerDiagnostics.Where(d => d.Id == "CS1066").ToList(); + + // CS1066 should be suppressed in the final diagnostics + var unsuppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed).ToList(); + var suppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && d.IsSuppressed).ToList(); + + Assert.True(cs1066FromCompiler.Count > 0 || suppressedCs1066.Count > 0, + $"Expected CS1066 diagnostics. Compiler diagnostics: {string.Join(", ", result.CompilerDiagnostics.Select(d => d.Id))}"); + Assert.Empty(unsuppressedCs1066); } [Fact] @@ -116,8 +137,14 @@ public partial string TestMethod(string input = "default") """); // CS1066 should NOT be suppressed (no MCP attribute) - Assert.Contains(result.Diagnostics, d => d.Id == "CS1066" && !d.IsSuppressed); - Assert.Empty(result.Diagnostics.Where(d => d.Id == "CS1066" && d.IsSuppressed)); + // Check we have the CS1066 diagnostic from compiler + var cs1066FromCompiler = result.CompilerDiagnostics.Where(d => d.Id == "CS1066").ToList(); + Assert.NotEmpty(cs1066FromCompiler); + + // It should NOT be suppressed in the final diagnostics (still present as unsuppressed) + var unsuppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed).ToList(); + Assert.NotEmpty(unsuppressedCs1066); + Assert.DoesNotContain(result.Diagnostics, d => d.Id == "CS1066" && d.IsSuppressed); } [Fact] @@ -144,10 +171,13 @@ public partial string TestMethod(string input = "default", int count = 42, bool } """); + // Check we have CS1066 diagnostics from compiler (one per parameter with default) + var cs1066FromCompiler = result.CompilerDiagnostics.Where(d => d.Id == "CS1066").ToList(); + Assert.Equal(3, cs1066FromCompiler.Count); // Three parameters with defaults + // All CS1066 warnings should be suppressed - var cs1066Diagnostics = result.Diagnostics.Where(d => d.Id == "CS1066").ToList(); - Assert.Equal(3, cs1066Diagnostics.Count); // Three parameters with defaults - Assert.All(cs1066Diagnostics, d => Assert.True(d.IsSuppressed)); + var unsuppressedCs1066 = result.Diagnostics.Where(d => d.Id == "CS1066" && !d.IsSuppressed).ToList(); + Assert.Empty(unsuppressedCs1066); } private SuppressorResult RunSuppressor(string source) @@ -166,18 +196,11 @@ private SuppressorResult RunSuppressor(string source) referenceList.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "System.Runtime.dll"))); referenceList.Add(MetadataReference.CreateFromFile(Path.Combine(runtimePath, "netstandard.dll"))); - // Add ModelContextProtocol.Core - try + // Add ModelContextProtocol.Core if available + var coreAssemblyPath = Path.Combine(AppContext.BaseDirectory, "ModelContextProtocol.Core.dll"); + if (File.Exists(coreAssemblyPath)) { - var coreAssemblyPath = Path.Combine(AppContext.BaseDirectory, "ModelContextProtocol.Core.dll"); - if (File.Exists(coreAssemblyPath)) - { - referenceList.Add(MetadataReference.CreateFromFile(coreAssemblyPath)); - } - } - catch - { - // If we can't find it, the compilation will fail with appropriate errors + referenceList.Add(MetadataReference.CreateFromFile(coreAssemblyPath)); } var compilation = CSharpCompilation.Create( @@ -186,20 +209,24 @@ private SuppressorResult RunSuppressor(string source) referenceList, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + // Get compilation diagnostics first (includes CS1066) + var compilerDiagnostics = compilation.GetDiagnostics(); + // Run the suppressor var analyzers = ImmutableArray.Create(new CS1066Suppressor()); - var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); - var diagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); + var allDiagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); return new SuppressorResult { - Diagnostics = diagnostics.ToList() + Diagnostics = allDiagnostics.ToList(), + CompilerDiagnostics = compilerDiagnostics.ToList() }; } private class SuppressorResult { public List Diagnostics { get; set; } = []; + public List CompilerDiagnostics { get; set; } = []; } } From aad7546cdadd8e25d122097a27da8e343f8faa82 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 22:36:11 +0000 Subject: [PATCH 4/7] Address review feedback: factor out attribute constants and match on 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> --- .../CS1066Suppressor.cs | 36 ++++++++++++++----- .../McpAttributeNames.cs | 12 +++++++ .../XmlToDescriptionGenerator.cs | 12 +++---- 3 files changed, 43 insertions(+), 17 deletions(-) create mode 100644 src/ModelContextProtocol.Analyzers/McpAttributeNames.cs diff --git a/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs index d5c46370..ff8cdcc3 100644 --- a/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs +++ b/src/ModelContextProtocol.Analyzers/CS1066Suppressor.cs @@ -4,7 +4,6 @@ using Microsoft.CodeAnalysis.Diagnostics; using System.Collections.Generic; using System.Collections.Immutable; -using System.Linq; using System.Threading; namespace ModelContextProtocol.Analyzers; @@ -53,8 +52,12 @@ public sealed class CS1066Suppressor : DiagnosticSuppressor /// public override void ReportSuppressions(SuppressionAnalysisContext context) { - // Cache semantic models per syntax tree to avoid redundant calls + // Cache semantic models and attribute symbols per syntax tree/compilation to avoid redundant calls Dictionary? semanticModelCache = null; + INamedTypeSymbol? mcpToolAttribute = null; + INamedTypeSymbol? mcpPromptAttribute = null; + INamedTypeSymbol? mcpResourceAttribute = null; + bool attributesResolved = false; foreach (Diagnostic diagnostic in context.ReportedDiagnostics) { @@ -83,8 +86,17 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) semanticModelCache[tree] = semanticModel; } + // Resolve attribute symbols once per compilation + if (!attributesResolved) + { + mcpToolAttribute = semanticModel.Compilation.GetTypeByMetadataName(McpAttributeNames.McpServerToolAttribute); + mcpPromptAttribute = semanticModel.Compilation.GetTypeByMetadataName(McpAttributeNames.McpServerPromptAttribute); + mcpResourceAttribute = semanticModel.Compilation.GetTypeByMetadataName(McpAttributeNames.McpServerResourceAttribute); + attributesResolved = true; + } + // Check for MCP attributes - SuppressionDescriptor? suppression = GetSuppressionForMethod(method, semanticModel, context.CancellationToken); + SuppressionDescriptor? suppression = GetSuppressionForMethod(method, semanticModel, mcpToolAttribute, mcpPromptAttribute, mcpResourceAttribute, context.CancellationToken); if (suppression is not null) { context.ReportSuppression(Suppression.Create(suppression, diagnostic)); @@ -92,7 +104,13 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) } } - private static SuppressionDescriptor? GetSuppressionForMethod(MethodDeclarationSyntax method, SemanticModel semanticModel, CancellationToken cancellationToken) + private static SuppressionDescriptor? GetSuppressionForMethod( + MethodDeclarationSyntax method, + SemanticModel semanticModel, + INamedTypeSymbol? mcpToolAttribute, + INamedTypeSymbol? mcpPromptAttribute, + INamedTypeSymbol? mcpResourceAttribute, + CancellationToken cancellationToken) { IMethodSymbol? methodSymbol = semanticModel.GetDeclaredSymbol(method, cancellationToken); @@ -103,23 +121,23 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) foreach (AttributeData attribute in methodSymbol.GetAttributes()) { - string? fullName = attribute.AttributeClass?.ToDisplayString(); - if (fullName is null) + INamedTypeSymbol? attributeClass = attribute.AttributeClass; + if (attributeClass is null) { continue; } - if (fullName == "ModelContextProtocol.Server.McpServerToolAttribute") + if (mcpToolAttribute is not null && SymbolEqualityComparer.Default.Equals(attributeClass, mcpToolAttribute)) { return McpToolSuppression; } - if (fullName == "ModelContextProtocol.Server.McpServerPromptAttribute") + if (mcpPromptAttribute is not null && SymbolEqualityComparer.Default.Equals(attributeClass, mcpPromptAttribute)) { return McpPromptSuppression; } - if (fullName == "ModelContextProtocol.Server.McpServerResourceAttribute") + if (mcpResourceAttribute is not null && SymbolEqualityComparer.Default.Equals(attributeClass, mcpResourceAttribute)) { return McpResourceSuppression; } diff --git a/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs b/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs new file mode 100644 index 00000000..f615d07d --- /dev/null +++ b/src/ModelContextProtocol.Analyzers/McpAttributeNames.cs @@ -0,0 +1,12 @@ +namespace ModelContextProtocol.Analyzers; + +/// +/// Contains the fully qualified metadata names for MCP server attributes. +/// +internal static class McpAttributeNames +{ + public const string McpServerToolAttribute = "ModelContextProtocol.Server.McpServerToolAttribute"; + public const string McpServerPromptAttribute = "ModelContextProtocol.Server.McpServerPromptAttribute"; + public const string McpServerResourceAttribute = "ModelContextProtocol.Server.McpServerResourceAttribute"; + public const string DescriptionAttribute = "System.ComponentModel.DescriptionAttribute"; +} diff --git a/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs b/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs index 992a8394..71982a3f 100644 --- a/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs +++ b/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs @@ -18,18 +18,14 @@ namespace ModelContextProtocol.Analyzers; public sealed class XmlToDescriptionGenerator : IIncrementalGenerator { private const string GeneratedFileName = "ModelContextProtocol.Descriptions.g.cs"; - private const string McpServerToolAttributeName = "ModelContextProtocol.Server.McpServerToolAttribute"; - private const string McpServerPromptAttributeName = "ModelContextProtocol.Server.McpServerPromptAttribute"; - private const string McpServerResourceAttributeName = "ModelContextProtocol.Server.McpServerResourceAttribute"; - private const string DescriptionAttributeName = "System.ComponentModel.DescriptionAttribute"; public void Initialize(IncrementalGeneratorInitializationContext context) { // Extract method information for all MCP tools, prompts, and resources. // The transform extracts all necessary data upfront so the output doesn't depend on the compilation. - var allMethods = CreateProviderForAttribute(context, McpServerToolAttributeName).Collect() - .Combine(CreateProviderForAttribute(context, McpServerPromptAttributeName).Collect()) - .Combine(CreateProviderForAttribute(context, McpServerResourceAttributeName).Collect()) + var allMethods = CreateProviderForAttribute(context, McpAttributeNames.McpServerToolAttribute).Collect() + .Combine(CreateProviderForAttribute(context, McpAttributeNames.McpServerPromptAttribute).Collect()) + .Combine(CreateProviderForAttribute(context, McpAttributeNames.McpServerResourceAttribute).Collect()) .Select(static (tuple, _) => { var ((tools, prompts), resources) = tuple; @@ -84,7 +80,7 @@ private static MethodToGenerate ExtractMethodInfo( Compilation compilation) { bool isPartial = methodDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword); - var descriptionAttribute = compilation.GetTypeByMetadataName(DescriptionAttributeName); + var descriptionAttribute = compilation.GetTypeByMetadataName(McpAttributeNames.DescriptionAttribute); // Try to extract XML documentation var (xmlDocs, hasInvalidXml) = TryExtractXmlDocumentation(methodSymbol); From db0ba50274dc68e722cfb8bc7840069b0f63d3f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 22:50:19 +0000 Subject: [PATCH 5/7] Update generator tests to validate CS1066 warnings are suppressed 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> --- .../XmlToDescriptionGeneratorTests.cs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs index 0feacd0b..c9abeb34 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -1,5 +1,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using Xunit; @@ -1755,15 +1757,28 @@ private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source) var driver = (CSharpGeneratorDriver)CSharpGeneratorDriver .Create(new XmlToDescriptionGenerator()) - .RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics); + .RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var generatorDiagnostics); var runResult = driver.GetRunResult(); + // Check for generator errors (the original check) + var hasGeneratorErrors = generatorDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error); + + // Run the suppressor and check specifically for unsuppressed CS1066 warnings + var analyzers = ImmutableArray.Create(new CS1066Suppressor()); + var compilationWithAnalyzers = outputCompilation.WithAnalyzers(analyzers); + var allDiagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); + + // Check for any unsuppressed CS1066 warnings (these should be suppressed by our suppressor) + var unsuppressedCs1066 = allDiagnostics + .Where(d => d.Id == "CS1066" && !d.IsSuppressed) + .ToList(); + return new GeneratorRunResult { - Success = !diagnostics.Any(d => d.Severity == DiagnosticSeverity.Error), + Success = !hasGeneratorErrors && unsuppressedCs1066.Count == 0, GeneratedSources = runResult.GeneratedTrees.Select(t => (t.FilePath, t.GetText())).ToList(), - Diagnostics = diagnostics.ToList(), + Diagnostics = generatorDiagnostics.Concat(unsuppressedCs1066).ToList(), Compilation = outputCompilation }; } From 846a93c2446b4326084a749e0acb7ea61bdc1707 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Dec 2025 01:52:16 +0000 Subject: [PATCH 6/7] Update generator tests to validate CS1066 warnings are suppressed - 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> --- .../XmlToDescriptionGeneratorTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs index c9abeb34..4a46393c 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -1761,15 +1761,16 @@ private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source) var runResult = driver.GetRunResult(); - // Check for generator errors (the original check) + // Check for generator errors (warnings are allowed as some tests explicitly expect them, e.g., MCP001) var hasGeneratorErrors = generatorDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error); - // Run the suppressor and check specifically for unsuppressed CS1066 warnings + // Run the suppressor to check that CS1066 warnings for MCP methods are suppressed var analyzers = ImmutableArray.Create(new CS1066Suppressor()); var compilationWithAnalyzers = outputCompilation.WithAnalyzers(analyzers); var allDiagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); - // Check for any unsuppressed CS1066 warnings (these should be suppressed by our suppressor) + // Check for any unsuppressed CS1066 warnings - these should be suppressed by our suppressor + // This is specifically what we're testing - that the suppressor works correctly var unsuppressedCs1066 = allDiagnostics .Where(d => d.Id == "CS1066" && !d.IsSuppressed) .ToList(); From f8114557ee20fe5f326efccde8c19ed89cfb31eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Dec 2025 02:25:15 +0000 Subject: [PATCH 7/7] Update generator tests to fail on any unexpected diagnostic - 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> --- .../XmlToDescriptionGeneratorTests.cs | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs index 4a46393c..b2cf8365 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -349,7 +349,7 @@ public static string TestMethod(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -376,7 +376,7 @@ public static string TestMethod(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -407,7 +407,7 @@ public static string TestMethod(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -436,7 +436,7 @@ public static string TestMethod(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -554,7 +554,7 @@ public static string TestMethod(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -585,7 +585,7 @@ public static string TestPrompt(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -616,7 +616,7 @@ public static string TestResource(string input) return input; } } - """); + """, "MCP002"); Assert.True(result.Success); Assert.Empty(result.GeneratedSources); @@ -697,7 +697,7 @@ public static partial string TestInvalidXml(string input) return input; } } - """); + """, "MCP001"); // Should not throw, generates partial implementation without Description attributes Assert.True(result.Success); @@ -1719,7 +1719,7 @@ partial class TestTools AssertGeneratedSourceEquals(expected, result.GeneratedSources[0].SourceText.ToString()); } - private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source) + private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source, params string[] expectedDiagnosticIds) { var syntaxTree = CSharpSyntaxTree.ParseText(source); @@ -1761,25 +1761,30 @@ private GeneratorRunResult RunGenerator([StringSyntax("C#-test")] string source) var runResult = driver.GetRunResult(); - // Check for generator errors (warnings are allowed as some tests explicitly expect them, e.g., MCP001) - var hasGeneratorErrors = generatorDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error); - // Run the suppressor to check that CS1066 warnings for MCP methods are suppressed var analyzers = ImmutableArray.Create(new CS1066Suppressor()); var compilationWithAnalyzers = outputCompilation.WithAnalyzers(analyzers); var allDiagnostics = compilationWithAnalyzers.GetAllDiagnosticsAsync().GetAwaiter().GetResult(); // Check for any unsuppressed CS1066 warnings - these should be suppressed by our suppressor - // This is specifically what we're testing - that the suppressor works correctly var unsuppressedCs1066 = allDiagnostics .Where(d => d.Id == "CS1066" && !d.IsSuppressed) .ToList(); + // Collect all diagnostics from the generator (any verbosity level) + var allGeneratorDiagnostics = generatorDiagnostics.Concat(unsuppressedCs1066).ToList(); + + // Check for unexpected diagnostics - any diagnostic that isn't in the expected list + var expectedSet = new HashSet(expectedDiagnosticIds); + var unexpectedDiagnostics = allGeneratorDiagnostics + .Where(d => !expectedSet.Contains(d.Id)) + .ToList(); + return new GeneratorRunResult { - Success = !hasGeneratorErrors && unsuppressedCs1066.Count == 0, + Success = unexpectedDiagnostics.Count == 0, GeneratedSources = runResult.GeneratedTrees.Select(t => (t.FilePath, t.GetText())).ToList(), - Diagnostics = generatorDiagnostics.Concat(unsuppressedCs1066).ToList(), + Diagnostics = allGeneratorDiagnostics, Compilation = outputCompilation }; }