Skip to content

Commit 9941a37

Browse files
captainsafiacaptainsafia
authored
Standardize pattern used for property comments (#63224)
* Standardize pattern used for property comments * Use Environment.Newline
1 parent a7e8fa4 commit 9941a37

16 files changed

+340
-44
lines changed

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -452,11 +452,20 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
452452
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyDocId), out var propertyComment))
453453
{
454454
var parameter = operation.Parameters?.SingleOrDefault(p => p.Name == metadata.Name);
455+
var description = propertyComment.Summary;
456+
if (!string.IsNullOrEmpty(description) && !string.IsNullOrEmpty(propertyComment.Value))
457+
{
458+
description = $"{description}\n{propertyComment.Value}";
459+
}
460+
else if (string.IsNullOrEmpty(description))
461+
{
462+
description = propertyComment.Value;
463+
}
455464
if (parameter is null)
456465
{
457466
if (operation.RequestBody is not null)
458467
{
459-
operation.RequestBody.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
468+
operation.RequestBody.Description = description;
460469
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
461470
{
462471
var content = operation.RequestBody.Content?.Values;
@@ -476,7 +485,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
476485
var targetOperationParameter = UnwrapOpenApiParameter(parameter);
477486
if (targetOperationParameter is not null)
478487
{
479-
targetOperationParameter.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
488+
targetOperationParameter.Description = description;
480489
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
481490
{
482491
targetOperationParameter.Example = jsonString.Parse();
@@ -533,12 +542,21 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
533542
// Apply comments from the property
534543
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyInfo.CreateDocumentationId()), out var propertyComment))
535544
{
545+
var description = propertyComment.Summary;
546+
if (!string.IsNullOrEmpty(description) && !string.IsNullOrEmpty(propertyComment.Value))
547+
{
548+
description = $"{description}\n{propertyComment.Value}";
549+
}
550+
else if (string.IsNullOrEmpty(description))
551+
{
552+
description = propertyComment.Value;
553+
}
536554
if (schema.Metadata is null
537555
|| !schema.Metadata.TryGetValue("x-schema-id", out var schemaId)
538556
|| string.IsNullOrEmpty(schemaId as string))
539557
{
540558
// Inlined schema
541-
schema.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary!;
559+
schema.Description = description;
542560
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
543561
{
544562
schema.Example = jsonString.Parse();
@@ -547,7 +565,10 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
547565
else
548566
{
549567
// Schema Reference
550-
schema.Metadata["x-ref-description"] = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary!;
568+
if (!string.IsNullOrEmpty(description))
569+
{
570+
schema.Metadata["x-ref-description"] = description;
571+
}
551572
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
552573
{
553574
schema.Metadata["x-ref-example"] = jsonString.Parse()!;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, additionalAssemblies, docume
215215
todo = path.RequestBody.Content["application/json"].Schema;
216216
Assert.Equal("The identifier of the todo.", todo.Properties["id"].Description);
217217
Assert.Equal("The name of the todo.", todo.Properties["name"].Description);
218-
Assert.Equal("Another description of the todo.", todo.Properties["description"].Description);
218+
Assert.Equal($"A description of the todo.\nAnother description of the todo.", todo.Properties["description"].Description);
219219

220220
path = document.Paths["/type-with-examples"].Operations[HttpMethod.Post];
221221
var typeWithExamples = path.RequestBody.Content["application/json"].Schema;

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public async Task SupportsAllXmlTagsOnSchemas()
3737
app.MapPost("/generic-class", (GenericClass<string> generic) => { });
3838
app.MapPost("/generic-parent", (GenericParent parent) => { });
3939
app.MapPost("/params-and-param-refs", (ParamsAndParamRefs refs) => { });
40+
app.MapPost("/xml-property-test", (XmlPropertyTestClass test) => { });
4041
4142
4243
app.Run();
@@ -471,6 +472,37 @@ protected virtual void Dispose(bool disposing)
471472
// No-op
472473
}
473474
}
475+
476+
/// <summary>
477+
/// This class tests different XML comment scenarios for properties.
478+
/// </summary>
479+
public class XmlPropertyTestClass
480+
{
481+
/// <summary>
482+
/// A property with only summary tag.
483+
/// </summary>
484+
public string SummaryOnly { get; set; }
485+
486+
/// <value>
487+
/// A property with only value tag.
488+
/// </value>
489+
public string ValueOnly { get; set; }
490+
491+
/// <summary>
492+
/// A property with both summary and value.
493+
/// </summary>
494+
/// <value>Additional value information.</value>
495+
public string BothSummaryAndValue { get; set; }
496+
497+
/// <returns>This should be ignored for properties.</returns>
498+
public string ReturnsOnly { get; set; }
499+
500+
/// <summary>
501+
/// A property with summary and returns.
502+
/// </summary>
503+
/// <returns>This should be ignored for properties.</returns>
504+
public string SummaryAndReturns { get; set; }
505+
}
474506
""";
475507
var generator = new XmlCommentGenerator();
476508
await SnapshotTestHelper.Verify(source, generator, out var compilation);
@@ -479,6 +511,7 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
479511
var path = document.Paths["/example-class"].Operations[HttpMethod.Post];
480512
var exampleClass = path.RequestBody.Content["application/json"].Schema;
481513
Assert.Equal("Every class and member should have a one sentence\nsummary describing its purpose.", exampleClass.Description, ignoreLineEndingDifferences: true);
514+
// Label property has only <value> tag -> uses value directly
482515
Assert.Equal("The `Label` property represents a label\nfor this instance.", exampleClass.Properties["label"].Description, ignoreLineEndingDifferences: true);
483516

484517
path = document.Paths["/person"].Operations[HttpMethod.Post];
@@ -523,6 +556,26 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
523556
path = document.Paths["/params-and-param-refs"].Operations[HttpMethod.Post];
524557
var paramsAndParamRefs = path.RequestBody.Content["application/json"].Schema;
525558
Assert.Equal("This shows examples of typeparamref and typeparam tags", paramsAndParamRefs.Description);
559+
560+
// Test new XML property documentation behavior
561+
path = document.Paths["/xml-property-test"].Operations[HttpMethod.Post];
562+
var xmlPropertyTest = path.RequestBody.Content["application/json"].Schema;
563+
Assert.Equal("This class tests different XML comment scenarios for properties.", xmlPropertyTest.Description);
564+
565+
// Property with only <summary> -> uses summary directly
566+
Assert.Equal("A property with only summary tag.", xmlPropertyTest.Properties["summaryOnly"].Description);
567+
568+
// Property with only <value> -> uses value directly
569+
Assert.Equal("A property with only value tag.", xmlPropertyTest.Properties["valueOnly"].Description);
570+
571+
// Property with both <summary> and <value> -> combines with newline separator
572+
Assert.Equal($"A property with both summary and value.\nAdditional value information.", xmlPropertyTest.Properties["bothSummaryAndValue"].Description);
573+
574+
// Property with only <returns> -> should be null (ignored)
575+
Assert.Null(xmlPropertyTest.Properties["returnsOnly"].Description);
576+
577+
// Property with <summary> and <returns> -> uses summary only, ignores returns
578+
Assert.Equal("A property with summary and returns.", xmlPropertyTest.Properties["summaryAndReturns"].Description);
526579
});
527580
}
528581
}

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

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public async Task SupportsXmlCommentsOnOperationsFromMinimalApis()
4949
app.MapPost("/19", RouteHandlerExtensionMethods.Post19);
5050
app.MapGet("/20", RouteHandlerExtensionMethods.Get20);
5151
app.MapGet("/21", RouteHandlerExtensionMethods.Get21);
52+
app.MapGet("/22", RouteHandlerExtensionMethods.Get22);
5253
5354
app.Run();
5455
@@ -249,6 +250,15 @@ public static IResult Get21([AsParameters] XmlDocPriorityParametersClass priorit
249250
{
250251
return TypedResults.Ok($"Processed parameters");
251252
}
253+
254+
/// <summary>
255+
/// Tests summary and value documentation priority on AsParameters properties.
256+
/// </summary>
257+
/// <param name="summaryValueParams">Parameters testing summary vs value priority.</param>
258+
public static IResult Get22([AsParameters] SummaryValueParametersClass summaryValueParams)
259+
{
260+
return TypedResults.Ok($"Summary: {summaryValueParams.SummaryProperty}, Value: {summaryValueParams.ValueProperty}");
261+
}
252262
}
253263
254264
public class FirstParameters
@@ -371,6 +381,23 @@ public class XmlDocPriorityParametersClass
371381
/// <value>Value-only description.</value>
372382
public string? ValueOnlyProperty { get; set; }
373383
}
384+
385+
public class SummaryValueParametersClass
386+
{
387+
/// <summary>
388+
/// Property with only summary documentation.
389+
/// </summary>
390+
public string? SummaryProperty { get; set; }
391+
392+
/// <summary>
393+
/// Property with summary that should be overridden by value.
394+
/// </summary>
395+
/// <value>Value description that should take precedence over summary.</value>
396+
public string? ValueProperty { get; set; }
397+
398+
/// <value>Property with only value documentation.</value>
399+
public string? ValueOnlyProperty { get; set; }
400+
}
374401
""";
375402
var generator = new XmlCommentGenerator();
376403
await SnapshotTestHelper.Verify(source, generator, out var compilation);
@@ -478,16 +505,29 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
478505
Assert.Equal("Property with only summary documentation.", summaryOnlyParam.Description);
479506

480507
var summaryAndReturnsParam = path22.Parameters.First(p => p.Name == "SummaryAndReturnsProperty");
481-
Assert.Equal("Returns-based description that should take precedence over summary.", summaryAndReturnsParam.Description);
508+
Assert.Equal("Property with summary documentation that should be overridden.", summaryAndReturnsParam.Description);
482509

483510
var allThreeParam = path22.Parameters.First(p => p.Name == "AllThreeProperty");
484-
Assert.Equal("Value-based description that should take highest precedence.", allThreeParam.Description);
511+
Assert.Equal($"Property with all three types of documentation.\nValue-based description that should take highest precedence.", allThreeParam.Description);
485512

486513
var returnsOnlyParam = path22.Parameters.First(p => p.Name == "ReturnsOnlyProperty");
487-
Assert.Equal("Returns-only description.", returnsOnlyParam.Description);
514+
Assert.Null(returnsOnlyParam.Description);
488515

489516
var valueOnlyParam = path22.Parameters.First(p => p.Name == "ValueOnlyProperty");
490517
Assert.Equal("Value-only description.", valueOnlyParam.Description);
518+
519+
// Test summary and value documentation priority for AsParameters
520+
var path23 = document.Paths["/22"].Operations[HttpMethod.Get];
521+
Assert.Equal("Tests summary and value documentation priority on AsParameters properties.", path23.Summary);
522+
523+
var summaryParam = path23.Parameters.First(p => p.Name == "SummaryProperty");
524+
Assert.Equal("Property with only summary documentation.", summaryParam.Description);
525+
526+
var valueParam = path23.Parameters.First(p => p.Name == "ValueProperty");
527+
Assert.Equal($"Property with summary that should be overridden by value.\nValue description that should take precedence over summary.", valueParam.Description);
528+
529+
var valueOnlyParam2 = path23.Parameters.First(p => p.Name == "ValueOnlyProperty");
530+
Assert.Equal("Property with only value documentation.", valueOnlyParam2.Description);
491531
});
492532
}
493533
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,13 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
206206

207207
path = document.Paths["/todo-with-description"].Operations[HttpMethod.Post];
208208
todo = path.RequestBody.Content["application/json"].Schema;
209+
// Test different XML comment scenarios for properties:
210+
// Id: only <summary> tag -> uses summary directly
209211
Assert.Equal("The identifier of the todo.", todo.Properties["id"].Description);
212+
// Name: only <value> tag -> uses value directly
210213
Assert.Equal("The name of the todo.", todo.Properties["name"].Description);
211-
Assert.Equal("Another description of the todo.", todo.Properties["description"].Description);
214+
// Description: both <summary> and <value> tags -> combines with newline separator
215+
Assert.Equal($"A description of the the todo.\nAnother description of the todo.", todo.Properties["description"].Description);
212216

213217
path = document.Paths["/type-with-examples"].Operations[HttpMethod.Post];
214218
var typeWithExamples = path.RequestBody.Content["application/json"].Schema;

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,11 +434,20 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
434434
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyDocId), out var propertyComment))
435435
{
436436
var parameter = operation.Parameters?.SingleOrDefault(p => p.Name == metadata.Name);
437+
var description = propertyComment.Summary;
438+
if (!string.IsNullOrEmpty(description) && !string.IsNullOrEmpty(propertyComment.Value))
439+
{
440+
description = $"{description}\n{propertyComment.Value}";
441+
}
442+
else if (string.IsNullOrEmpty(description))
443+
{
444+
description = propertyComment.Value;
445+
}
437446
if (parameter is null)
438447
{
439448
if (operation.RequestBody is not null)
440449
{
441-
operation.RequestBody.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
450+
operation.RequestBody.Description = description;
442451
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
443452
{
444453
var content = operation.RequestBody.Content?.Values;
@@ -458,7 +467,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
458467
var targetOperationParameter = UnwrapOpenApiParameter(parameter);
459468
if (targetOperationParameter is not null)
460469
{
461-
targetOperationParameter.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary;
470+
targetOperationParameter.Description = description;
462471
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
463472
{
464473
targetOperationParameter.Example = jsonString.Parse();
@@ -515,12 +524,21 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
515524
// Apply comments from the property
516525
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyInfo.CreateDocumentationId()), out var propertyComment))
517526
{
527+
var description = propertyComment.Summary;
528+
if (!string.IsNullOrEmpty(description) && !string.IsNullOrEmpty(propertyComment.Value))
529+
{
530+
description = $"{description}\n{propertyComment.Value}";
531+
}
532+
else if (string.IsNullOrEmpty(description))
533+
{
534+
description = propertyComment.Value;
535+
}
518536
if (schema.Metadata is null
519537
|| !schema.Metadata.TryGetValue("x-schema-id", out var schemaId)
520538
|| string.IsNullOrEmpty(schemaId as string))
521539
{
522540
// Inlined schema
523-
schema.Description = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary!;
541+
schema.Description = description;
524542
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
525543
{
526544
schema.Example = jsonString.Parse();
@@ -529,7 +547,10 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
529547
else
530548
{
531549
// Schema Reference
532-
schema.Metadata["x-ref-description"] = propertyComment.Value ?? propertyComment.Returns ?? propertyComment.Summary!;
550+
if (!string.IsNullOrEmpty(description))
551+
{
552+
schema.Metadata["x-ref-description"] = description;
553+
}
533554
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
534555
{
535556
schema.Metadata["x-ref-example"] = jsonString.Parse()!;

0 commit comments

Comments
 (0)