[mono-api-html] Fix placeholder leak and add a test project - #26405
Conversation
…w Write path. The MultiplexedFormatter builds descriptions using the placeholder tokens %LESSERTHANREPLACEMENT% / %GREATERTHANREPLACEMENT% (since a single shared description string must be converted into each sub-formatter's own representation: </> for HTML, </> for markdown). The structured methods already convert these via Replace (), but the raw Write/WriteLine string overloads did not. The "New Type" addition path (ClassComparer.AddedInner) writes generic interface lists such as IEnumerable<AuthorizationRight> directly through Output.Write, so the placeholders leaked verbatim into the generated HTML (and markdown). Apply Replace () in the six string-based Write/WriteLine overloads so the placeholders are converted per sub-formatter on this path too. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…tion. We've had a few regressions in mono-api-html, so add a unit test project to guard against them. * Add tools/api-tools/mono-api-html-tests, an NUnit test project referencing mono-api-html. The first test (GenericInterfacesDoNotLeakPlaceholders) covers the placeholder-leak regression just fixed: it runs ApiDiffFormatted.Generate over in-memory api-info XML, producing both HTML and markdown, and asserts no %*THANREPLACEMENT% tokens leak and that the generic interface list renders correctly in both formats. * Add tools/api-tools/api-tools.slnx referencing mono-api-info, mono-api-html and the new test project, so 'dotnet test' in tools/api-tools builds all three and runs the tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a formatting bug in mono-api-html where placeholder tokens (%LESSERTHANREPLACEMENT% / %GREATERTHANREPLACEMENT%) could leak into generated HTML/markdown when raw Write/WriteLine string overloads were used, and adds a small NUnit regression test project to prevent recurrence.
Changes:
- Apply placeholder replacement in the string-based
Write/WriteLineoverloads inMultiplexedFormatter. - Add
mono-api-html-tests(NUnit) to exerciseApiDiffFormatted.Generateand assert correct rendering of generic interfaces + no placeholder leakage. - Add
tools/api-tools/api-tools.slnxto make it easy to rundotnet testfor the api-tools set.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/api-tools/mono-api-html/MultiplexedFormatter.cs | Ensures placeholder replacement occurs for raw string-based write paths across sub-formatters. |
| tools/api-tools/mono-api-html-tests/mono-api-html-tests.csproj | Introduces a new NUnit test project for mono-api-html regressions. |
| tools/api-tools/mono-api-html-tests/ApiDiffTests.cs | Adds a regression test covering generic interface rendering and placeholder non-leakage in HTML/markdown. |
| tools/api-tools/api-tools.slnx | Provides a solution entrypoint for building/testing the api-tools projects together. |
Suppressed comments (3)
tools/api-tools/mono-api-html/MultiplexedFormatter.cs:271
- 🤖
⚠️ Performance — Same as theWriteLine (string format, ...)overload:string.Formatis done once per formatter even though the result is identical. Cache once and reuse, only applyingReplaceper formatter.
public override void Write (string format, params object [] arguments)
{
foreach (var formatter in formatters)
formatter.Write (Replace (formatter, string.Format (format, arguments)));
}
tools/api-tools/mono-api-html/MultiplexedFormatter.cs:253
- 🤖
⚠️ Performance —sb.ToString ()is called once per sub-formatter even though the string content is identical; this adds avoidable allocations. Cachesb.ToString ()once and then applyReplaceper formatter.
public override void WriteLine (StringBuilder sb)
{
foreach (var formatter in formatters)
formatter.WriteLine (Replace (formatter, sb.ToString ()));
}
tools/api-tools/mono-api-html/MultiplexedFormatter.cs:277
- 🤖
⚠️ Performance — Same asWriteLine (StringBuilder):sb.ToString ()is computed once per formatter. Compute it once outside the loop, thenReplaceper formatter.
public override void Write (StringBuilder sb)
{
foreach (var formatter in formatters)
formatter.Write (Replace (formatter, sb.ToString ()));
}
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
* MultiplexedFormatter: compute the shared string.Format (...) / sb.ToString () result once outside the loop instead of once per sub-formatter, then apply Replace () per formatter. * Tests: assert against the exact placeholder tokens (%LESSERTHANREPLACEMENT% / %GREATERTHANREPLACEMENT%) instead of the imprecise "THANREPLACEMENT" substring. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #73703cb] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 203 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
We've had a few regressions in mono-api-html, so this fixes the latest one and adds a unit test project to guard against future ones.
The bug
The
MultiplexedFormatterbuilds descriptions using the placeholder tokens%LESSERTHANREPLACEMENT%/%GREATERTHANREPLACEMENT%, because a single shared description string must be converted into each sub-formatter's own representation (</>for HTML,</>for markdown). The structured methods already convert these viaReplace (), but the rawWrite/WriteLinestring overloads did not.The "New Type" addition path (
ClassComparer.AddedInner) writes generic interface lists such asIEnumerable<AuthorizationRight>directly throughOutput.Write, so the placeholders leaked verbatim into the generated HTML (and markdown), e.g.:Applying
Replace ()in the six string-basedWrite/WriteLineoverloads converts the placeholders per sub-formatter on this path too.Tests & solution
Added
tools/api-tools/mono-api-html-tests, an NUnit test project that referencesmono-api-html. The first test drives the publicApiDiffFormatted.Generateover in-memory api-info XML (the exactAuthorizationRightscase), producing both HTML and markdown, and asserts no*THANREPLACEMENTtokens leak and that the generic interface renders correctly in both formats. Verified the test fails without the fix.Added
tools/api-tools/api-tools.slnxreferencingmono-api-info,mono-api-htmland the new test project, sodotnet testintools/api-toolsbuilds all three and runs the tests.🤖 Pull request created by Copilot