Skip to content

Commit 3f2b6b3

Browse files
captainsafiacaptainsafia
committed
Address feedback
1 parent afede74 commit 3f2b6b3

File tree

7 files changed

+146
-21
lines changed

7 files changed

+146
-21
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public sealed partial class XmlCommentGenerator
9494
if (DocumentationCommentId.GetFirstSymbolForDeclarationId(name, compilation) is ISymbol symbol &&
9595
// Only include symbols that are declared in the application assembly or are
9696
// accessible from the application assembly.
97-
(SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, input.Compilation.Assembly) || symbol.IsAccessibleType()) &&
97+
(SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, compilation.Assembly) || symbol.IsAccessibleType()) &&
9898
// Skip static classes that are just containers for members with annotations
9999
// since they cannot be instantiated.
100100
symbol is not INamedTypeSymbol { TypeKind: TypeKind.Class, IsStatic: true })
@@ -104,7 +104,7 @@ public sealed partial class XmlCommentGenerator
104104
{
105105
var memberKey = symbol switch
106106
{
107-
IMethodSymbol methodSymbol => MemberKey.FromMethodSymbol(methodSymbol, input.Compilation),
107+
IMethodSymbol methodSymbol => MemberKey.FromMethodSymbol(methodSymbol),
108108
IPropertySymbol propertySymbol => MemberKey.FromPropertySymbol(propertySymbol),
109109
INamedTypeSymbol typeSymbol => MemberKey.FromTypeSymbol(typeSymbol),
110110
_ => null

‎src/OpenApi/gen/XmlComments/MemberKey.cs‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ internal sealed record MemberKey(
2121
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
2222
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters);
2323

24-
private static readonly SymbolDisplayFormat _sansTypeParametersFormat = new(
24+
private static readonly SymbolDisplayFormat _withoutTypeParametersFormat = new(
2525
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Included,
2626
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
2727
genericsOptions: SymbolDisplayGenericsOptions.None);
2828

29-
public static MemberKey? FromMethodSymbol(IMethodSymbol method, Compilation compilation)
29+
public static MemberKey? FromMethodSymbol(IMethodSymbol method)
3030
{
3131
string returnType;
3232
if (method.ReturnsVoid)
@@ -35,20 +35,7 @@ internal sealed record MemberKey(
3535
}
3636
else
3737
{
38-
// Handle Task/ValueTask for async methods
3938
var actualReturnType = method.ReturnType;
40-
if (method.IsAsync && actualReturnType is INamedTypeSymbol namedType)
41-
{
42-
if (namedType.TypeArguments.Length > 0)
43-
{
44-
actualReturnType = namedType.TypeArguments[0];
45-
}
46-
else
47-
{
48-
actualReturnType = compilation.GetSpecialType(SpecialType.System_Void);
49-
}
50-
}
51-
5239
if (actualReturnType.TypeKind == TypeKind.TypeParameter)
5340
{
5441
returnType = "typeof(object)";
@@ -162,7 +149,7 @@ private static bool TryGetFormattedTypeName(ITypeSymbol typeSymbol, [NotNullWhen
162149
// generics if possible to avoid emitting a type with type parameters that
163150
// cannot be used in a typeof expression.
164151
var hasTypeParameters = genericType.TypeArguments.Any(t => t.TypeKind == TypeKind.TypeParameter);
165-
var baseTypeName = genericType.ToDisplayString(_sansTypeParametersFormat);
152+
var typeNameWithoutGenerics = genericType.ToDisplayString(_withoutTypeParametersFormat);
166153

167154
if (!hasTypeParameters)
168155
{
@@ -185,7 +172,7 @@ private static bool TryGetFormattedTypeName(ITypeSymbol typeSymbol, [NotNullWhen
185172

186173
if (allArgumentsResolved)
187174
{
188-
typeName = $"{baseTypeName}<{string.Join(", ", typeArgStrings)}>";
175+
typeName = $"{typeNameWithoutGenerics}<{string.Join(", ", typeArgStrings)}>";
189176
return true;
190177
}
191178
}
@@ -204,7 +191,7 @@ private static bool TryGetFormattedTypeName(ITypeSymbol typeSymbol, [NotNullWhen
204191
var genericArgumentsCount = genericType.TypeArguments.Length;
205192
var openGenericsPlaceholder = "<" + new string(',', genericArgumentsCount - 1) + ">";
206193

207-
typeName = baseTypeName + openGenericsPlaceholder;
194+
typeName = typeNameWithoutGenerics + openGenericsPlaceholder;
208195
return true;
209196
}
210197
}

‎src/OpenApi/src/Services/OpenApiDocumentProvider.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal sealed class OpenApiDocumentProvider(IServiceProvider serviceProvider)
2626
public async Task GenerateAsync(string documentName, TextWriter writer)
2727
{
2828
// See OpenApiServiceCollectionExtensions.cs to learn why we lowercase the document name
29-
var lowercasedDocumentName = documentName.ToLowerInvariant();
29+
var lowercasedDocumentName = documentName.ToLowerInvariant();
3030

3131
var options = serviceProvider.GetRequiredService<IOptionsMonitor<OpenApiOptions>>();
3232
var namedOption = options.Get(lowercasedDocumentName);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,11 @@ public class GenericParent
364364
/// </summary>
365365
public (int, Dictionary<int, string>) TupleWithGenericProp { get; set; }
366366
367+
/// <summary>
368+
/// This property is a tuple with a nested generic type inside.
369+
/// </summary>
370+
public (int, Dictionary<int, Dictionary<string, int>>) TupleWithNestedGenericProp { get; set; }
371+
367372
/// <summary>
368373
/// This method returns a generic type containing a tuple.
369374
/// </summary>
@@ -481,6 +486,7 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
481486
Assert.Equal("This property is a nullable reference type.", genericParent.Properties["name"].Description);
482487
Assert.Equal("This property is a generic type containing a tuple.", genericParent.Properties["taskOfTupleProp"].Description);
483488
Assert.Equal("This property is a tuple with a generic type inside.", genericParent.Properties["tupleWithGenericProp"].Description);
489+
Assert.Equal("This property is a tuple with a nested generic type inside.", genericParent.Properties["tupleWithNestedGenericProp"].Description);
484490

485491
path = document.Paths["/params-and-param-refs"].Operations[OperationType.Post];
486492
var paramsAndParamRefs = path.RequestBody.Content["application/json"].Schema;

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/OperationTests.MinimalApis.cs‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ public async Task SupportsXmlCommentsOnOperationsFromMinimalApis()
1414
{
1515
var source = """
1616
using System;
17+
using System.Threading.Tasks;
18+
using System.Collections.Generic;
1719
using Microsoft.AspNetCore.Builder;
1820
using Microsoft.Extensions.DependencyInjection;
1921
using Microsoft.AspNetCore.Http.HttpResults;
@@ -32,6 +34,15 @@ public async Task SupportsXmlCommentsOnOperationsFromMinimalApis()
3234
app.MapGet("/5", RouteHandlerExtensionMethods.Get5);
3335
app.MapPost("/6", RouteHandlerExtensionMethods.Post6);
3436
app.MapPut("/7", RouteHandlerExtensionMethods.Put7);
37+
app.MapGet("/8", RouteHandlerExtensionMethods.Get8);
38+
app.MapGet("/9", RouteHandlerExtensionMethods.Get9);
39+
app.MapGet("/10", RouteHandlerExtensionMethods.Get10);
40+
app.MapGet("/11", RouteHandlerExtensionMethods.Get11);
41+
app.MapGet("/12", RouteHandlerExtensionMethods.Get12);
42+
app.MapGet("/13", RouteHandlerExtensionMethods.Get13);
43+
app.MapGet("/14", RouteHandlerExtensionMethods.Get14);
44+
app.MapGet("/15", RouteHandlerExtensionMethods.Get15);
45+
app.MapPost("/16", RouteHandlerExtensionMethods.Post16);
3546
3647
app.Run();
3748
@@ -114,13 +125,97 @@ public static IResult Put7(int? id, string uuid)
114125
{
115126
return TypedResults.NoContent();
116127
}
128+
129+
/// <summary>
130+
/// A summary of Get8.
131+
/// </summary>
132+
public static async Task Get8()
133+
{
134+
await Task.Delay(1000);
135+
return;
136+
}
137+
/// <summary>
138+
/// A summary of Get9.
139+
/// </summary>
140+
public static async ValueTask Get9()
141+
{
142+
await Task.Delay(1000);
143+
return;
144+
}
145+
/// <summary>
146+
/// A summary of Get10.
147+
/// </summary>
148+
public static Task Get10()
149+
{
150+
return Task.CompletedTask;
151+
}
152+
/// <summary>
153+
/// A summary of Get11.
154+
/// </summary>
155+
public static ValueTask Get11()
156+
{
157+
return ValueTask.CompletedTask;
158+
}
159+
/// <summary>
160+
/// A summary of Get12.
161+
/// </summary>
162+
public static Task<string> Get12()
163+
{
164+
return Task.FromResult("Hello, World!");
165+
}
166+
/// <summary>
167+
/// A summary of Get13.
168+
/// </summary>
169+
public static ValueTask<string> Get13()
170+
{
171+
return new ValueTask<string>("Hello, World!");
172+
}
173+
/// <summary>
174+
/// A summary of Get14.
175+
/// </summary>
176+
public static async Task<Holder<string>> Get14()
177+
{
178+
await Task.Delay(1000);
179+
return new Holder<string> { Value = "Hello, World!" };
180+
}
181+
/// <summary>
182+
/// A summary of Get15.
183+
/// </summary>
184+
public static Task<Holder<string>> Get15()
185+
{
186+
return Task.FromResult(new Holder<string> { Value = "Hello, World!" });
187+
}
188+
189+
/// <summary>
190+
/// A summary of Post16.
191+
/// </summary>
192+
public static void Post16(Example example)
193+
{
194+
return;
195+
}
117196
}
118197
119198
public class User
120199
{
121200
public string Username { get; set; } = string.Empty;
122201
public string Email { get; set; } = string.Empty;
123202
}
203+
204+
public class Holder<T>
205+
{
206+
public T Value { get; set; } = default!;
207+
}
208+
209+
public class Example : Task<int>
210+
{
211+
public Example(Func<int> function) : base(function)
212+
{
213+
}
214+
215+
public Example(Func<object?, int> function, object? state) : base(function, state)
216+
{
217+
}
218+
}
124219
""";
125220
var generator = new XmlCommentGenerator();
126221
await SnapshotTestHelper.Verify(source, generator, out var compilation);
@@ -159,6 +254,33 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
159254
var idParam = path7.Parameters.First(p => p.Name == "id");
160255
Assert.True(idParam.Deprecated);
161256
Assert.Equal("Legacy ID parameter - use uuid instead.", idParam.Description);
257+
258+
var path8 = document.Paths["/8"].Operations[OperationType.Get];
259+
Assert.Equal("A summary of Get8.", path8.Summary);
260+
261+
var path9 = document.Paths["/9"].Operations[OperationType.Get];
262+
Assert.Equal("A summary of Get9.", path9.Summary);
263+
264+
var path10 = document.Paths["/10"].Operations[OperationType.Get];
265+
Assert.Equal("A summary of Get10.", path10.Summary);
266+
267+
var path11 = document.Paths["/11"].Operations[OperationType.Get];
268+
Assert.Equal("A summary of Get11.", path11.Summary);
269+
270+
var path12 = document.Paths["/12"].Operations[OperationType.Get];
271+
Assert.Equal("A summary of Get12.", path12.Summary);
272+
273+
var path13 = document.Paths["/13"].Operations[OperationType.Get];
274+
Assert.Equal("A summary of Get13.", path13.Summary);
275+
276+
var path14 = document.Paths["/14"].Operations[OperationType.Get];
277+
Assert.Equal("A summary of Get14.", path14.Summary);
278+
279+
var path15 = document.Paths["/15"].Operations[OperationType.Get];
280+
Assert.Equal("A summary of Get15.", path15.Summary);
281+
282+
var path16 = document.Paths["/16"].Operations[OperationType.Post];
283+
Assert.Equal("A summary of Post16.", path16.Summary);
162284
});
163285
}
164286
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ Note that there isn't a way to provide a ""cref"" to
273273
_cache.Add(new MemberKey(typeof(global::GenericParent), MemberType.Property, "Name", null, []), new XmlComment(@"This property is a nullable reference type.", null, null, null, null, false, null, null, null));
274274
_cache.Add(new MemberKey(typeof(global::GenericParent), MemberType.Property, "TaskOfTupleProp", null, []), new XmlComment(@"This property is a generic type containing a tuple.", null, null, null, null, false, null, null, null));
275275
_cache.Add(new MemberKey(typeof(global::GenericParent), MemberType.Property, "TupleWithGenericProp", null, []), new XmlComment(@"This property is a tuple with a generic type inside.", null, null, null, null, false, null, null, null));
276+
_cache.Add(new MemberKey(typeof(global::GenericParent), MemberType.Property, "TupleWithNestedGenericProp", null, []), new XmlComment(@"This property is a tuple with a nested generic type inside.", null, null, null, null, false, null, null, null));
276277
_cache.Add(new MemberKey(typeof(global::ExampleClass), MemberType.Method, "Add", typeof(global::System.Int32), [typeof(global::System.Int32), typeof(global::System.Int32)]), new XmlComment(@"Adds two integers and returns the result.", null, null, @"The sum of two integers.", null, false, [@" ```int c = Math.Add(4, 5);
277278
if (c &gt; 10)
278279
{

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,15 @@ private static Dictionary<MemberKey, XmlComment> GenerateCacheEntries()
198198
""email"": ""[email protected]""
199199
}", null, null, false, null, [new XmlParameterComment(@"user", @"The user information.", @"{""username"": ""johndoe"", ""email"": ""[email protected]""}", false)], [new XmlResponseComment(@"201", @"Successfully created the user.", @""), new XmlResponseComment(@"400", @"If the user data is invalid.", @"")]));
200200
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Put7", typeof(global::Microsoft.AspNetCore.Http.IResult), [typeof(global::System.Int32?), typeof(global::System.String)]), new XmlComment(@"Updates an existing record.", null, null, null, null, false, null, [new XmlParameterComment(@"id", @"Legacy ID parameter - use uuid instead.", null, true), new XmlParameterComment(@"uuid", @"Unique identifier for the record.", null, false)], [new XmlResponseComment(@"204", @"Update successful.", @""), new XmlResponseComment(@"404", @"Legacy response - will be removed.", @"")]));
201+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get8", typeof(global::System.Threading.Tasks.Task), []), new XmlComment(@"A summary of Get8.", null, null, null, null, false, null, null, null));
202+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get9", typeof(global::System.Threading.Tasks.ValueTask), []), new XmlComment(@"A summary of Get9.", null, null, null, null, false, null, null, null));
203+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get10", typeof(global::System.Threading.Tasks.Task), []), new XmlComment(@"A summary of Get10.", null, null, null, null, false, null, null, null));
204+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get11", typeof(global::System.Threading.Tasks.ValueTask), []), new XmlComment(@"A summary of Get11.", null, null, null, null, false, null, null, null));
205+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get12", typeof(global::System.Threading.Tasks.Task<global::System.String>), []), new XmlComment(@"A summary of Get12.", null, null, null, null, false, null, null, null));
206+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get13", typeof(global::System.Threading.Tasks.ValueTask<global::System.String>), []), new XmlComment(@"A summary of Get13.", null, null, null, null, false, null, null, null));
207+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get14", typeof(global::System.Threading.Tasks.Task<global::Holder<global::System.String>>), []), new XmlComment(@"A summary of Get14.", null, null, null, null, false, null, null, null));
208+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Get15", typeof(global::System.Threading.Tasks.Task<global::Holder<global::System.String>>), []), new XmlComment(@"A summary of Get15.", null, null, null, null, false, null, null, null));
209+
_cache.Add(new MemberKey(typeof(global::RouteHandlerExtensionMethods), MemberType.Method, "Post16", typeof(void), [typeof(global::Example)]), new XmlComment(@"A summary of Post16.", null, null, null, null, false, null, null, null));
201210

202211
return _cache;
203212
}

0 commit comments

Comments
 (0)