From 8cf8baa4f71891ca54e2a3958af4868ae5c38be0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:07:16 +0000 Subject: [PATCH 1/3] Initial plan From aa9f2bc4852d4623cb4c294b2237778c9fa5406b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:16:36 +0000 Subject: [PATCH 2/3] Fix source generator diagnostic locations for incremental caching The DiagnosticInfo struct was storing Location objects directly but not including them in equality comparisons. This caused stale Location objects from earlier compilations to be reused when the generator cache returned cached values, leading to exceptions in Roslyn. Fix by: 1. Add LocationInfo record struct that stores serializable location data (file path, text span, line position span) 2. Update DiagnosticInfo to use LocationInfo instead of Location 3. Use record struct equality which automatically includes all fields 4. Reconstruct Location in CreateDiagnostic using Location.Create() Added a test to verify diagnostic locations are properly preserved. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../XmlToDescriptionGenerator.cs | 44 +++++++++--------- .../XmlToDescriptionGeneratorTests.cs | 46 +++++++++++++++++++ 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs b/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs index c0cad01fe..0842289f3 100644 --- a/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs +++ b/src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs @@ -72,7 +72,7 @@ private static Diagnostic CreateDiagnostic(DiagnosticInfo info) => "MCP001" => Diagnostics.InvalidXmlDocumentation, "MCP002" => Diagnostics.McpMethodMustBePartial, _ => throw new InvalidOperationException($"Unknown diagnostic ID: {info.Id}") - }, info.Location, info.MessageArgs); + }, info.Location?.ToLocation(), info.MessageArgs); private static IncrementalValuesProvider CreateProviderForAttribute( IncrementalGeneratorInitializationContext context, @@ -108,7 +108,7 @@ private static MethodToGenerate ExtractMethodInfo( HasGeneratableContent(xmlDocs, methodSymbol, descriptionAttribute)) { return MethodToGenerate.CreateDiagnosticOnly( - new DiagnosticInfo("MCP002", methodDeclaration.Identifier.GetLocation(), methodSymbol.Name)); + DiagnosticInfo.Create("MCP002", methodDeclaration.Identifier.GetLocation(), methodSymbol.Name)); } return MethodToGenerate.Empty; @@ -116,7 +116,7 @@ private static MethodToGenerate ExtractMethodInfo( // For partial methods with invalid XML, report diagnostic but still generate partial declaration. EquatableArray diagnostics = hasInvalidXml ? - new EquatableArray(ImmutableArray.Create(new DiagnosticInfo("MCP001", methodSymbol.Locations.FirstOrDefault(), methodSymbol.Name))) : + new EquatableArray(ImmutableArray.Create(DiagnosticInfo.Create("MCP001", methodSymbol.Locations.FirstOrDefault(), methodSymbol.Name))) : default; bool needsMethodDescription = xmlDocs is not null && @@ -514,29 +514,29 @@ private readonly record struct TypeDeclarationInfo( string Name, string TypeKeyword); - /// Holds diagnostic information to be reported. - private readonly struct DiagnosticInfo + /// Holds serializable location information for incremental generator caching. + /// + /// Roslyn objects cannot be stored in incremental generator cached data + /// because they contain references to syntax trees from specific compilations. Storing them + /// causes issues when the generator returns cached data with locations from earlier compilations. + /// + private readonly record struct LocationInfo(string FilePath, TextSpan TextSpan, LinePositionSpan LineSpan) { - public string Id { get; } - public Location? Location { get; } - public string MethodName { get; } - - public DiagnosticInfo(string id, Location? location, string methodName) - { - Id = id; - Location = location; - MethodName = methodName; - } - - public object?[] MessageArgs => [MethodName]; + public static LocationInfo? FromLocation(Location? location) => + location is null || !location.IsInSource ? null : + new LocationInfo(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span); - // For incremental generator caching, we compare only the logical content, not the Location object - public bool Equals(DiagnosticInfo other) => - Id == other.Id && MethodName == other.MethodName; + public Location ToLocation() => + Location.Create(FilePath, TextSpan, LineSpan); + } - public override bool Equals(object? obj) => obj is DiagnosticInfo other && Equals(other); + /// Holds diagnostic information to be reported. + private readonly record struct DiagnosticInfo(string Id, LocationInfo? Location, string MethodName) + { + public static DiagnosticInfo Create(string id, Location? location, string methodName) => + new(id, LocationInfo.FromLocation(location), methodName); - public override int GetHashCode() => (Id, MethodName).GetHashCode(); + public object?[] MessageArgs => [MethodName]; } /// Holds extracted XML documentation for a method (used only during extraction, not cached). diff --git a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs index 53fabfec3..a8e407029 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -729,6 +729,52 @@ partial class TestTools Assert.Contains("invalid", diagnostic.GetMessage(), StringComparison.OrdinalIgnoreCase); } + [Fact] + public void Generator_DiagnosticHasValidSourceLocation() + { + // This test verifies that diagnostic locations are properly reconstructed + // and point to valid source positions (regression test for locations from stale compilations) + var result = RunGenerator(""" + using ModelContextProtocol.Server; + using System.ComponentModel; + + namespace Test; + + [McpServerToolType] + public class TestTools + { + /// + /// Test tool + /// + [McpServerTool] + public static string TestMethod(string input) + { + return input; + } + } + """, "MCP002"); + + Assert.True(result.Success); + + // Verify the diagnostic has a valid location with correct line/column information + var diagnostic = Assert.Single(result.Diagnostics, d => d.Id == "MCP002"); + Assert.NotNull(diagnostic.Location); + + // Verify the location span has valid position information + var lineSpan = diagnostic.Location.GetLineSpan(); + Assert.True(lineSpan.IsValid, "Diagnostic line span should be valid"); + + // The line should point to the line containing "TestMethod" (line 14, 0-indexed would be 13) + // Line numbers may vary based on how the source is parsed, so just verify it's reasonable + Assert.True(lineSpan.StartLinePosition.Line >= 0, "Start line should be non-negative"); + Assert.True(lineSpan.StartLinePosition.Character >= 0, "Start character should be non-negative"); + Assert.True(lineSpan.EndLinePosition.Line >= lineSpan.StartLinePosition.Line, "End line should be >= start line"); + + // The span should have a non-zero length (the identifier "TestMethod" is 10 characters) + Assert.True(diagnostic.Location.SourceSpan.Length > 0, "Span should have non-zero length"); + Assert.Equal(10, diagnostic.Location.SourceSpan.Length); // "TestMethod".Length == 10 + } + [Fact] public void Generator_WithGenericType_GeneratesCorrectly() { From 1ddbd5d94afad0f5d1ef3da4a3347f0b63f07d01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:27:55 +0000 Subject: [PATCH 3/3] Address code review feedback: fix misleading comment in test Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../XmlToDescriptionGeneratorTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs index a8e407029..af5024fb0 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -764,8 +764,7 @@ public static string TestMethod(string input) var lineSpan = diagnostic.Location.GetLineSpan(); Assert.True(lineSpan.IsValid, "Diagnostic line span should be valid"); - // The line should point to the line containing "TestMethod" (line 14, 0-indexed would be 13) - // Line numbers may vary based on how the source is parsed, so just verify it's reasonable + // Verify reasonable location values without assuming specific line numbers Assert.True(lineSpan.StartLinePosition.Line >= 0, "Start line should be non-negative"); Assert.True(lineSpan.StartLinePosition.Character >= 0, "Start character should be non-negative"); Assert.True(lineSpan.EndLinePosition.Line >= lineSpan.StartLinePosition.Line, "End line should be >= start line");