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..af5024fb0 100644 --- a/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs +++ b/tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs @@ -729,6 +729,51 @@ 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"); + + // 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"); + + // 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() {