Fix missing interface XML docs in multi-interface generation#1110
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughXmlDocumentationGenerator now always emits interface documentation using fallback text when descriptions are missing; test build helpers and project templates were added to enable warnings-as-errors compilation checks, and regression tests verify generated summaries and successful compilation.
|
| Layer / File(s) | Summary |
|---|---|
XML documentation fallback summary generation src/Refitter.Core/XmlDocumentationGenerator.cs |
XmlDocumentationGenerator now computes fallback text and emits interface <summary> via a new AppendInterfaceSummary helper: by-tag falls back to Operations for {tag}; by-endpoint prefers Summary then Operations for {operationId} or Operations for endpoint.. |
Test infrastructure for warnings-as-errors builds src/Refitter.Tests/Build/BuildHelper.cs, src/Refitter.Tests/Build/ProjectFileContents.cs |
BuildHelper.BuildCSharp gained overloads accepting warningsAsErrors and routes through a core overload; new .csproj templates Net80AppWithWarningsAsErrors, Net90AppWithWarningsAsErrors, and Net100AppWithWarningsAsErrors set <TreatWarningsAsErrors>true for tests and are selected based on the warningsAsErrors flag. |
Regression tests for fallback summaries and compilation src/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cs, src/Refitter.Tests/XmlDocumentationGeneratorTests.cs |
Adds MissingInterfaceXmlDocsWarningsAsErrorsTests to assert fallback interface summaries for tag- and endpoint-grouped specs and to verify generated code builds with warningsAsErrors: true; updates an existing test to expect the fallback summary when tags are null. |
CI and dependency tweaks .github/workflows/codecov.yml, test/ConsoleApp/Directory.Build.props |
Adjusts Codecov action pin and minor workflow formatting; bumps several Microsoft.Extensions package references in the test ConsoleApp props. |
src/Refitter.Core/XmlDocumentationGenerator.cs to change how interface XML <summary> comments are emitted (including fallback/default behavior).src/Refitter.Core/XmlDocumentationGenerator.cs around interface XML summary generation and related refactors.BuildHelper.BuildCSharp/ProjectFileContents and net10.0 handling.\.NET, bugfix🐰
I hopped through tags and endpoint trails,
Escaped the symbols, filled the missing veils.
When summaries vanished, fallbacks found their place,
Generated code compiled with warnings in embrace.
A tiny rabbit cheers the build's green trace.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately and specifically describes the main change: fixing missing interface XML documentation in multi-interface code generation. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
christianhelle/issue-1109-investigation
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1110 +/- ##
==========================================
+ Coverage 95.32% 95.33% +0.01%
==========================================
Files 28 28
Lines 3013 3021 +8
==========================================
+ Hits 2872 2880 +8
Misses 45 45
Partials 96 96
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cs (1)
70-78: 💤 Low valueConsider a more precise assertion for the endpoint without operationId.
The assertion at Line 77 checks for the prefix
"/// <summary>Operations for", but based on theByEndpointSpecwhere the/ordersendpoint lacks bothoperationIdandsummary, the fallback should be the complete string"Operations for endpoint."Test precision would improve by asserting the exact expected fallback.🔍 Suggested improvement
generatedCode.Should().Contain("/// <summary>Operations for GetUsers.</summary>"); generatedCode.Should().Contain("partial interface IOrdersEndpoint"); - generatedCode.Should().Contain("/// <summary>Operations for"); + generatedCode.Should().Contain("/// <summary>Operations for endpoint.</summary>");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cs` around lines 70 - 78, The test currently asserts a loose prefix ("/// <summary>Operations for") for the endpoint with no operationId/summary; update the assertion to check the exact fallback summary string expected for that endpoint by replacing the loose contains with an assertion that generatedCode contains "/// <summary>Operations for endpoint.</summary>" (locate the assertion in MissingInterfaceXmlDocsWarningsAsErrorsTests.cs near the GenerateCode(ByEndpointSpec, MultipleInterfaces.ByEndpoint) call and update the third Should().Contain to the precise string).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cs`:
- Around line 70-78: The test currently asserts a loose prefix ("///
<summary>Operations for") for the endpoint with no operationId/summary; update
the assertion to check the exact fallback summary string expected for that
endpoint by replacing the loose contains with an assertion that generatedCode
contains "/// <summary>Operations for endpoint.</summary>" (locate the assertion
in MissingInterfaceXmlDocsWarningsAsErrorsTests.cs near the
GenerateCode(ByEndpointSpec, MultipleInterfaces.ByEndpoint) call and update the
third Should().Contain to the precise string).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48943d87-189b-4dc3-847b-97dd4cfbb7ea
📒 Files selected for processing (5)
src/Refitter.Core/XmlDocumentationGenerator.cssrc/Refitter.Tests/Build/BuildHelper.cssrc/Refitter.Tests/Build/ProjectFileContents.cssrc/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cssrc/Refitter.Tests/XmlDocumentationGeneratorTests.cs
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
Contributor
Merge conflicts resolved in commit
|
christianhelle
force-pushed
the
christianhelle/issue-1109-investigation
branch
from
281c8ab to
e036d85
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Refitter.Tests/Build/ProjectFileContents.cs (1)
142-168: net10.0 warnings-as-errors template: package TFMs include net10.0, so mismatch is compatible
The prior concern was thatNet100AppWithWarningsAsErrorsuses a different aligned package set than the net8.0/net9.0 templates. For the key packages checked insrc/Refitter.Tests/Build/ProjectFileContents.cs(lines 142-168), the NuGet packages includelib/net10.0/*:
Refit.HttpClientFactory11.0.1:lib/net10.0/Refit.HttpClientFactory.dllMicrosoft.Extensions.DependencyInjection10.0.8:lib/net10.0/Microsoft.Extensions.DependencyInjection.dll
So the net10.0 template’s package versions shouldn’t inherently break due to missingnet10.0assemblies; documenting this rationale (or confirming the rest of the packages’ TFMs) would be sufficient rather than changing versions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Tests/Build/ProjectFileContents.cs` around lines 142 - 168, Reviewer concern: the Net100AppWithWarningsAsErrors template may mismatch package TFMs; fix by adding a brief inline comment above the Net100AppWithWarningsAsErrors constant in ProjectFileContents.cs stating that the listed package versions provide lib/net10.0 assets (e.g., Refit.HttpClientFactory 11.0.1 and Microsoft.Extensions.DependencyInjection 10.0.8) and that you verified the remaining packages expose net10.0; if any package does not, update that package version to one that provides net10.0 or replace it, then commit the verification note.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cs`:
- Around line 75-78: The assertion for the no-operationId fallback is too weak —
change the assertion in MissingInterfaceXmlDocsWarningsAsErrorsTests (the
generatedCode.Should().Contain checks) to assert the exact fallback summary text
produced for the ByEndpoint branch (e.g. require the full string "///
<summary>Operations for endpoint.</summary>" and similarly assert the exact
interface declaration "partial interface IOrdersEndpoint") so the test proves
the concrete fallback is used; after updating these assertions, also add/verify
equivalent tests exercising both multi-interface generation modes (ByEndpoint
and ByTag) to ensure the XML-doc fallback behavior is correct for each mode.
---
Nitpick comments:
In `@src/Refitter.Tests/Build/ProjectFileContents.cs`:
- Around line 142-168: Reviewer concern: the Net100AppWithWarningsAsErrors
template may mismatch package TFMs; fix by adding a brief inline comment above
the Net100AppWithWarningsAsErrors constant in ProjectFileContents.cs stating
that the listed package versions provide lib/net10.0 assets (e.g.,
Refit.HttpClientFactory 11.0.1 and Microsoft.Extensions.DependencyInjection
10.0.8) and that you verified the remaining packages expose net10.0; if any
package does not, update that package version to one that provides net10.0 or
replace it, then commit the verification note.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c0f3d9a-3c8c-4939-a895-9aec806794da
📒 Files selected for processing (5)
src/Refitter.Core/XmlDocumentationGenerator.cssrc/Refitter.Tests/Build/BuildHelper.cssrc/Refitter.Tests/Build/ProjectFileContents.cssrc/Refitter.Tests/Regression/MissingInterfaceXmlDocsWarningsAsErrorsTests.cssrc/Refitter.Tests/XmlDocumentationGeneratorTests.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Refitter.Tests/XmlDocumentationGeneratorTests.cs
- src/Refitter.Core/XmlDocumentationGenerator.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/codecov.yml:
- Line 58: Replace the moving tag usage of codecov/codecov-action@v5 with a
specific commit SHA (e.g., codecov/codecov-action@<sha>) to pin the action, and
update the workflow comment and the PR description to document why v5 is chosen
over v6 (note v6 Node.js 24 runtime compatibility concerns and runner support)
so reviewers can see the downgrade rationale and the supply-chain hardening
justification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9747b851-1ad0-42d0-8f18-51f0187b878a
📒 Files selected for processing (2)
.github/workflows/codecov.ymltest/ConsoleApp/Directory.Build.props
✅ Files skipped from review due to trivial changes (1)
- test/ConsoleApp/Directory.Build.props
christianhelle
force-pushed
the
christianhelle/issue-1109-investigation
branch
from
3c4ec1c to
57ea9d4
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This change fixes a regression where generated multi-interface clients could fail consumer builds with
CS1591when XML docs are enabled and warnings are treated as errors. The issue happened when OpenAPI metadata did not include tag or endpoint summaries, leaving generated public interfaces undocumented.The generator now always emits interface-level XML summaries for multi-interface output:
ByTag: uses tag description when available, otherwise a stable fallback summary.ByEndpoint: uses endpoint summary when available, otherwise a stable fallback summary.To prevent regressions, strict warnings-as-errors build coverage was added in tests by introducing strict project-file fixtures and regression scenarios that compile generated code under XML-doc + warnings-as-errors settings. The test build helper now also supports
net10.0target framework alongside the existingnet8.0andnet9.0targets.Example OpenAPI Specifications:
N/A
Example generated Refit interface
Summary by CodeRabbit
New Features
Tests
net10.0target framework in test project fixtures.Summary by CodeRabbit
New Features
Tests
Chores