fix: annotate HtmlToWmlConverterCore.cs and HtmlToWmlConverter.cs for nullable reference types - #663
Merged
Merged
Conversation
… nullable reference types Removes #nullable disable from the two remaining HTML->DOCX files (the transform/updater core and its public facade), completing the four-file scope of issue #647 (steps 1-2, the CSS parser/applier, landed in #661). HtmlToWmlConverterCore.cs (5361 lines, ~438 distinct warnings) required the most structural work: LocalExtensions.GetProp now returns CssExpression?, rippling through ~55 call sites; several return types widened to nullable (Transform, GetParagraphProperties, GetTableArray, GetCellShading, etc.) to honestly reflect paths that can produce no markup; CharStyleAttributes and HeadingInfo fields widened or promoted to `required` based on how they're actually constructed. Where a call site immediately dereferences a GetProp(...) result with no existing null-check, the CSS cascade/shorthand normalization guarantees non-null and a `!` assertion documents why. Where the original code already had a defensive null fallback (e.g. table cell margin properties), the fix widens the local to nullable instead of asserting non-null over an existing guard. One provably unreachable branch in FontMerge was removed as dead code. HtmlToWmlConverter.cs (534 lines, 52 warnings) required widening HtmlToWmlConverterSettings' 8 reference-type fields to nullable, since GetDefaultSettings() leaves BaseUriForImages null even in the trusted factory path. GetDefaultFontInfo keeps its pre-existing graceful `?.` fallback for StyleDefinitionsPart rather than asserting non-null. HtmlToWmlCssApplier.cs (already nullable-clean from #661) needed a small ripple fix: three settings.SectPr accesses now assert non-null now that SectPr itself is nullable. CLAUDE.md's warning-baseline prose is updated to match: 4 legacy #nullable-disable files remain (down from 6), library baseline 117 (was 119), test project baseline 693 (was 689, verified stable across three independent measurements including a clean rebuild). Validated: dotnet build (0 errors, 0 warnings in the touched files), full Docxodus.Tests suite (3923 passed, 3 skipped, 0 failed, including all 232 HwTests), WASM build (within size budget), and all out-of-solution tool/benchmark projects build clean. Closes #647 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VSnwkK1Nx6zZb2RnnoxKdx
JSv4
pushed a commit
that referenced
this pull request
Sep 2, 2026
#663-#666 continue the #650 nullable run. Two of them land on this demo's measured path: #665 and #666 annotate WmlToHtmlConverter.cs and .Charts.cs, which are the conversion stage of the full+HTML depth. They are not cosmetic either — removing the #nullable disable header added 27 null-handling lines to the converter, which is real runtime code, so this got a rebuild and the full browser run rather than a wave-through. 14/14 pass. Measured, and the numbers are NOT republished, deliberately. A single controlled session read 12-25% faster across most rows (revisions 50->38, redline 67->53, full 127->106, conversion stage in isolation 60->53) — but oneCall, which IS the redline depth's call, came back flat at 51->50. Redline cannot drop 21% while docxDiffCompareProducts holds; the pattern contradicts itself. Added null-guards should also cost time rather than save it, so the direction is wrong for the change too. That is variance, on one session, and the published figures came from a pooled two-session controlled measurement. The README asks for pooling before believing a movement and for labelling one that fits nothing as the machine; publishing this would break both rules on my own page. Also merges #663 (HtmlToWmlConverter/Core) and #664 (DocumentBuilder), neither of which this demo calls.
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
1 participant
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.
Summary
Completes issue #647: the HTML→DOCX converter family is now fully nullable-annotated. Steps 1–2 (the CSS parser/applier) landed in #661; this PR is steps 3–4, the transform/updater core and its public facade — the last two of the four files in scope.
HtmlToWmlConverterCore.cs(5,361 lines, the largest file touched in this whole campaign, ~438 distinct warnings):#nullable disableremoved. The bulk of the ripple comes fromLocalExtensions.GetPropnow returningCssExpression?(it can genuinely return null when a property isn't set), which touches ~55 call sites. Several methods had their return types widened to nullable where they can legitimately produce no markup for a given input (Transform,GetParagraphProperties,GetTableArray,GetCellShading, and similar). Where a call site dereferences aGetProp(...)result immediately with no pre-existing null-check, a!documents that CSS shorthand/cascade normalization guarantees the property is set by the time it reaches that point. Where the original code already had a defensive fallback (e.g. table cell margin properties defaulting when null), the fix widens the local to nullable instead of asserting non-null over an existing guard — the guard was there for a reason. One provably unreachable branch inFontMerge(both inputs null, already handled by an earlier early-return) was removed as dead code.HtmlToWmlConverter.cs(534 lines, 52 warnings):HtmlToWmlConverterSettings's 8 reference-type fields are widened nullable rather than maderequired, becauseGetDefaultSettings()— the library's own trusted factory — leavesBaseUriForImagesnull even on the happy path.GetDefaultFontInfokeeps its pre-existing graceful?.fallback forStyleDefinitionsPartrather than asserting non-null, matching the method's own designed behavior.HtmlToWmlCssApplier.cs(already nullable-clean from fix: annotate nullable reference types in the CSS layer of the HTML->DOCX family #661): small ripple fix — threesettings.SectPraccesses now assert non-null now thatSectPritself became nullable in this PR.CLAUDE.md: warning-baseline prose updated — 4 legacy#nullable disablefiles remain (down from 6), library baseline 117 (was 119), test project baseline 693 (was 689, verified stable across three independent measurements including a clean rebuild).Test plan
dotnet build Docxodus/Docxodus.csproj --no-incremental— 0 errors, 0 warnings in the four touched filesgrep -l "^#nullable disable" Docxodus/*.cs— none of the 4 target files remaindotnet test --filter "FullyQualifiedName~HwTests"— 232/232 passedDocxodus.Testssuite — 3923 passed, 3 skipped, 0 failed./scripts/build-wasm.sh— succeeds, within the wire-size budgetdotnet build Docxodus.sln— 0 errors, 117 warningsCloses #647
🤖 Generated with Claude Code
https://claude.ai/code/session_01VSnwkK1Nx6zZb2RnnoxKdx