Skip to content

Commit 73655ef

Browse files
captainsafiacaptainsafia
authored
Fix handling for more method signature types (#60464)
* Fix handling for more method signature types * Address feedback and add more tests * Increase default timeout on build tests
1 parent 30f9f49 commit 73655ef

10 files changed

+751
-19
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public sealed partial class XmlCommentGenerator
9898
{
9999
var memberKey = symbol switch
100100
{
101-
IMethodSymbol methodSymbol => MemberKey.FromMethodSymbol(methodSymbol),
101+
IMethodSymbol methodSymbol => MemberKey.FromMethodSymbol(methodSymbol, input.Compilation),
102102
IPropertySymbol propertySymbol => MemberKey.FromPropertySymbol(propertySymbol),
103103
INamedTypeSymbol typeSymbol => MemberKey.FromTypeSymbol(typeSymbol),
104104
_ => null

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

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

24-
public static MemberKey FromMethodSymbol(IMethodSymbol method)
24+
public static MemberKey FromMethodSymbol(IMethodSymbol method, Compilation compilation)
2525
{
26+
string returnType;
27+
if (method.ReturnsVoid)
28+
{
29+
returnType = "typeof(void)";
30+
}
31+
else
32+
{
33+
// Handle Task/ValueTask for async methods
34+
var actualReturnType = method.ReturnType;
35+
if (method.IsAsync && actualReturnType is INamedTypeSymbol namedType)
36+
{
37+
if (namedType.TypeArguments.Length > 0)
38+
{
39+
actualReturnType = namedType.TypeArguments[0];
40+
}
41+
else
42+
{
43+
actualReturnType = compilation.GetSpecialType(SpecialType.System_Void);
44+
}
45+
}
46+
47+
returnType = actualReturnType.TypeKind == TypeKind.TypeParameter
48+
? "typeof(object)"
49+
: $"typeof({actualReturnType.ToDisplayString(_typeKeyFormat)})";
50+
}
51+
52+
// Handle extension methods by skipping the 'this' parameter
53+
var parameters = method.Parameters
54+
.Where(p => !p.IsThis)
55+
.Select(p =>
56+
{
57+
if (p.Type.TypeKind == TypeKind.TypeParameter)
58+
{
59+
return "typeof(object)";
60+
}
61+
62+
// For params arrays, use the array type
63+
if (p.IsParams && p.Type is IArrayTypeSymbol arrayType)
64+
{
65+
return $"typeof({arrayType.ToDisplayString(_typeKeyFormat)})";
66+
}
67+
68+
return $"typeof({p.Type.ToDisplayString(_typeKeyFormat)})";
69+
})
70+
.ToArray();
71+
72+
// For generic methods, use the containing type with generic parameters
73+
var declaringType = method.ContainingType;
74+
var typeDisplay = declaringType.ToDisplayString(_typeKeyFormat);
75+
76+
// If the method is in a generic type, we need to handle the type parameters
77+
if (declaringType.IsGenericType)
78+
{
79+
typeDisplay = ReplaceGenericArguments(typeDisplay);
80+
}
81+
2682
return new MemberKey(
27-
$"typeof({ReplaceGenericArguments(method.ContainingType.ToDisplayString(_typeKeyFormat))})",
83+
$"typeof({typeDisplay})",
2884
MemberType.Method,
29-
method.MetadataName,
30-
method.ReturnType.TypeKind == TypeKind.TypeParameter
31-
? "typeof(object)"
32-
: $"typeof({method.ReturnType.ToDisplayString(_typeKeyFormat)})",
33-
[.. method.Parameters.Select(p =>
34-
p.Type.TypeKind == TypeKind.TypeParameter
35-
? "typeof(object)"
36-
: $"typeof({p.Type.ToDisplayString(_typeKeyFormat)})")]);
85+
method.MetadataName, // Use MetadataName to match runtime MethodInfo.Name
86+
returnType,
87+
parameters);
3788
}
3889

3990
public static MemberKey FromPropertySymbol(IPropertySymbol property)

‎src/OpenApi/gen/XmlComments/XmlComment.InheritDoc.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
namespace Microsoft.AspNetCore.OpenApi.SourceGenerators.Xml;
1717

1818
/// <remarks>
19-
/// Source code in this class is derived from Roslyn's Xml documentation comment processing
19+
/// Source code in this class is derived from Roslyn's XML documentation comment processing
2020
/// to support the full resolution of <inheritdoc /> tags.
2121
/// For the original code, see https://github.com/dotnet/roslyn/blob/ef1d7fe925c94e96a93e4c9af50983e0f675a9fd/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs.
2222
/// </remarks>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ private static void ResolveCrefLink(Compilation compilation, XNode node, string
191191
var symbol = DocumentationCommentId.GetFirstSymbolForDeclarationId(cref, compilation);
192192
if (symbol is not null)
193193
{
194-
var type = symbol.ToDisplayString();
194+
var type = symbol.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);
195195
item.ReplaceWith(new XText(type));
196196
}
197197
}

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Build.Tests/GenerateAdditionalXmlFilesForOpenApiTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.OpenApi.Build.Tests;
99

1010
public class GenerateAdditionalXmlFilesForOpenApiTests
1111
{
12-
private static readonly TimeSpan _defaultProcessTimeout = TimeSpan.FromSeconds(45);
12+
private static readonly TimeSpan _defaultProcessTimeout = TimeSpan.FromSeconds(120);
1313

1414
[Fact]
1515
public void VerifiesTargetGeneratesXmlFiles()

‎src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/AdditionalTextsTests.Schemas.cs

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
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.Text.Json.Nodes;
5+
using Microsoft.OpenApi.Models;
6+
7+
namespace Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests;
8+
9+
[UsesVerify]
10+
public partial class AdditionalTextsTests
11+
{
12+
[Fact]
13+
public async Task CanHandleXmlForSchemasInAdditionalTexts()
14+
{
15+
var source = """
16+
using System;
17+
using Microsoft.AspNetCore.Builder;
18+
using Microsoft.Extensions.DependencyInjection;
19+
using ClassLibrary;
20+
21+
var builder = WebApplication.CreateBuilder();
22+
23+
builder.Services.AddOpenApi();
24+
25+
var app = builder.Build();
26+
27+
app.MapPost("/todo", (Todo todo) => { });
28+
app.MapPost("/project", (Project project) => { });
29+
app.MapPost("/board", (ProjectBoard.BoardItem boardItem) => { });
30+
app.MapPost("/project-record", (ProjectRecord project) => { });
31+
app.MapPost("/todo-with-description", (TodoWithDescription todo) => { });
32+
app.MapPost("/type-with-examples", (TypeWithExamples typeWithExamples) => { });
33+
34+
app.Run();
35+
""";
36+
37+
var librarySource = """
38+
using System;
39+
40+
namespace ClassLibrary;
41+
42+
/// <summary>
43+
/// This is a todo item.
44+
/// </summary>
45+
public class Todo
46+
{
47+
public int Id { get; set; }
48+
public string Name { get; set; }
49+
public string Description { get; set; }
50+
}
51+
52+
/// <summary>
53+
/// The project that contains <see cref="Todo"/> items.
54+
/// </summary>
55+
public record Project(string Name, string Description);
56+
57+
public class ProjectBoard
58+
{
59+
/// <summary>
60+
/// An item on the board.
61+
/// </summary>
62+
public class BoardItem
63+
{
64+
public string Name { get; set; }
65+
}
66+
}
67+
68+
/// <summary>
69+
/// The project that contains <see cref="Todo"/> items.
70+
/// </summary>
71+
/// <param name="Name">The name of the project.</param>
72+
/// <param name="Description">The description of the project.</param>
73+
public record ProjectRecord(string Name, string Description);
74+
75+
public class TodoWithDescription
76+
{
77+
/// <summary>
78+
/// The identifier of the todo.
79+
/// </summary>
80+
public int Id { get; set; }
81+
/// <value>
82+
/// The name of the todo.
83+
/// </value>
84+
public string Name { get; set; }
85+
/// <summary>
86+
/// A description of the todo.
87+
/// </summary>
88+
/// <value>Another description of the todo.</value>
89+
public string Description { get; set; }
90+
}
91+
92+
public class TypeWithExamples
93+
{
94+
/// <example>true</example>
95+
public bool BooleanType { get; set; }
96+
/// <example>42</example>
97+
public int IntegerType { get; set; }
98+
/// <example>1234567890123456789</example>
99+
public long LongType { get; set; }
100+
/// <example>3.14</example>
101+
public double DoubleType { get; set; }
102+
/// <example>3.14</example>
103+
public float FloatType { get; set; }
104+
/// <example>2022-01-01T00:00:00Z</example>
105+
public DateTime DateTimeType { get; set; }
106+
/// <example>2022-01-01</example>
107+
public DateOnly DateOnlyType { get; set; }
108+
/// <example>Hello, World!</example>
109+
public string StringType { get; set; }
110+
/// <example>2d8f1eac-b5c6-4e29-8c62-4d9d75ef3d3d</example>
111+
public Guid GuidType { get; set; }
112+
/// <example>12:30:45</example>
113+
public TimeOnly TimeOnlyType { get; set; }
114+
/// <example>P3DT4H5M</example>
115+
public TimeSpan TimeSpanType { get; set; }
116+
/// <example>255</example>
117+
public byte ByteType { get; set; }
118+
/// <example>3.14159265359</example>
119+
public decimal DecimalType { get; set; }
120+
/// <example>https://example.com</example>
121+
public Uri UriType { get; set; }
122+
}
123+
""";
124+
var references = new Dictionary<string, List<string>>
125+
{
126+
{ "ClassLibrary", [librarySource] }
127+
};
128+
129+
var generator = new XmlCommentGenerator();
130+
await SnapshotTestHelper.Verify(source, generator, references, out var compilation, out var additionalAssemblies);
131+
await SnapshotTestHelper.VerifyOpenApi(compilation, additionalAssemblies, document =>
132+
{
133+
var path = document.Paths["/todo"].Operations[OperationType.Post];
134+
var todo = path.RequestBody.Content["application/json"].Schema;
135+
Assert.Equal("This is a todo item.", todo.Description);
136+
137+
path = document.Paths["/project"].Operations[OperationType.Post];
138+
var project = path.RequestBody.Content["application/json"].Schema;
139+
Assert.Equal("The project that contains Todo items.", project.Description);
140+
141+
path = document.Paths["/board"].Operations[OperationType.Post];
142+
var board = path.RequestBody.Content["application/json"].Schema;
143+
Assert.Equal("An item on the board.", board.Description);
144+
145+
path = document.Paths["/project-record"].Operations[OperationType.Post];
146+
project = path.RequestBody.Content["application/json"].Schema;
147+
148+
Assert.Equal("The name of the project.", project.Properties["name"].Description);
149+
Assert.Equal("The description of the project.", project.Properties["description"].Description);
150+
151+
path = document.Paths["/todo-with-description"].Operations[OperationType.Post];
152+
todo = path.RequestBody.Content["application/json"].Schema;
153+
Assert.Equal("The identifier of the todo.", todo.Properties["id"].Description);
154+
Assert.Equal("The name of the todo.", todo.Properties["name"].Description);
155+
Assert.Equal("Another description of the todo.", todo.Properties["description"].Description);
156+
157+
path = document.Paths["/type-with-examples"].Operations[OperationType.Post];
158+
var typeWithExamples = path.RequestBody.Content["application/json"].Schema;
159+
160+
var booleanTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["booleanType"].Example);
161+
Assert.True(booleanTypeExample.GetValue<bool>());
162+
163+
var integerTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["integerType"].Example);
164+
Assert.Equal(42, integerTypeExample.GetValue<int>());
165+
166+
var longTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["longType"].Example);
167+
Assert.Equal(1234567890123456789, longTypeExample.GetValue<long>());
168+
169+
// Broken due to https://github.com/microsoft/OpenAPI.NET/issues/2137
170+
// var doubleTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["doubleType"].Example);
171+
// Assert.Equal("3.14", doubleTypeExample.GetValue<string>());
172+
173+
// var floatTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["floatType"].Example);
174+
// Assert.Equal(3.14f, floatTypeExample.GetValue<float>());
175+
176+
// var dateTimeTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["dateTimeType"].Example);
177+
// Assert.Equal(DateTime.Parse("2022-01-01T00:00:00Z", CultureInfo.InvariantCulture), dateTimeTypeExample.GetValue<DateTime>());
178+
179+
// var dateOnlyTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["dateOnlyType"].Example);
180+
// Assert.Equal(DateOnly.Parse("2022-01-01", CultureInfo.InvariantCulture), dateOnlyTypeExample.GetValue<DateOnly>());
181+
182+
var stringTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["stringType"].Example);
183+
Assert.Equal("Hello, World!", stringTypeExample.GetValue<string>());
184+
185+
var guidTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["guidType"].Example);
186+
Assert.Equal("2d8f1eac-b5c6-4e29-8c62-4d9d75ef3d3d", guidTypeExample.GetValue<string>());
187+
188+
var byteTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["byteType"].Example);
189+
Assert.Equal(255, byteTypeExample.GetValue<int>());
190+
191+
// Broken due to https://github.com/microsoft/OpenAPI.NET/issues/2137
192+
// var timeOnlyTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["timeOnlyType"].Example);
193+
// Assert.Equal(TimeOnly.Parse("12:30:45", CultureInfo.InvariantCulture), timeOnlyTypeExample.GetValue<TimeOnly>());
194+
195+
// var timeSpanTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["timeSpanType"].Example);
196+
// Assert.Equal(TimeSpan.Parse("P3DT4H5M", CultureInfo.InvariantCulture), timeSpanTypeExample.GetValue<TimeSpan>());
197+
198+
// var decimalTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["decimalType"].Example);
199+
// Assert.Equal(3.14159265359m, decimalTypeExample.GetValue<decimal>());
200+
201+
var uriTypeExample = Assert.IsAssignableFrom<JsonNode>(typeWithExamples.Properties["uriType"].Example);
202+
Assert.Equal("https://example.com", uriTypeExample.GetValue<string>());
203+
});
204+
}
205+
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public async Task SupportsAllXmlTagsOnSchemas()
1717
{
1818
var source = """
1919
using System;
20+
using System.Threading.Tasks;
2021
using Microsoft.AspNetCore.Builder;
2122
using Microsoft.Extensions.DependencyInjection;
2223
@@ -180,6 +181,38 @@ public static int Add(int left, int right)
180181
181182
return left + right;
182183
}
184+
185+
/// <summary>
186+
/// This method is an example of a method that
187+
/// returns an awaitable item.
188+
/// </summary>
189+
public static Task<int> AddAsync(int left, int right)
190+
{
191+
return Task.FromResult(Add(left, right));
192+
}
193+
194+
/// <summary>
195+
/// This method is an example of a method that
196+
/// returns a Task which should map to a void return type.
197+
/// </summary>
198+
public static Task DoNothingAsync()
199+
{
200+
return Task.CompletedTask;
201+
}
202+
203+
/// <summary>
204+
/// This method is an example of a method that consumes
205+
/// an params array.
206+
/// </summary>
207+
public static int AddNumbers(params int[] numbers)
208+
{
209+
var sum = 0;
210+
foreach (var number in numbers)
211+
{
212+
sum += number;
213+
}
214+
return sum;
215+
}
183216
}
184217
185218
/// <summary>

0 commit comments

Comments
 (0)