Skip to content

Add Unicode support for XML doc comment generation#948

Merged
christianhelle merged 5 commits into
mainfrom
unicode-support
Mar 7, 2026
Merged

Add Unicode support for XML doc comment generation#948
christianhelle merged 5 commits into
mainfrom
unicode-support

Conversation

@christianhelle

@christianhelle christianhelle commented Mar 7, 2026

Copy link
Copy Markdown
Owner

This pull request fixes #944 by improving the handling of Unicode and escape sequences in XML documentation generated from OpenAPI response descriptions. The main focus is on ensuring that non-ASCII characters are correctly rendered in XML status-code comments, and that only user-sourced text is sanitized, preserving hardcoded XML fragments.

Unicode and XML documentation improvements:

  • Added a DecodeJsonEscapedText() method in XmlDocumentationGenerator.cs to decode JSON-style escape sequences (like \uXXXX, \", \n) before inserting response descriptions into XML documentation.
  • Ensured that only user-sourced response description text is sanitized, while hardcoded XML doc fragments (such as <see cref="Task"/> and <list>) remain untouched. Reserved XML characters (&, <, >) are escaped at insertion.

Testing and validation:

  • Verified that Cyrillic Unicode and other non-ASCII characters render correctly in XML comments, with escape sequences properly decoded and compilation succeeding. All 1415 tests pass with no regressions.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected XML documentation generation so non-ASCII Unicode characters appear as readable text instead of escaped sequences.
  • Tests

    • Added unit and integration tests verifying Unicode preservation and successful end-to-end generation/build.
  • Documentation

    • Added guidance on sanitizing XML doc descriptions and a how-to for focused test execution; updated change/history records and decision notes.

christianhelle and others added 2 commits March 6, 2026 17:41
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@christianhelle christianhelle self-assigned this Mar 7, 2026
Copilot AI review requested due to automatic review settings March 7, 2026 06:51
@christianhelle christianhelle added enhancement New feature, bug fix, or request .NET Pull requests that contain changes to .NET code labels Mar 7, 2026
@coderabbitai

coderabbitai Bot commented Mar 7, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR decodes JSON-style escapes in NSwag-sourced response descriptions before XML-escaping, preventing \uXXXX sequences in generated XML doc comments. It adds DecodeJsonEscapedText() and SanitizeResponseDescription(), updates XmlDocumentationGenerator usage, and adds unit/integration tests plus documentation and orchestration notes.

Changes

Cohort / File(s) Summary
Production Code - XML Documentation Generator
src/Refitter.Core/XmlDocumentationGenerator.cs
Added DecodeJsonEscapedText() and SanitizeResponseDescription() helpers; updated AppendMethodDocumentation() and BuildResponseDescription() to sanitize NSwag-sourced descriptions before XML-escaping; added IsValidXmlChar checks and using System.Globalization;.
Unit Tests
src/Refitter.Tests/XmlDocumentationGeneratorTests.cs
Added tests verifying readable Unicode in / XML comments and absence of \uXXXX escapes for Cyrillic descriptions.
Integration Tests / Examples
src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs
Added Unicode-rich OpenAPI spec constant, new end-to-end tests that generate code with status-code comments and verify generated C# builds; refactored GenerateCode helper to accept generateStatusCodeComments flag.
Squad History & Agents
.squad/agents/fenster/history.md, .squad/agents/hockney/history.md, .squad/agents/keaton/history.md, .squad/agents/mcmanus/history.md
Added documentation entries covering the issue analysis, implementation notes, test descriptions, review approval, and validation gate results.
Decisions & Metadata
.squad/decisions.md, .squad/identity/now.md
Recorded decision entry for Issue #944, described fix and test coverage; updated identity metadata/focus area to reflect Issue #944 work.
Orchestration Logs
.squad/orchestration-log/...-fenster.md, ...-hockney.md, ...-keaton.md
Added logs describing implementation, added regression tests, and review/gate outcomes (build/tests passing).
Skill Documentation
.squad/skills/tunit-test-filtering/SKILL.md, .squad/skills/xml-doc-description-sanitization/SKILL.md
Added guidance for focused TUnit test filtering and detailed pattern for XML doc description sanitization (decode JSON escapes, then XML-escape, preserve XML markup).

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • christianhelle/refitter#850: Modifies XmlDocumentationGenerator and AppendMethodDocumentation(); touches the same class/method area as this PR’s sanitization changes.
  • christianhelle/refitter#941: Adds test coverage around XmlDocumentationGenerator behavior; related at the test/class level.

Poem

🐰✨ Bounced through escapes with a floppy-ear cheer,

Decoded the \u’s so the text may appear.
Cyrillic sings in comments now bright,
No more backslash-ruled Unicode night.
Hop, code, hop—XML reads clear! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% 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 'Add Unicode support for XML doc comment generation' directly describes the main change in the PR: implementing Unicode support for XML documentation by decoding JSON escapes and preserving non-ASCII characters.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from issue #944: DecodeJsonEscapedText() decodes JSON escapes, SanitizeResponseDescription() sanitizes only user-provided descriptions while preserving XML fragments, and tests verify Unicode preservation and successful compilation.
Out of Scope Changes check ✅ Passed All changes directly support issue #944: core fix in XmlDocumentationGenerator.cs, unit/integration tests in XmlDocumentationGeneratorTests.cs and GenerateStatusCodeCommentsTests.cs, and supporting documentation in .squad/ directories describing the fix and validation process.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch unicode-support

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes #944 by ensuring Unicode response descriptions are rendered as readable characters (not \uXXXX) in generated XML documentation, while keeping intentional XML doc markup intact.

Changes:

  • Decode JSON-style escape sequences (e.g., \uXXXX, \", \n) for OpenAPI-sourced response descriptions before XML-escaping reserved characters.
  • Apply sanitization only to user-sourced response text (e.g., method.ResultDescription, response.ExceptionDescription), preserving hardcoded XML doc fragments.
  • Add unit + integration regression tests to ensure Unicode remains readable and generated code compiles.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Refitter.Core/XmlDocumentationGenerator.cs Decodes JSON-style escapes in response descriptions and XML-escapes reserved characters at insertion points.
src/Refitter.Tests/XmlDocumentationGeneratorTests.cs Adds unit coverage verifying readable Unicode in status-code comments and absence of raw \uXXXX.
src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs Adds end-to-end generation + compilation tests for Unicode status-code comments; refactors helper to accept a spec string.
.squad/skills/xml-doc-description-sanitization/SKILL.md Documents the sanitization pattern and scope (user text only, preserve XML markup).
.squad/skills/tunit-test-filtering/SKILL.md Documents the repo’s TUnit-focused test execution pattern using --treenode-filter.
.squad/orchestration-log/2026-03-06T16-29-27Z-keaton.md Records review/validation notes for the #944 patch.
.squad/orchestration-log/2026-03-06T16-29-27Z-hockney.md Records added regression test coverage for #944.
.squad/orchestration-log/2026-03-06T16-29-27Z-fenster.md Records implementation notes for the #944 production fix.
.squad/identity/now.md Updates current focus to #944 implementation/validation.
.squad/decisions.md Captures the consolidated decision/approval for the #944 fix and testing approach.
.squad/agents/mcmanus/history.md Logs gate validation results for #944 (build/test/format).
.squad/agents/keaton/history.md Logs review findings/validation details for #944.
.squad/agents/hockney/history.md Updates testing framework notes and records #944 test additions.
.squad/agents/fenster/history.md Logs the #944 sanitization approach and implementation summary.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +399 to +413
case 'b':
result.Append('\b');
break;
case 'f':
result.Append('\f');
break;
case 'n':
result.Append('\n');
break;
case 'r':
result.Append('\r');
break;
case 't':
result.Append('\t');
break;

Copilot AI Mar 7, 2026

Copy link

Choose a reason for hiding this comment

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

DecodeJsonEscapedText() decodes \b and \f into backspace/form-feed control characters. Those characters (and many \u0000\u001F code points) are not valid in XML 1.0 and can produce invalid XML documentation (compiler warnings, or errors for consumers treating warnings as errors). Consider keeping \b/\f as literal sequences, or post-filtering the decoded text to only XML-valid characters (e.g., allow \t, \n, \r and strip/replace other control chars) before EscapeSymbols().

Copilot uses AI. Check for mistakes.
@codecov

codecov Bot commented Mar 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.00000% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.37%. Comparing base (0113c51) to head (3414c6c).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/Refitter.Core/XmlDocumentationGenerator.cs 58.00% 20 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #948      +/-   ##
==========================================
- Coverage   94.47%   93.37%   -1.11%     
==========================================
  Files          26       26              
  Lines        1593     1645      +52     
==========================================
+ Hits         1505     1536      +31     
- Misses         20       40      +20     
- Partials       68       69       +1     
Flag Coverage Δ
unittests 93.37% <58.00%> (-1.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
.squad/skills/tunit-test-filtering/SKILL.md (1)

15-15: Consider noting platform-specific path syntax.

The executable paths use Windows-style backslashes, which are appropriate for the PowerShell examples but won't work on Linux/macOS. Consider adding a note about platform differences or providing a cross-platform example using forward slashes.

Also applies to: 22-27

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.squad/skills/tunit-test-filtering/SKILL.md at line 15, Update the
Windows-style path examples (e.g. the line containing "Test app path:
`src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe`" and the similar
entries around the section for lines 22-27) to call out platform-specific
syntax: add a brief note that backslashes are Windows/PowerShell only and
provide a cross-platform alternative using forward slashes (e.g.
src/Refitter.Tests/bin/Release/net10.0/Refitter.Tests.exe) and mention which
shells/examples use which syntax (PowerShell vs bash/zsh) so readers on
Linux/macOS know to use the forward-slash form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.squad/agents/hockney/history.md:
- Line 13: The documentation example in the test pattern currently uses the
xUnit attribute `[Fact]` but the actual test suite `Refitter.Tests.Examples` and
its async test methods (e.g., those calling `GenerateCode()` and asserting via
`BuildHelper.BuildCSharp(code).Should().BeTrue()`) use TUnit's `[Test]`
attribute; update the example on line 13 to replace `[Fact]` with `[Test]` so
the documented pattern matches the real tests and new examples compile.

In @.squad/decisions.md:
- Around line 339-343: Update the Gates summary to match the validated total of
1,451 tests: locate the "#### Gates" section and replace the hardcoded
"1415/1415" string with "1451/1451" (or the exact count shown in the 2026-03-06
validation gate entry in the consolidated history file) so the gate summary and
the consolidated validation record remain consistent.

In `@src/Refitter.Core/XmlDocumentationGenerator.cs`:
- Line 1: The DecodeJsonEscapedText method currently only escapes reserved XML
symbols but must also remove/filter XML-invalid control characters emitted by
JSON escapes (e.g., \u0008, \u000C); update DecodeJsonEscapedText (and the
post-decode handling around lines 352-355) to post-process the decoded string
and strip any characters illegal in XML 1.0 (allow only 0x9, 0xA, 0xD and
characters >= 0x20, and valid Unicode scalar ranges), then return the cleaned
string so generated XML documentation can't contain invalid control characters.

In `@src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs`:
- Around line 58-76: Add a Swagger 2.0 variant of the Unicode status-code
comment test to cover the older parser: create a new constant (e.g.,
Swagger20SpecWithUnicodeStatusCodeComments) mirroring
OpenApiSpecWithUnicodeStatusCodeComments but using Swagger 2.0 syntax, then
update or add a test in GenerateStatusCodeCommentsTests (the same test cases
referenced around OpenApiSpecWithUnicodeStatusCodeComments and the block at
125-141) to run the existing assertion/assertions against both the OpenAPI 3.0
constant and the new Swagger 2.0 constant so the unicode status-code comments
path is exercised for both parsers.

In `@src/Refitter.Tests/XmlDocumentationGeneratorTests.cs`:
- Around line 224-244: The test currently only verifies sanitization of
response.ExceptionDescription; add coverage for the method's ResultDescription
(the code path in XmlDocumentationGenerator.AppendMethodDocumentation that
sanitizes method.ResultDescription) by setting method.ResultDescription to a
Unicode string (e.g., "Ошибка ответа") in the CreateOperationModel call or by
patching the operation model, call AppendMethodDocumentation as before, and
assert the resulting docs contain the readable Unicode inside the <returns>
block (e.g., contain "<description>Ошибка ответа</description>") and do NOT
contain escaped Unicode sequences like "\u041e\u0448"; reference
AppendMethodDocumentation and the method.ResultDescription property when
locating where to add the extra setup and assertions.

---

Nitpick comments:
In @.squad/skills/tunit-test-filtering/SKILL.md:
- Line 15: Update the Windows-style path examples (e.g. the line containing
"Test app path: `src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe`" and
the similar entries around the section for lines 22-27) to call out
platform-specific syntax: add a brief note that backslashes are
Windows/PowerShell only and provide a cross-platform alternative using forward
slashes (e.g. src/Refitter.Tests/bin/Release/net10.0/Refitter.Tests.exe) and
mention which shells/examples use which syntax (PowerShell vs bash/zsh) so
readers on Linux/macOS know to use the forward-slash form.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef32b870-b4a9-4776-9485-1fc726f592b1

📥 Commits

Reviewing files that changed from the base of the PR and between dd167ce and 608eb08.

📒 Files selected for processing (14)
  • .squad/agents/fenster/history.md
  • .squad/agents/hockney/history.md
  • .squad/agents/keaton/history.md
  • .squad/agents/mcmanus/history.md
  • .squad/decisions.md
  • .squad/identity/now.md
  • .squad/orchestration-log/2026-03-06T16-29-27Z-fenster.md
  • .squad/orchestration-log/2026-03-06T16-29-27Z-hockney.md
  • .squad/orchestration-log/2026-03-06T16-29-27Z-keaton.md
  • .squad/skills/tunit-test-filtering/SKILL.md
  • .squad/skills/xml-doc-description-sanitization/SKILL.md
  • src/Refitter.Core/XmlDocumentationGenerator.cs
  • src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs
  • src/Refitter.Tests/XmlDocumentationGeneratorTests.cs

Comment thread .squad/agents/hockney/history.md Outdated
Comment thread .squad/decisions.md
Comment thread src/Refitter.Core/XmlDocumentationGenerator.cs
Comment thread src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs
Comment thread src/Refitter.Tests/XmlDocumentationGeneratorTests.cs
@christianhelle

Copy link
Copy Markdown
Owner Author

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot AI commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

@christianhelle I've opened a new pull request, #949, to work on those changes. Once the pull request is ready, I'll request review from you.

@sonarqubecloud

sonarqubecloud Bot commented Mar 7, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.squad/decisions.md (1)

331-342: ⚠️ Potential issue | 🟡 Minor

Internal inconsistency: test count differs within the same section.

Line 331 states "All 1415 tests pass" while line 342 states "All tests passing (1451/1451)". These should be aligned to avoid an inconsistent audit trail.

📝 Proposed fix
 - Surrogate pairs handled naturally via C# UTF-16
-- All 1415 tests pass; no regressions
+- All 1451 tests pass; no regressions
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.squad/decisions.md around lines 331 - 342, Update the inconsistent test
counts in the decision note so both statements match: replace "All 1415 tests
pass" with the accurate test count used in the Gates line (or vice versa) so
that "All ... tests pass" and "All tests passing (1451/1451)" use the same
number; ensure both occurrences in the section are identical and reflect the
actual test run total.
🧹 Nitpick comments (3)
src/Refitter.Core/XmlDocumentationGenerator.cs (3)

398-410: Loop index mutation is idiomatic but flagged by static analysis.

Mutating index inside the loop body (lines 398, 410) is a common pattern for escape-sequence parsers. SonarCloud flags this as it can obscure control flow. If you refactor per the complexity suggestion, this would be resolved naturally. Otherwise, switching to a while loop with explicit index advancement would silence the warning without changing behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Refitter.Core/XmlDocumentationGenerator.cs` around lines 398 - 410, The
loop mutates the for-loop index variable `index` inside the body when parsing
escape sequences (e.g., handling `'u'` hex escapes), which triggers static
analysis; change the parsing loop to an explicit while-loop over `index` (e.g.,
`while (index < input.Length)`) and advance `index` only in one place
(incrementing by 1 for normal characters and by 5 when consuming a `\uXXXX`
sequence) so you no longer modify the loop control variable inside the body;
keep the same logic using `input`, `result.Append`, `IsValidXmlChar`, and the
hex parsing with `int.TryParse` but move index advancement out of the inner
block to make control flow explicit and satisfy SonarCloud.

357-445: Consider extracting helper methods to reduce cognitive complexity.

SonarCloud flags cognitive complexity of 37 (allowed 15). While the parsing logic is functionally correct, extracting helpers would improve maintainability:

  • TryDecodeUnicodeEscape(string input, int index, out char decoded, out int consumed)
  • DecodeSimpleEscape(char escapedChar)

Additionally, since DecodeJsonEscapedText doesn't access instance state, it could be made static (along with IsValidXmlChar).

♻️ Sketch of extracted helpers
-    private string DecodeJsonEscapedText(string input)
+    private static string DecodeJsonEscapedText(string input)
     {
         // ... early returns unchanged ...
         
         for (var index = 0; index < input.Length; index++)
         {
             // ... use extracted helpers:
-            if (escapedCharacter == 'u' && index + 4 < input.Length)
-            {
-                var hexValue = input.Substring(index + 1, 4);
-                if (int.TryParse(hexValue, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out var unicodeCodePoint))
-                {
-                    // ...
-                }
-            }
+            if (TryDecodeUnicodeEscape(input, index, out var decoded, out var consumed))
+            {
+                if (IsValidXmlChar(decoded)) result.Append(decoded);
+                index += consumed;
+                continue;
+            }
         }
     }
+
+    private static bool TryDecodeUnicodeEscape(string input, int index, out char decoded, out int consumed)
+    {
+        decoded = default;
+        consumed = 0;
+        if (index + 5 >= input.Length || input[index + 1] != 'u') return false;
+        var hex = input.Substring(index + 2, 4);
+        if (!int.TryParse(hex, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out var cp)) return false;
+        decoded = (char)cp;
+        consumed = 5; // \uXXXX
+        return true;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Refitter.Core/XmlDocumentationGenerator.cs` around lines 357 - 445, The
DecodeJsonEscapedText method is too complex; refactor by extracting the Unicode
and simple-escape parsing into helpers and making relevant methods static: add
TryDecodeUnicodeEscape(string input, int index, out char decoded, out int
consumed) to parse "\uXXXX" (return true and set consumed to number of chars
consumed when successful), add DecodeSimpleEscape(char escapedChar) -> char? to
map "n","t","r","b","f","\"", "\\", "/" to their char or null for unknowns, and
change IsValidXmlChar to static; then simplify DecodeJsonEscapedText to call
TryDecodeUnicodeEscape and DecodeSimpleEscape, update the loop to advance index
by consumed (when unicode) or 0/1 as appropriate, and keep the existing
sanitization behavior for unknown escapes by appending backslash + char only if
IsValidXmlChar permits. Ensure signatures of new helpers are used in
DecodeJsonEscapedText and mark all three methods static.

447-454: Minor: IsValidXmlChar doesn't exclude 0xFFFE/0xFFFF.

XML 1.0 spec excludes #xFFFE and #xFFFF as valid characters. The current check c >= 0x20 allows these. In practice, these codepoints are extremely rare in OpenAPI descriptions, so impact is minimal.

🛡️ Strict XML 1.0 compliance (optional)
     private static bool IsValidXmlChar(char c)
     {
-        return c == 0x09 || c == 0x0A || c == 0x0D || c >= 0x20;
+        return c == 0x09 || c == 0x0A || c == 0x0D ||
+               (c >= 0x20 && c != 0xFFFE && c != 0xFFFF);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Refitter.Core/XmlDocumentationGenerator.cs` around lines 447 - 454,
IsValidXmlChar currently allows all chars >= 0x20 which inadvertently permits
the disallowed 0xFFFE and 0xFFFF; update IsValidXmlChar to only accept
characters in the XML 1.0 allowed ranges (allow 0x09, 0x0A, 0x0D, and chars from
0x20 through 0xFFFD) and explicitly exclude 0xFFFE and 0xFFFF (i.e., ensure the
check uses an upper bound of 0xFFFD and filters out 0xFFFE/0xFFFF) so the method
enforces XML 1.0 compliance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.squad/decisions.md:
- Around line 331-342: Update the inconsistent test counts in the decision note
so both statements match: replace "All 1415 tests pass" with the accurate test
count used in the Gates line (or vice versa) so that "All ... tests pass" and
"All tests passing (1451/1451)" use the same number; ensure both occurrences in
the section are identical and reflect the actual test run total.

---

Nitpick comments:
In `@src/Refitter.Core/XmlDocumentationGenerator.cs`:
- Around line 398-410: The loop mutates the for-loop index variable `index`
inside the body when parsing escape sequences (e.g., handling `'u'` hex
escapes), which triggers static analysis; change the parsing loop to an explicit
while-loop over `index` (e.g., `while (index < input.Length)`) and advance
`index` only in one place (incrementing by 1 for normal characters and by 5 when
consuming a `\uXXXX` sequence) so you no longer modify the loop control variable
inside the body; keep the same logic using `input`, `result.Append`,
`IsValidXmlChar`, and the hex parsing with `int.TryParse` but move index
advancement out of the inner block to make control flow explicit and satisfy
SonarCloud.
- Around line 357-445: The DecodeJsonEscapedText method is too complex; refactor
by extracting the Unicode and simple-escape parsing into helpers and making
relevant methods static: add TryDecodeUnicodeEscape(string input, int index, out
char decoded, out int consumed) to parse "\uXXXX" (return true and set consumed
to number of chars consumed when successful), add DecodeSimpleEscape(char
escapedChar) -> char? to map "n","t","r","b","f","\"", "\\", "/" to their char
or null for unknowns, and change IsValidXmlChar to static; then simplify
DecodeJsonEscapedText to call TryDecodeUnicodeEscape and DecodeSimpleEscape,
update the loop to advance index by consumed (when unicode) or 0/1 as
appropriate, and keep the existing sanitization behavior for unknown escapes by
appending backslash + char only if IsValidXmlChar permits. Ensure signatures of
new helpers are used in DecodeJsonEscapedText and mark all three methods static.
- Around line 447-454: IsValidXmlChar currently allows all chars >= 0x20 which
inadvertently permits the disallowed 0xFFFE and 0xFFFF; update IsValidXmlChar to
only accept characters in the XML 1.0 allowed ranges (allow 0x09, 0x0A, 0x0D,
and chars from 0x20 through 0xFFFD) and explicitly exclude 0xFFFE and 0xFFFF
(i.e., ensure the check uses an upper bound of 0xFFFD and filters out
0xFFFE/0xFFFF) so the method enforces XML 1.0 compliance.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da06f26f-cb0a-48a3-936b-6a4a0e48cc01

📥 Commits

Reviewing files that changed from the base of the PR and between 608eb08 and 3414c6c.

📒 Files selected for processing (6)
  • .squad/agents/hockney/history.md
  • .squad/decisions.md
  • .squad/skills/tunit-test-filtering/SKILL.md
  • src/Refitter.Core/XmlDocumentationGenerator.cs
  • src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs
  • src/Refitter.Tests/XmlDocumentationGeneratorTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
  • .squad/skills/tunit-test-filtering/SKILL.md
  • src/Refitter.Tests/XmlDocumentationGeneratorTests.cs
  • .squad/agents/hockney/history.md
  • src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs

@christianhelle
christianhelle merged commit 5b03407 into main Mar 7, 2026
12 of 14 checks passed
@christianhelle
christianhelle deleted the unicode-support branch March 7, 2026 11:56
hwinther pushed a commit to hwinther/test that referenced this pull request May 6, 2026
Updated [refitter](https://github.com/christianhelle/refitter) from
1.7.1 to 2.0.0.

<details>
<summary>Release notes</summary>

_Sourced from [refitter's
releases](https://github.com/christianhelle/refitter/releases)._

## 2.0.0

## What's Changed

* Fix numeric format with pattern quirk - infer type from format for all
numeric types by @​Copilot in
christianhelle/refitter#869
* Read group documentation from document tags. by @​DJ4ddi in
christianhelle/refitter#887
* Fix null reference and XML escaping in XmlDocumentationGenerator by
@​Copilot in christianhelle/refitter#890
* Fix build workflow: add dotnet restore before dotnet msbuild in
Prepare step by @​Copilot in
christianhelle/refitter#892
christianhelle/refitter#895
* Fix MSBuild workflow by @​christianhelle in
christianhelle/refitter#898
* Add support for generating a single client from multiple OpenAPI
specifications by @​Copilot in
christianhelle/refitter#904
* Migrate from Microsoft.OpenApi.Readers 1.x to Microsoft.OpenApi 3.x by
@​vgmello in christianhelle/refitter#907
* Improve Smoke Tests execution time by @​christianhelle in
christianhelle/refitter#915
* Fix #​580: Nullable strings marked correctly by @​christianhelle in
christianhelle/refitter#921
* Fix #​672: MultipleInterfaces ByTag method naming scoped per-interface
by @​christianhelle in
christianhelle/refitter#922
* Fix #​635: Refactor source generator to use context.AddSource() by
@​christianhelle in christianhelle/refitter#923
* Add custom format mappings configuration by @​christianhelle in
christianhelle/refitter#927
* Fix multipart form-data parameter extraction by @​christianhelle in
christianhelle/refitter#928
christianhelle/refitter#911
christianhelle/refitter#902
* Fix smoke tests: --interface-only variant missing using directive for
contract types by @​Copilot in
christianhelle/refitter#933
* Fix SonarCloud Code Quality Issues by @​Copilot in
christianhelle/refitter#932
* Add debug logging for source generator when searching for .refitter
files by @​codymullins in
christianhelle/refitter#743
* Fix: Base type not generated for types using oneOf with discriminator
by @​Copilot in christianhelle/refitter#906
* Add option for Method Level Authorization header attribute by
@​Roflincopter in christianhelle/refitter#897
* Fix PR #​897 review feedback and add comprehensive bearer auth tests
by @​christianhelle in
christianhelle/refitter#936
* Fix numeric suffix added to interface method names in ByTag mode by
@​Copilot in christianhelle/refitter#914
* Improve OpenAPI parse + codegen throughput by removing avoidable
allocations and repeated regex work by @​Copilot in
christianhelle/refitter#937
* Move [JsonConverter] from enum properties to enum types by
@​christianhelle in christianhelle/refitter#938
* Fix broken CLI tool help text by @​christianhelle in
christianhelle/refitter#940
* Improve code coverage to >90% by @​christianhelle in
christianhelle/refitter#941
* Microsoft.OpenApi v3.4 by @​christianhelle in
christianhelle/refitter#945
* Add Unicode support for XML doc comment generation by @​christianhelle
in christianhelle/refitter#948
* Add PropertyNamingPolicy support for JSON property naming by
@​christianhelle in christianhelle/refitter#969
christianhelle/refitter#966
* Fix recursive schema stack overflows by @​christianhelle in
christianhelle/refitter#971
* Made Header Parameters for Security Schemes safe to use as C# variable
name by @​smoerijf in
christianhelle/refitter#977
* Enhance schema alias handling and property name generation by
@​christianhelle in christianhelle/refitter#996
* Verify alias handling and sanitize PascalCase properties by
@​christianhelle in christianhelle/refitter#997
* Harden .refitter settings deserialization and output path handling by
@​christianhelle in christianhelle/refitter#1000
* Special-case the default .refitter filename before deriving
OutputFilename by @​coderabbitai[bot] in
christianhelle/refitter#1002
* [v2.0 audit] Fix pre-release regressions from #​1057 by
@​christianhelle in christianhelle/refitter#1064
* Resolve high-severity audit findings from #​1057 by @​christianhelle
in christianhelle/refitter#1067
* [v2.0 audit] Close remaining verified #​1057 regressions by
@​christianhelle in christianhelle/refitter#1070
* Harden generation flows by @​christianhelle in
christianhelle/refitter#1071
* Breaking changes and Migration Guide for v2.0.0 by @​christianhelle in
christianhelle/refitter#1009
* Handle equivalent duplicate schemas in multi-spec merge by
@​christianhelle in christianhelle/refitter#1076
* Tighten warning handling across Refitter builds by @​christianhelle in
christianhelle/refitter#1079
* Fix default solution path and update .NET version in VS Code config by
@​christianhelle in christianhelle/refitter#1082
* Fix docker smoke tests by @​christianhelle in
christianhelle/refitter#1081
* Sanitize malformed schema type names without renaming clean schemas by
@​christianhelle in christianhelle/refitter#1085

 ... (truncated)

## 1.7.3

## What's Changed
* Add support for systems running only .NET 10.0 (without .NET 8.0 or
9.0) in Refitter.MSBuild by @​christianhelle in
christianhelle/refitter#882
* Update to return HttpResponseMessage for file downloads by @​frogcrush
in christianhelle/refitter#877

## New Contributors
* @​frogcrush made their first contribution in
christianhelle/refitter#877

**Full Changelog**:
christianhelle/refitter@1.7.2...1.7.3

## 1.7.2

**Implemented enhancements:**

- Improve Immutable Records ergonomics
[\#​844](christianhelle/refitter#844)
- Omit certain operation headers and include all others
[\#​840](christianhelle/refitter#840)
- Create .refitter settings file as part of output
[\#​859](christianhelle/refitter#859) by
@[christianhelle
- Fix integerType enum deserialization issue
[\#​855](christianhelle/refitter#855)
([christianhelle](https://github.com/christianhelle))
- support custom nswag template directory \#​844
[\#​854](christianhelle/refitter#854) by
@​kmc059000
- Fix missing method parameter XML code-documentation
[\#​850](christianhelle/refitter#850)
([christianhelle](https://github.com/christianhelle))

**Fixed bugs:**

- integerType parsing in settings file fails
[\#​851](christianhelle/refitter#851)
- CS1573 : Method parameter has no matching XML comment
[\#​846](christianhelle/refitter#846)

**Merged pull requests:**

- docs: add frogcrush as a contributor for code
[\#​878](christianhelle/refitter#878)
([allcontributors[bot]](https://github.com/apps/allcontributors))
- Migrate solution files from .sln to .slnx format
[\#​876](christianhelle/refitter#876)
([Copilot](https://github.com/apps/copilot-swe-agent))
- Fix issue with randomly failing tests to due parallel execution
[\#​872](christianhelle/refitter#872)
([christianhelle](https://github.com/christianhelle))
- chore\(deps\): update dependency tunit to 1.9.2
[\#​863](christianhelle/refitter#863)
([renovate[bot]](https://github.com/apps/renovate))
- chore\(deps\): update dependency tunit to 1.7.7
[\#​862](christianhelle/refitter#862)
([renovate[bot]](https://github.com/apps/renovate))
- Add unit tests for WriteRefitterSettingsFile functionality
[\#​860](christianhelle/refitter#860)
([Copilot](https://github.com/apps/copilot-swe-agent))
- chore\(deps\): update dependency ruby to v4
[\#​858](christianhelle/refitter#858)
([renovate[bot]](https://github.com/apps/renovate))
- chore\(deps\): update dependency tunit to 1.6.28
[\#​857](christianhelle/refitter#857)
([renovate[bot]](https://github.com/apps/renovate))
- docs: add 0x2badc0de as a contributor for bug
[\#​856](christianhelle/refitter#856)
([allcontributors[bot]](https://github.com/apps/allcontributors))
- chore\(deps\): update dependency swashbuckle.aspnetcore to 10.1.0
[\#​853](christianhelle/refitter#853)
([renovate[bot]](https://github.com/apps/renovate))
- chore\(deps\): update dependency tunit to 1.6.0
[\#​852](christianhelle/refitter#852)
([renovate[bot]](https://github.com/apps/renovate))
- docs: add lilinus as a contributor for code
[\#​849](christianhelle/refitter#849)
([allcontributors[bot]](https://github.com/apps/allcontributors))
- chore\(deps\): update dependency ruby to v3.4.8
[\#​845](christianhelle/refitter#845)
([renovate[bot]](https://github.com/apps/renovate))
- chore\(deps\): update dependency tunit to 1.5.70
[\#​837](christianhelle/refitter#837)
([renovate[bot]](https://github.com/apps/renovate))


**Full Changelog**:
christianhelle/refitter@1.7.1...1.7.2

Commits viewable in [compare
view](christianhelle/refitter@1.7.1...2.0.0).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=refitter&package-manager=nuget&previous-version=1.7.1&new-version=2.0.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
vgmello pushed a commit to vgmello/momentum that referenced this pull request May 10, 2026
Updated [Refitter.MSBuild](https://github.com/christianhelle/refitter)
from 1.7.3 to 2.0.0.

<details>
<summary>Release notes</summary>

_Sourced from [Refitter.MSBuild's
releases](https://github.com/christianhelle/refitter/releases)._

## 2.0.0

## What's Changed

* Fix numeric format with pattern quirk - infer type from format for all
numeric types by @​Copilot in
christianhelle/refitter#869
* Read group documentation from document tags. by @​DJ4ddi in
christianhelle/refitter#887
* Fix null reference and XML escaping in XmlDocumentationGenerator by
@​Copilot in christianhelle/refitter#890
* Fix build workflow: add dotnet restore before dotnet msbuild in
Prepare step by @​Copilot in
christianhelle/refitter#892
christianhelle/refitter#895
* Fix MSBuild workflow by @​christianhelle in
christianhelle/refitter#898
* Add support for generating a single client from multiple OpenAPI
specifications by @​Copilot in
christianhelle/refitter#904
* Migrate from Microsoft.OpenApi.Readers 1.x to Microsoft.OpenApi 3.x by
@​vgmello in christianhelle/refitter#907
* Improve Smoke Tests execution time by @​christianhelle in
christianhelle/refitter#915
* Fix #​580: Nullable strings marked correctly by @​christianhelle in
christianhelle/refitter#921
* Fix #​672: MultipleInterfaces ByTag method naming scoped per-interface
by @​christianhelle in
christianhelle/refitter#922
* Fix #​635: Refactor source generator to use context.AddSource() by
@​christianhelle in christianhelle/refitter#923
* Add custom format mappings configuration by @​christianhelle in
christianhelle/refitter#927
* Fix multipart form-data parameter extraction by @​christianhelle in
christianhelle/refitter#928
christianhelle/refitter#911
christianhelle/refitter#902
* Fix smoke tests: --interface-only variant missing using directive for
contract types by @​Copilot in
christianhelle/refitter#933
* Fix SonarCloud Code Quality Issues by @​Copilot in
christianhelle/refitter#932
* Add debug logging for source generator when searching for .refitter
files by @​codymullins in
christianhelle/refitter#743
* Fix: Base type not generated for types using oneOf with discriminator
by @​Copilot in christianhelle/refitter#906
* Add option for Method Level Authorization header attribute by
@​Roflincopter in christianhelle/refitter#897
* Fix PR #​897 review feedback and add comprehensive bearer auth tests
by @​christianhelle in
christianhelle/refitter#936
* Fix numeric suffix added to interface method names in ByTag mode by
@​Copilot in christianhelle/refitter#914
* Improve OpenAPI parse + codegen throughput by removing avoidable
allocations and repeated regex work by @​Copilot in
christianhelle/refitter#937
* Move [JsonConverter] from enum properties to enum types by
@​christianhelle in christianhelle/refitter#938
* Fix broken CLI tool help text by @​christianhelle in
christianhelle/refitter#940
* Improve code coverage to >90% by @​christianhelle in
christianhelle/refitter#941
* Microsoft.OpenApi v3.4 by @​christianhelle in
christianhelle/refitter#945
* Add Unicode support for XML doc comment generation by @​christianhelle
in christianhelle/refitter#948
* Add PropertyNamingPolicy support for JSON property naming by
@​christianhelle in christianhelle/refitter#969
christianhelle/refitter#966
* Fix recursive schema stack overflows by @​christianhelle in
christianhelle/refitter#971
* Made Header Parameters for Security Schemes safe to use as C# variable
name by @​smoerijf in
christianhelle/refitter#977
* Enhance schema alias handling and property name generation by
@​christianhelle in christianhelle/refitter#996
* Verify alias handling and sanitize PascalCase properties by
@​christianhelle in christianhelle/refitter#997
* Harden .refitter settings deserialization and output path handling by
@​christianhelle in christianhelle/refitter#1000
* Special-case the default .refitter filename before deriving
OutputFilename by @​coderabbitai[bot] in
christianhelle/refitter#1002
* [v2.0 audit] Fix pre-release regressions from #​1057 by
@​christianhelle in christianhelle/refitter#1064
* Resolve high-severity audit findings from #​1057 by @​christianhelle
in christianhelle/refitter#1067
* [v2.0 audit] Close remaining verified #​1057 regressions by
@​christianhelle in christianhelle/refitter#1070
* Harden generation flows by @​christianhelle in
christianhelle/refitter#1071
* Breaking changes and Migration Guide for v2.0.0 by @​christianhelle in
christianhelle/refitter#1009
* Handle equivalent duplicate schemas in multi-spec merge by
@​christianhelle in christianhelle/refitter#1076
* Tighten warning handling across Refitter builds by @​christianhelle in
christianhelle/refitter#1079
* Fix default solution path and update .NET version in VS Code config by
@​christianhelle in christianhelle/refitter#1082
* Fix docker smoke tests by @​christianhelle in
christianhelle/refitter#1081
* Sanitize malformed schema type names without renaming clean schemas by
@​christianhelle in christianhelle/refitter#1085

 ... (truncated)

Commits viewable in [compare
view](christianhelle/refitter@1.7.3...2.0.0).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Refitter.MSBuild&package-manager=nuget&previous-version=1.7.3&new-version=2.0.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, bug fix, or request .NET Pull requests that contain changes to .NET code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-ASCII response descriptions become \uXXXX in XML comments

3 participants