Fix source generator diagnostic locations for incremental caching (#1153)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>

src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs

@@ -72,7 +72,7 @@ public sealed class XmlToDescriptionGenerator : IIncrementalGenerator
             "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<MethodToGenerate> CreateProviderForAttribute(
         IncrementalGeneratorInitializationContext context,
@@ -108,7 +108,7 @@ public sealed class XmlToDescriptionGenerator : IIncrementalGenerator
                 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 @@ public sealed class XmlToDescriptionGenerator : IIncrementalGenerator
 
         // For partial methods with invalid XML, report diagnostic but still generate partial declaration.
         EquatableArray<DiagnosticInfo> diagnostics = hasInvalidXml ?
-            new EquatableArray<DiagnosticInfo>(ImmutableArray.Create(new DiagnosticInfo("MCP001", methodSymbol.Locations.FirstOrDefault(), methodSymbol.Name))) :
+            new EquatableArray<DiagnosticInfo>(ImmutableArray.Create(DiagnosticInfo.Create("MCP001", methodSymbol.Locations.FirstOrDefault(), methodSymbol.Name))) :
             default;
 
         bool needsMethodDescription = xmlDocs is not null &&
@@ -514,29 +514,29 @@ public sealed class XmlToDescriptionGenerator : IIncrementalGenerator
         string Name,
         string TypeKeyword);
 
-    /// <summary>Holds diagnostic information to be reported.</summary>
+    /// <summary>Holds serializable location information for incremental generator caching.</summary>
-    private readonly struct DiagnosticInfo
+    /// <remarks>
+    /// Roslyn <see cref="Location"/> 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.
+    /// </remarks>
+    private readonly record struct LocationInfo(string FilePath, TextSpan TextSpan, LinePositionSpan LineSpan)
     {
-        public string Id { get; }
+        public static LocationInfo? FromLocation(Location? location) =>
-        public Location? Location { get; }
+            location is null || !location.IsInSource ? null :
-        public string MethodName { get; }
+            new LocationInfo(location.SourceTree?.FilePath ?? "", location.SourceSpan, location.GetLineSpan().Span);
 
-        public DiagnosticInfo(string id, Location? location, string methodName)
+        public Location ToLocation() =>
-        {
+            Location.Create(FilePath, TextSpan, LineSpan);
-            Id = id;
+    }
-            Location = location;
+
-            MethodName = methodName;
+    /// <summary>Holds diagnostic information to be reported.</summary>
-        }
+    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 object?[] MessageArgs => [MethodName];
-
-        // 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 override bool Equals(object? obj) => obj is DiagnosticInfo other && Equals(other);
-
-        public override int GetHashCode() => (Id, MethodName).GetHashCode();
     }
 
     /// <summary>Holds extracted XML documentation for a method (used only during extraction, not cached).</summary>

tests/ModelContextProtocol.Analyzers.Tests/XmlToDescriptionGeneratorTests.cs

@@ -729,6 +729,51 @@ public partial class XmlToDescriptionGeneratorTests
         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
+            {
+                /// <summary>
+                /// Test tool
+                /// </summary>
+                [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()
     {