fix: replace the XML generator with a reflection-based one

[gennaroprota]

@alandefreitas: Please let me know what you think and... if I have commented it too much :-).

[github-actions]

🚧 Danger.js checks for MrDocs are experimental; expect some rough edges while we tune the rules.

ℹ️ Info

Note

Commit 7b094ae (fix: replace the XML generator with a reflection-based one) touches 2512 source lines (non-test). Large change; add reviewer context if needed.

✨ Highlights

• 🧪 Existing golden tests changed (behavior likely shifted)

🧾 Changes by Scope

Scope	Lines Δ	Lines +	Lines -	Files Δ	Files +	Files ~	Files ↔	Files -
🥇 Golden Tests	45492	36338	9154	243	12	231	-	-
🛠️ Source	2533	839	1694	10	1	7	-	2
🏗️ Build / Toolchain	1135	781	354	2	-	2	-	-
📦 Other	6	6	-	1	-	1	-	-
⚙️ CI	2	2	-	1	-	1	-	-
Total	49168	37966	11202	257	13	242	-	2

Legend: Files + (added), Files ~ (modified), Files ↔ (renamed), Files - (removed)

🔝 Top Files

• test-files/golden-tests/javadoc/ref/ref.xml (Golden Tests): 2765 lines Δ (+2182 / -583)
• test-files/golden-tests/config/auto-function-metadata/returns-from-special.xml (Golden Tests): 2613 lines Δ (+1936 / -677)
• test-files/golden-tests/javadoc/copydoc/param-types.xml (Golden Tests): 2243 lines Δ (+1768 / -475)

Generated by 🚫 dangerJS against 53bcebf

[cppalliance-bot]

An automated preview of the documentation is available at https://1151.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-01-21 17:01:42 UTC

[alandefreitas]

I think we should be removing way more code than including for this PR to make sense. The test outputs haven't even changed. This should be a bug fix, not a refactor. The biggest problem with this generator is that it's not in sync with the metadata. And the reason this generator is out of sync is that we didn't have reflection, and manually keeping it in sync was too expensive and fragile. So what we want to do is let reflection handle the whole thing, and the whole generator will have only one or two functions that recursively iterate over everything to generate the metadata. 🥳

This means the XML outputs will change completely, as they now reflect everything in the metadata, which is what this generator was meant for. Our intention is not to recreate the broken version, but to recreate it with reflection. If we don't fix it, reflection would mean more code rather than less, as we now have to force reflection to recreate the errors we had without it (which is even harder than just keeping the original errors). And that's going to be extra code just to maintain it broken.

I think another issue here is that we should probably address the other issue first, where we should remove those small workarounds from reflection to make it reflect into different key names. Only canonical names should be used. All generators should have access to the same fields with the same names. Making this happen is basically just the finalization of the previous issue we already merged. And maybe the @meta field is required to make that happen, since ambiguity might arise. Only after establishing the canonical field names can we use them to generate XML. 😅

[alandefreitas]

What happened to this PR? I thought it would become more active after #1153 🚀

[gennaroprota]

I removed the commit before pulling, after merging #1153, and GitHub closed it. I shouldn't have used the develop branch.

[alandefreitas]

Oh 😅

[codecov]

Codecov Report

❌ Patch coverage is 93.10345% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.02%. Comparing base (a1f9a82) to head (53bcebf).

Files with missing lines	Patch %	Lines
src/lib/Gen/xml/XMLWriter.cpp	92.68%	0 Missing and 6 partials ⚠️
src/lib/AST/ExtractDocComment.cpp	93.75%	0 Missing and 1 partial ⚠️
src/lib/Support/Reflection/ReadableTypeName.hpp	92.85%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1151      +/-   ##
===========================================
+ Coverage    76.38%   77.02%   +0.64%     
===========================================
  Files          311      310       -1     
  Lines        29672    28974     -698     
  Branches      5863     5781      -82     
===========================================
- Hits         22664    22318     -346     
+ Misses        4735     4429     -306     
+ Partials      2273     2227      -46     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cppalliance-bot]

An automated preview of the documentation is available at https://1151.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-02-26 16:31:14 UTC

[alandefreitas]

This logic is great and correct. It can be simplified, though.

The first issue is that the code hardcodes MRDOCS_BUILD_TESTS=ON (which already makes BUILD_TESTING a noop because its only role is to set MRDOCS_BUILD_TESTS), then this is what you have for everything else:

Is Bottleneck	Flag (tests-flag)	Value (run tests)
True	MRDOCS_RUN_ALL_TESTS	True
False	BUILD_TESTING	False

The true path is noop because there's no MRDOCS_RUN_ALL_TESTS in CMakeLists.txt, unless this was included in another commit. The false path is also a noop anyway because of this table. So this code makes BUILD_TESTING irrelevant twice. Omitting everything would have the same result. No need for -D MRDOCS_BUILD_TESTS=ON, no need for custom tests flag (see issue 2), and no need for custom run-tests (see third issue).

The second issue is that once tests are forced to True, the infrastructure is repurposed to specify the test flag for something else and rely on the action to circumstantially turn it on. Instead of relying on this chain of events to turn this other flag on and off, you can just specify it directly in extra-args as -D MRDOCS_RUN_ALL_TESTS=${{ matrix.is-bottleneck && 'OFF' || 'ON' }} (or come up with a more representative name).

The third issue is that the logic reads as "I don't want to run tests," and then it creates another step that says "since I didn't run the tests before, let's run them now". But you don't need a separate step. If the extra-args already only built and registered the tests you need, then you already have only the ones you need and you already ran them.

To summarize, all you need is some -D MRDOCS_EXPENSIVE_TESTS=${{ matrix.is-bottleneck && 'OFF' || 'ON' }} (a single line changes in ci.yml) and in CMakeLists.txt skip registering (add_test) expensive tests with if (NOT MRDOCS_EXPENSIVE_TESTS).

[gennaroprota]

Yes, my solution was needlessly convoluted. Thanks for steering me the right way.