Skip to content

Commit 84cd99a

Browse files
CopilotcaptainsafiaCopilot
authored
Unify handling of documentation IDs in OpenAPI XML comment generator (#62692)
Co-authored-by: captainsafia <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Safia Abdalla <[email protected]>
1 parent 36e93fa commit 84cd99a

10 files changed

+866
-61
lines changed

‎src/OpenApi/gen/XmlCommentGenerator.Emitter.cs‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,33 @@ private static string GetTypeDocId(Type type, bool includeGenericArguments, bool
302302
// For non-generic types, use FullName (if available) and replace nested type separators.
303303
return (type.FullName ?? type.Name).Replace('+', '.');
304304
}
305+
306+
/// <summary>
307+
/// Normalizes a documentation comment ID to match the compiler-style format.
308+
/// Strips the return type suffix for ordinary methods but retains it for conversion operators.
309+
/// </summary>
310+
/// <param name="docId">The documentation comment ID to normalize.</param>
311+
/// <returns>The normalized documentation comment ID.</returns>
312+
public static string NormalizeDocId(string docId)
313+
{
314+
// Find the tilde character that indicates the return type suffix
315+
var tildeIndex = docId.IndexOf('~');
316+
if (tildeIndex == -1)
317+
{
318+
// No return type suffix, return as-is
319+
return docId;
320+
}
321+
322+
// Check if this is a conversion operator (op_Implicit or op_Explicit)
323+
// For these operators, we need to keep the return type suffix
324+
if (docId.Contains("op_Implicit") || docId.Contains("op_Explicit"))
325+
{
326+
return docId;
327+
}
328+
329+
// For ordinary methods, strip the return type suffix
330+
return docId.Substring(0, tildeIndex);
331+
}
305332
}
306333
307334
{{GeneratedCodeAttribute}}
@@ -317,7 +344,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
317344
{
318345
return Task.CompletedTask;
319346
}
320-
if (XmlCommentCache.Cache.TryGetValue(methodInfo.CreateDocumentationId(), out var methodComment))
347+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(methodInfo.CreateDocumentationId()), out var methodComment))
321348
{
322349
if (methodComment.Summary is { } summary)
323350
{
@@ -423,7 +450,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
423450
{
424451
if (context.JsonPropertyInfo is { AttributeProvider: PropertyInfo propertyInfo })
425452
{
426-
if (XmlCommentCache.Cache.TryGetValue(propertyInfo.CreateDocumentationId(), out var propertyComment))
453+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyInfo.CreateDocumentationId()), out var propertyComment))
427454
{
428455
schema.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
429456
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
@@ -432,7 +459,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
432459
}
433460
}
434461
}
435-
if (XmlCommentCache.Cache.TryGetValue(context.JsonTypeInfo.Type.CreateDocumentationId(), out var typeComment))
462+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(context.JsonTypeInfo.Type.CreateDocumentationId()), out var typeComment))
436463
{
437464
schema.Description = typeComment.Summary;
438465
if (typeComment.Examples?.FirstOrDefault() is { } jsonString)

‎src/OpenApi/gen/XmlCommentGenerator.Parser.cs‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Globalization;
67
using System.Threading;
@@ -14,6 +15,32 @@ namespace Microsoft.AspNetCore.OpenApi.SourceGenerators;
1415

1516
public sealed partial class XmlCommentGenerator
1617
{
18+
/// <summary>
19+
/// Normalizes a documentation comment ID to match the compiler-style format.
20+
/// Strips the return type suffix for ordinary methods but retains it for conversion operators.
21+
/// </summary>
22+
/// <param name="docId">The documentation comment ID to normalize.</param>
23+
/// <returns>The normalized documentation comment ID.</returns>
24+
internal static string NormalizeDocId(string docId)
25+
{
26+
// Find the tilde character that indicates the return type suffix
27+
var tildeIndex = docId.IndexOf('~');
28+
if (tildeIndex == -1)
29+
{
30+
// No return type suffix, return as-is
31+
return docId;
32+
}
33+
34+
// Check if this is a conversion operator (op_Implicit or op_Explicit)
35+
// For these operators, we need to keep the return type suffix
36+
if (docId.Contains("op_Implicit", StringComparison.Ordinal) || docId.Contains("op_Explicit", StringComparison.Ordinal))
37+
{
38+
return docId;
39+
}
40+
41+
// For ordinary methods, strip the return type suffix
42+
return docId.Substring(0, tildeIndex);
43+
}
1744
internal static List<(string, string)> ParseXmlFile(AdditionalText additionalText, CancellationToken cancellationToken)
1845
{
1946
var text = additionalText.GetText(cancellationToken);
@@ -37,7 +64,7 @@ public sealed partial class XmlCommentGenerator
3764
var name = member.Attribute(DocumentationCommentXmlNames.NameAttributeName)?.Value;
3865
if (name is not null)
3966
{
40-
comments.Add((name, member.ToString()));
67+
comments.Add((NormalizeDocId(name), member.ToString()));
4168
}
4269
}
4370
return comments;
@@ -54,7 +81,7 @@ public sealed partial class XmlCommentGenerator
5481
if (DocumentationCommentId.CreateDeclarationId(type) is string name &&
5582
type.GetDocumentationCommentXml(CultureInfo.InvariantCulture, expandIncludes: true, cancellationToken: cancellationToken) is string xml)
5683
{
57-
comments.Add((name, xml));
84+
comments.Add((NormalizeDocId(name), xml));
5885
}
5986
}
6087
var properties = visitor.GetPublicProperties();
@@ -63,7 +90,7 @@ public sealed partial class XmlCommentGenerator
6390
if (DocumentationCommentId.CreateDeclarationId(property) is string name &&
6491
property.GetDocumentationCommentXml(CultureInfo.InvariantCulture, expandIncludes: true, cancellationToken: cancellationToken) is string xml)
6592
{
66-
comments.Add((name, xml));
93+
comments.Add((NormalizeDocId(name), xml));
6794
}
6895
}
6996
var methods = visitor.GetPublicMethods();
@@ -77,7 +104,7 @@ public sealed partial class XmlCommentGenerator
77104
if (DocumentationCommentId.CreateDeclarationId(method) is string name &&
78105
method.GetDocumentationCommentXml(CultureInfo.InvariantCulture, expandIncludes: true, cancellationToken: cancellationToken) is string xml)
79106
{
80-
comments.Add((name, xml));
107+
comments.Add((NormalizeDocId(name), xml));
81108
}
82109
}
83110
return comments;

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/XmlCommentDocumentationIdTests.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Net.Http;
5+
using System.Text.Json.Nodes;
6+
7+
namespace Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests;
8+
9+
[UsesVerify]
10+
public class XmlCommentDocumentationIdTests
11+
{
12+
[Fact]
13+
public async Task CanMergeXmlCommentsWithDifferentDocumentationIdFormats()
14+
{
15+
// This test verifies that XML comments from referenced assemblies (without return type suffix)
16+
// are properly merged with in-memory symbols (with return type suffix)
17+
var source = """
18+
using System;
19+
using System.Threading.Tasks;
20+
using Microsoft.AspNetCore.Builder;
21+
using Microsoft.Extensions.DependencyInjection;
22+
using ReferencedLibrary;
23+
24+
var builder = WebApplication.CreateBuilder();
25+
26+
builder.Services.AddOpenApi();
27+
28+
var app = builder.Build();
29+
30+
app.MapPost("/test-method", ReferencedLibrary.TestApi.TestMethod);
31+
32+
app.Run();
33+
""";
34+
35+
var referencedLibrarySource = """
36+
using System;
37+
using System.Threading.Tasks;
38+
39+
namespace ReferencedLibrary;
40+
41+
public static class TestApi
42+
{
43+
/// <summary>
44+
/// This method should have its XML comment merged properly.
45+
/// </summary>
46+
/// <param name="id">The identifier for the test.</param>
47+
/// <returns>A task representing the asynchronous operation.</returns>
48+
public static Task TestMethod(int id)
49+
{
50+
return Task.CompletedTask;
51+
}
52+
}
53+
""";
54+
55+
var references = new Dictionary<string, List<string>>
56+
{
57+
{ "ReferencedLibrary", [referencedLibrarySource] }
58+
};
59+
60+
var generator = new XmlCommentGenerator();
61+
await SnapshotTestHelper.Verify(source, generator, references, out var compilation, out var additionalAssemblies);
62+
await SnapshotTestHelper.VerifyOpenApi(compilation, additionalAssemblies, document =>
63+
{
64+
var path = document.Paths["/test-method"].Operations[HttpMethod.Post];
65+
66+
// Verify that the XML comment from the referenced library was properly merged
67+
// This would fail before the fix because the documentation IDs didn't match
68+
Assert.NotNull(path.Summary);
69+
Assert.Equal("This method should have its XML comment merged properly.", path.Summary);
70+
71+
// Verify the parameter comment is also available
72+
Assert.NotNull(path.Parameters);
73+
Assert.Single(path.Parameters);
74+
Assert.Equal("The identifier for the test.", path.Parameters[0].Description);
75+
});
76+
}
77+
78+
[Theory]
79+
[InlineData("M:Sample.MyMethod(System.Int32)~System.Threading.Tasks.Task", "M:Sample.MyMethod(System.Int32)")]
80+
[InlineData("M:Sample.MyMethod(System.Int32)", "M:Sample.MyMethod(System.Int32)")]
81+
[InlineData("M:Sample.op_Implicit(System.Int32)~Sample.MyClass", "M:Sample.op_Implicit(System.Int32)~Sample.MyClass")]
82+
[InlineData("M:Sample.op_Explicit(System.Int32)~Sample.MyClass", "M:Sample.op_Explicit(System.Int32)~Sample.MyClass")]
83+
[InlineData("T:Sample.MyClass", "T:Sample.MyClass")]
84+
[InlineData("P:Sample.MyClass.MyProperty", "P:Sample.MyClass.MyProperty")]
85+
public void NormalizeDocId_ReturnsExpectedResult(string input, string expected)
86+
{
87+
var result = XmlCommentGenerator.NormalizeDocId(input);
88+
Assert.Equal(expected, result);
89+
}
90+
}

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/snapshots/AddOpenApiTests.CanInterceptAddOpenApi#OpenApiXmlCommentSupport.generated.verified.cs‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,33 @@ private static string GetTypeDocId(Type type, bool includeGenericArguments, bool
284284
// For non-generic types, use FullName (if available) and replace nested type separators.
285285
return (type.FullName ?? type.Name).Replace('+', '.');
286286
}
287+
288+
/// <summary>
289+
/// Normalizes a documentation comment ID to match the compiler-style format.
290+
/// Strips the return type suffix for ordinary methods but retains it for conversion operators.
291+
/// </summary>
292+
/// <param name="docId">The documentation comment ID to normalize.</param>
293+
/// <returns>The normalized documentation comment ID.</returns>
294+
public static string NormalizeDocId(string docId)
295+
{
296+
// Find the tilde character that indicates the return type suffix
297+
var tildeIndex = docId.IndexOf('~');
298+
if (tildeIndex == -1)
299+
{
300+
// No return type suffix, return as-is
301+
return docId;
302+
}
303+
304+
// Check if this is a conversion operator (op_Implicit or op_Explicit)
305+
// For these operators, we need to keep the return type suffix
306+
if (docId.Contains("op_Implicit") || docId.Contains("op_Explicit"))
307+
{
308+
return docId;
309+
}
310+
311+
// For ordinary methods, strip the return type suffix
312+
return docId.Substring(0, tildeIndex);
313+
}
287314
}
288315

289316
[System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.AspNetCore.OpenApi.SourceGenerators, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60", "42.42.42.42")]
@@ -299,7 +326,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
299326
{
300327
return Task.CompletedTask;
301328
}
302-
if (XmlCommentCache.Cache.TryGetValue(methodInfo.CreateDocumentationId(), out var methodComment))
329+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(methodInfo.CreateDocumentationId()), out var methodComment))
303330
{
304331
if (methodComment.Summary is { } summary)
305332
{
@@ -405,7 +432,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
405432
{
406433
if (context.JsonPropertyInfo is { AttributeProvider: PropertyInfo propertyInfo })
407434
{
408-
if (XmlCommentCache.Cache.TryGetValue(propertyInfo.CreateDocumentationId(), out var propertyComment))
435+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyInfo.CreateDocumentationId()), out var propertyComment))
409436
{
410437
schema.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
411438
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
@@ -414,7 +441,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
414441
}
415442
}
416443
}
417-
if (XmlCommentCache.Cache.TryGetValue(context.JsonTypeInfo.Type.CreateDocumentationId(), out var typeComment))
444+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(context.JsonTypeInfo.Type.CreateDocumentationId()), out var typeComment))
418445
{
419446
schema.Description = typeComment.Summary;
420447
if (typeComment.Examples?.FirstOrDefault() is { } jsonString)

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/snapshots/AdditionalTextsTests.CanHandleXmlForSchemasInAdditionalTexts#OpenApiXmlCommentSupport.generated.verified.cs‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,33 @@ private static string GetTypeDocId(Type type, bool includeGenericArguments, bool
313313
// For non-generic types, use FullName (if available) and replace nested type separators.
314314
return (type.FullName ?? type.Name).Replace('+', '.');
315315
}
316+
317+
/// <summary>
318+
/// Normalizes a documentation comment ID to match the compiler-style format.
319+
/// Strips the return type suffix for ordinary methods but retains it for conversion operators.
320+
/// </summary>
321+
/// <param name="docId">The documentation comment ID to normalize.</param>
322+
/// <returns>The normalized documentation comment ID.</returns>
323+
public static string NormalizeDocId(string docId)
324+
{
325+
// Find the tilde character that indicates the return type suffix
326+
var tildeIndex = docId.IndexOf('~');
327+
if (tildeIndex == -1)
328+
{
329+
// No return type suffix, return as-is
330+
return docId;
331+
}
332+
333+
// Check if this is a conversion operator (op_Implicit or op_Explicit)
334+
// For these operators, we need to keep the return type suffix
335+
if (docId.Contains("op_Implicit") || docId.Contains("op_Explicit"))
336+
{
337+
return docId;
338+
}
339+
340+
// For ordinary methods, strip the return type suffix
341+
return docId.Substring(0, tildeIndex);
342+
}
316343
}
317344

318345
[System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.AspNetCore.OpenApi.SourceGenerators, Version=42.42.42.42, Culture=neutral, PublicKeyToken=adb9793829ddae60", "42.42.42.42")]
@@ -328,7 +355,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
328355
{
329356
return Task.CompletedTask;
330357
}
331-
if (XmlCommentCache.Cache.TryGetValue(methodInfo.CreateDocumentationId(), out var methodComment))
358+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(methodInfo.CreateDocumentationId()), out var methodComment))
332359
{
333360
if (methodComment.Summary is { } summary)
334361
{
@@ -434,7 +461,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
434461
{
435462
if (context.JsonPropertyInfo is { AttributeProvider: PropertyInfo propertyInfo })
436463
{
437-
if (XmlCommentCache.Cache.TryGetValue(propertyInfo.CreateDocumentationId(), out var propertyComment))
464+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyInfo.CreateDocumentationId()), out var propertyComment))
438465
{
439466
schema.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
440467
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
@@ -443,7 +470,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
443470
}
444471
}
445472
}
446-
if (XmlCommentCache.Cache.TryGetValue(context.JsonTypeInfo.Type.CreateDocumentationId(), out var typeComment))
473+
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(context.JsonTypeInfo.Type.CreateDocumentationId()), out var typeComment))
447474
{
448475
schema.Description = typeComment.Summary;
449476
if (typeComment.Examples?.FirstOrDefault() is { } jsonString)

0 commit comments

Comments
 (0)