Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions src/ModelContextProtocol.Analyzers/XmlToDescriptionGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MethodToGenerate> CreateProviderForAttribute(
IncrementalGeneratorInitializationContext context,
Expand Down Expand Up @@ -108,15 +108,15 @@ 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;
}

// 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 &&
Expand Down Expand Up @@ -514,29 +514,29 @@ private readonly record struct TypeDeclarationInfo(
string Name,
string TypeKeyword);

/// <summary>Holds diagnostic information to be reported.</summary>
private readonly struct DiagnosticInfo
/// <summary>Holds serializable location information for incremental generator caching.</summary>
/// <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 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);
/// <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 override int GetHashCode() => (Id, MethodName).GetHashCode();
public object?[] MessageArgs => [MethodName];
}

/// <summary>Holds extracted XML documentation for a method (used only during extraction, not cached).</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
/// <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()
{
Expand Down