Skip to content

Code Review Bench PR #64401 - [release/10.0] Fix ModelMetadata null reference exception in emitted XMLComment transformers #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ketkarameya wants to merge 5 commits into
base: base_pr_64401_20260125_1476
Choose a base branch
from
Open

Code Review Bench PR #64401 - [release/10.0] Fix ModelMetadata null reference exception in emitted XMLComment transformers #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fef8c44
OpenAPI: Fix ModelMetadata null reference exception in emitted XMLCom…
desjoerd Sep 29, 2025
c1441a7
Change snapshots to utf-8 with bom
desjoerd Sep 30, 2025
11f5b24
Rename controller test to allow adding it for minimal api's as well
desjoerd Sep 30, 2025
bbc5308
Fix whitespace difference in OpenApi XML Source generator snapshot
desjoerd Oct 16, 2025
45b170b
update pr
tomerqodo Jan 25, 2026
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
12 changes: 5 additions & 7 deletions src/OpenApi/gen/XmlCommentGenerator.Emitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
Expand Down Expand Up @@ -382,7 +379,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
}
if (methodComment.Remarks is { } remarks)
{
operation.Description = remarks;
operation.Summary = remarks;
}
Comment on lines 380 to 383
Copy link

@gitar-bot-staging gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Remarks now overwrites Summary instead of setting Description

The change from operation.Description = remarks to operation.Summary = remarks (line 382) means that when both <summary> and <remarks> XML tags are present on a method, the remarks text will overwrite the summary that was just set on line 374.

Previously:

  • <summary> → operation.Summary
  • <remarks> → operation.Description

Now:

  • <summary> → operation.Summary
  • <remarks> → operation.Summary (overwrites the summary!)

This seems semantically incorrect. If both tags are provided, the summary is silently lost. The original behavior of mapping remarks to operation.Description makes more sense in the OpenAPI context, where Summary and Description are distinct fields.

Additionally, this change is NOT reflected in any of the 9 snapshot files (they all still have operation.Description = remarks), so the snapshot tests will fail when regenerated.

Was this helpful? React with 👍 / 👎

Suggested change
if (methodComment.Remarks is { } remarks)
{
operation.Description = remarks;
operation.Summary = remarks;
}
if (methodComment.Remarks is { } remarks)
{
operation.Description = remarks;
}
  • Apply suggested fix

if (methodComment.Parameters is { Count: > 0})
{
Expand Down Expand Up @@ -444,14 +441,15 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
foreach (var parameterDescription in context.Description.ParameterDescriptions)
{
var metadata = parameterDescription.ModelMetadata;
if (metadata.MetadataKind == ModelMetadataKind.Property
if (metadata is not null
&& metadata.MetadataKind == ModelMetadataKind.Property
&& metadata.ContainerType is { } containerType
&& metadata.PropertyName is { } propertyName)
{
var propertyDocId = DocumentationCommentIdHelper.CreateDocumentationId(containerType, propertyName);
if (XmlCommentCache.Cache.TryGetValue(DocumentationCommentIdHelper.NormalizeDocId(propertyDocId), out var propertyComment))
{
var parameter = operation.Parameters?.SingleOrDefault(p => p.Name == metadata.Name);
var parameter = operation.Parameters?.SingleOrDefault(p => p.Name == propertyName);
Copy link

@gitar-bot-staging gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Snapshot files not updated for propertyName and other changes

The emitter template has three changes beyond the null-check fix, but the snapshot verification files were only updated for the null-check. The other changes will cause snapshot test failures:

  1. p.Name == metadata.Name → p.Name == propertyName (line 452): All 9 snapshot files still use metadata.Name.
  2. operation.Description = remarks → operation.Summary = remarks (line 382): All 9 snapshot files still use Description.
  3. operation.RequestBody.Description = description moved after examples block (line 479): All 9 snapshot files still have the old ordering with Description before the examples check.

Since these are verified snapshot files (used by the test framework to verify source generator output), having them out of sync with the actual emitter template means the tests will fail. Either the snapshots need to be regenerated, or these emitter changes should be reverted if they were unintended.

Was this helpful? React with 👍 / 👎

  • Apply suggested fix

var description = propertyComment.Summary;
if (!string.IsNullOrEmpty(description) && !string.IsNullOrEmpty(propertyComment.Value))
{
Expand All @@ -465,7 +463,6 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
{
if (operation.RequestBody is not null)
{
operation.RequestBody.Description = description;
if (propertyComment.Examples?.FirstOrDefault() is { } jsonString)
{
var content = operation.RequestBody.Content?.Values;
Expand All @@ -479,6 +476,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
mediaType.Example = parsedExample;
}
}
operation.RequestBody.Description = description;
}
continue;
}
Expand Down
45 changes: 45 additions & 0 deletions ...pi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/OperationTests.Controllers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,49 @@ await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
Assert.Equal("The todo to insert into the database.", path3.RequestBody.Description);
});
}

[Fact]
public async Task SupportsRouteParametersFromControllers()
{
var source = """
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;

var builder = WebApplication.CreateBuilder();

builder.Services
.AddControllers()
.AddApplicationPart(typeof(TestController).Assembly);
builder.Services.AddOpenApi();

var app = builder.Build();

app.MapControllers();

app.Run();

[ApiController]
[Route("[controller]")]
public class TestController : ControllerBase
{
/// <param name="userId">The id of the user.</param>
[HttpGet("{userId}")]
public string Get()
{
return "Hello, World!";
}
}

public partial class Program {}

""";
var generator = new XmlCommentGenerator();
await SnapshotTestHelper.Verify(source, generator, out var compilation);
await SnapshotTestHelper.VerifyOpenApi(compilation, document =>
{
var path = document.Paths["/Test/{userId}"].Operations[HttpMethod.Get];
Assert.Equal("The id of the user.", path.Parameters[0].Description);
});
}
}
3 changes: 2 additions & 1 deletion ...ots/AddOpenApiTests.CanInterceptAddOpenApi#OpenApiXmlCommentSupport.generated.verified.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
foreach (var parameterDescription in context.Description.ParameterDescriptions)
{
var metadata = parameterDescription.ModelMetadata;
if (metadata.MetadataKind == ModelMetadataKind.Property
if (metadata is not null
&& metadata.MetadataKind == ModelMetadataKind.Property
&& metadata.ContainerType is { } containerType
&& metadata.PropertyName is { } propertyName)
{
Expand Down
3 changes: 2 additions & 1 deletion ...ts.CanHandleXmlForSchemasInAdditionalTexts#OpenApiXmlCommentSupport.generated.verified.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
foreach (var parameterDescription in context.Description.ParameterDescriptions)
{
var metadata = parameterDescription.ModelMetadata;
if (metadata.MetadataKind == ModelMetadataKind.Property
if (metadata is not null
&& metadata.MetadataKind == ModelMetadataKind.Property
&& metadata.ContainerType is { } containerType
&& metadata.PropertyName is { } propertyName)
{
Expand Down
3 changes: 2 additions & 1 deletion ...pletenessTests.SupportsAllXmlTagsOnSchemas#OpenApiXmlCommentSupport.generated.verified.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,8 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
foreach (var parameterDescription in context.Description.ParameterDescriptions)
{
var metadata = parameterDescription.ModelMetadata;
if (metadata.MetadataKind == ModelMetadataKind.Property
if (metadata is not null
&& metadata.MetadataKind == ModelMetadataKind.Property
&& metadata.ContainerType is { } containerType
&& metadata.PropertyName is { } propertyName)
{
Expand Down
Loading