Conversation
Mearman
force-pushed
the
feat/100-percent-mutation-rtf-codec
branch
5 times, most recently
from
September 14, 2026 12:20
1a5029d to
9168d90
Compare
… utilities base64.ts, bytes.ts, group.ts, list-id.ts, and diagnostics.ts each had no dedicated test file of their own and were only exercised indirectly through the read/write pipeline, leaving boundary conditions (base64 padding for every length-mod-3 case, the base64/hex invalid-character paths, chunked ASCII conversion across its 8192-byte boundary, rtfBytesFromLatin1's exact error message, group-matching at an unbalanced or sibling-adjacent close, groupHead's \* ignorable marker against an unrelated control symbol, every numId mint/parse edge, and every diagnostic error class's own name/code/message/fields) untested at the unit level.
… codepage decode Every SINGLE_BYTE_PAGES entry is exactly 128 characters covering 0x80..0xFF in full, so table[byte - 0x80] can never be undefined for a byte in that range and the "?? �" fallback the bracket read's own string | undefined type demanded was dead code. Switching to table.charAt(byte - 0x80) (a total string method with no undefined case to guard) removes the fallback outright rather than papering over an index the noUncheckedIndexedAccess type checker cannot itself prove is always in bounds -- the identical charAt-over-bracket-read convention bytesToBase64 already documents in base64.ts. Also covers the read side directly: an empty input, \ansicpg65001's UTF-8 path, the exact unsupported-codepage diagnostic message, and the DBCS decode's own undefined-trail-byte and no-single-byte-extras-table fallbacks, none of which had a dedicated assertion before.
…okenize.ts isAsciiLetter's 'A'/'Z'/'a'/'z' range edges and hexDigitValue's uppercase hex-letter branch had no direct assertion: every existing control-word fixture used only lowercase names, and every \'hh fixture used only lowercase hex digits, so a boundary shifted by one letter in either direction produced no observable difference in any existing test.
… side-to-word lookup CellBorderSide is an exhaustive four-member union, so the control word for a given side can never fail to resolve -- the prior find() over CELL_BORDER_SIDES plus its "not found" branch returning "" was dead code that only existed to satisfy the Map's own possibly-undefined return type. CELL_BORDER_SIDE_WORDS is CELL_BORDER_SIDES' own inverse, typed as a total Record over CellBorderSide, so the lookup is direct and the unreachable branch is gone.
…ing brace
The five-byte "{\rtf1" magic check needs every byte to match, not merely the
first -- this covers a candidate that opens with the same brace and then
diverges immediately, which the existing cases (no brace at all, or a ZIP
signature) didn't isolate.
…ed functions applyCellDefinitionControlWord, resolveBorder, resolveCellFill, borderControlWords, and cellFillControlWords were previously exercised only indirectly through full RTF round-trips in read.test.ts/write.test.ts, which left most of their individual branches -- each merge flag, each border descriptor keyword, the shading boundary cases, the pattern-fill colour omissions -- with no isolating assertion of their own. This calls each function directly with the specific inputs its own branches need, including the tie-break case (shading exactly halfway between two percentN steps) and the brdr/brsp prefix-vs-suffix distinction the control-word fallback depends on.
…s already make redundant readLengthPrefixedAnsiString, readObjectHeader, skipPresentationObjectHeader, and skipClipboardFormatHeader each threw a custom "ends before its X field" error ahead of a DataView.getUint32 read that already throws a RangeError for the identical out-of-bounds condition -- readEmbeddedObjectData's shared catch treats both identically, so the explicit checks added nothing beyond a different error message nobody reads. readEmbeddedObjectData's own NativeDataSize check was the same shape. Removed all of them; kept the two checks that are genuinely load-bearing (NativeData and PresentationData's own subarray bounds, since Uint8Array.subarray silently clamps rather than throwing). Also drops readLengthPrefixedAnsiString's decoded string value entirely: every caller (readObjectHeader, skipPresentationObjectHeader) only ever used the returned offset to keep walking the structure, never the decoded ClassName/TopicName/ItemName text itself, so decoding it was pure dead work. The function now returns just the next offset, and the now-unused ANSI decoder and per-character ASCII validation in writeLengthPrefixedAnsiString (every caller passes one of this module's own fixed ASCII constants, never caller-supplied text) are gone with it.
…remaining boundary checks
Adds byte-level assertions the existing round-trip tests never made: the
full ObjectHeader ClassName content ("Package", not merely its length),
every field of writeMinimalDib's own DeviceIndependentBitmap Object, and
off-by-one boundary cases for NativeDataSize and PresentationDataSize
(exactly one byte more than actually remains, rather than a wildly oversized
value). Also covers the optional envelope fields (anchorRow/anchorColumn/
offsetXPt/offsetYPt/source) being omitted entirely rather than merely
undefined when the source embed carries none of them, and a NativeData
payload that decodes to a non-object JSON value (null, an array, a bare
string, a bare number) instead of the record ContentEmbeddedObjectSchema
expects.
…l, not decorative NUMID_PATTERN's (\d+) capture group is required, so a genuine regex match can never actually leave match[1] undefined -- but TypeScript still types every numeric index into a match array as possibly undefined, since it has no way to encode "always present for a required group". The existing guard had no test that could ever exercise it, since no real input reaches that branch. Adds a test that mocks RegExp.prototype.exec for one call to force the otherwise-impossible case, confirming the guard actually returns undefined rather than only existing to satisfy the type checker.
…t8Array already makes redundant asciiStringFromBytes and rtfBytesFromLatin1 both looped with an explicit start/index < length bound whose off-by-one mutation (<= in place of <) is unobservable: Uint8Array.subarray already clamps a past-the-end range to empty on its own, and writing past a Uint8Array's own length is already a silent no-op, so one extra iteration at the boundary produced no output either way. rtfBytesFromLatin1's index increments by exactly 1 per iteration, so its bound is now index !== source.length -- exactly equivalent to < here, but an off-by-one mutation of !== (=== in its place) stops the loop from running at all instead of surviving unobserved. asciiStringFromBytes advances by ASCII_CHUNK_SIZE instead, so the same trick isn't safe there (a chunk size that doesn't evenly divide the input could skip past an exact !== match); it now loops until subarray hands back an empty chunk, the one condition that actually distinguishes "more bytes remain" from "done".
…roupEnd index < tokens.length's own off-by-one mutation (<= in place of <) was unobservable: index increments by exactly 1 per iteration and the optional chain on tokens[index]?.kind already treats a past-the-end read as neither a groupStart nor a groupEnd, so one extra boundary iteration changed nothing. index !== tokens.length is exactly equivalent here, but an off-by-one mutation of !== stops the loop from running at all instead of surviving unobserved.
…d impossible fallbacks decodeCodepageBytes's leading input.length === 0 check was dead weight: the UTF-8 TextDecoder, the DBCS state machine, and the single-byte for-of loop below all already produce "" on their own for zero bytes, with no codepage lookup or diagnostic ever triggered along the way, so the dedicated early return changed nothing any test could observe. decodeDbcsBytes's own trailTable[trail] and singleByteExtras[byte - 0x80] bracket reads each carried a `?? "diamond question mark"` fallback for a `string | undefined` type that can never actually be undefined: both tables are dense, fixed-length strings per their own generation header comment (256 and 128 characters), so any in-range index always returns a real character. Switched both to charAt, which returns a plain string even past the end, removing the unreachable fallback the same way decodeCodepageBytes's own single-byte loop already does for SINGLE_BYTE_PAGES. Also drops the now-redundant while (i < input.length) bound on decodeDbcsBytes's own loop: i's step varies between 1 and 2 bytes, but a Uint8Array read at or past its own length already returns undefined rather than throwing, so the existing byte === undefined check is already the one real stopping condition.
…UTF-8 and DBCS paths too The existing empty-input test only exercised code page 1252 (the single-byte path); the UTF-8 TextDecoder and the DBCS state machine each have their own independent route to producing "" for zero bytes, worth proving separately now that decodeCodepageBytes no longer special-cases length 0 up front. The DBCS case also confirms no diagnostic fires for it.
…tead of re-looking them up close() took a key and re-fetched its entry from the open map, guarding against a "not found" case that could never actually occur: both call sites already derived key from iterating open itself, so the entry was always present. Passing the entry through directly removes the dead guard along with the Map.get call it existed only to check.
…d functions bookmarkAnchorDescriptor, bookmarkResidueControlWords, isBookmarkAnchor, hasRevision, provenanceDescriptors, isoFromDttm, dttmFromIso, formFieldControlType, formFieldContentControl, and coalesceRunConstructs were previously exercised only indirectly through full RTF round-trips in read.test.ts/write.test.ts. This calls each function directly with the specific inputs its own branches need: every DTTM boundary (day/month/year at their exact limits either side), the FFDataBits undefined-result sentinel falling through to a default, a dropdown's own bounds-checked selection index including a genuine sparse-array hole, and coalesceRunConstructs' own sort tie-break when two extents open on the same run but close at different ones.
index < rtf.length's own off-by-one mutation was unobservable: index's step varies (1 or 2 characters per iteration, since an escape sequence consumes two), but a string index at or past its own length already reads back undefined rather than throwing, and undefined matches none of the character branches below -- so one extra boundary iteration always changed nothing. Replaced with the same character === undefined stopping condition decodeDbcsBytes already states for the identical shape of loop in ../codepage.ts.
The brace-counting helper itself had no test of its own, only indirect
exercise through write.test.ts's use of it to check writer output. Covers
the escape-recognition rules directly: a literal \{/\} pair not miscounted
as a real delimiter, an escaped backslash (\\) not swallowing the real
brace that follows it, an unrelated control-word backslash (\b) not
absorbing the brace after it, and a lone trailing backslash with nothing
after it.
… that never change the result readLengthPrefixedAnsiString's length === 0 early return produced the exact same value as its own fallthrough (offset + 0 is offset), so the dedicated case was dead weight. writeObjectHeader wrote TopicName and ItemName as explicit zero bytes even though they are always empty and `out`'s own zero-initialization already supplies those bytes -- only their length needed accounting for, not the write itself. writeLengthPrefixedAnsiString's own copy loop now bounds on index !== value.length: it increments by exactly 1 per iteration, so this is equivalent to < but, unlike <, an off-by-one mutation of it stops the loop rather than surviving unobserved. isRecord drops its Array.isArray exclusion: it is only ever called on a JSON.parse result within withoutInvalidSource, and a JSON array can never carry a "source" own-property to strip (JSON.stringify only serialises an array's numeric indices), so treating an array as record-shaped here changes nothing -- ContentEmbeddedObjectSchema.safeParse still rejects it for not being an object.
… missing Package stream Adds the FormatID/ClipboardFormat wrong-value cases the existing truncation tests didn't isolate (a field with an incorrect value but otherwise well-formed framing, not one that never arrives at all), a direct check that the written Package stream's own sourcePath/tempPath decode to empty strings rather than arbitrary filler, and a compound file that parses fine but carries no stream literally named "Package".
… and exact border output Two gaps the existing assertions didn't isolate: the clvertalc/clvertalb cases only checked the resulting verticalAlign, never the function's own return value, so a mutant forcing that return to false went unnoticed; and the colour-control-word test only checked for the substring's absence/presence, which still passes even if the "no colour index" branch returned arbitrary text instead of the empty string it's actually meant to produce. Both now assert the complete output.
…when trail is undefined trail is only ever undefined when i + 1 is already past input's own end, meaning i was already the last valid index -- advancing by 1 or by 2 from there both land past input.length either way, so there is no real pair left to skip over by always advancing the full 2. The conditional step only existed to look correct; it never changed which iteration the loop stops at.
…onstructs' sort test The existing tie-break test only covered two extents sharing the same startRun, which a corrupted comparator can satisfy by accident once startRun differs (the first comparison term already short-circuits before the tie-break term is ever evaluated). Adds an overlapping-extent case where the shorter extent closes -- and is pushed onto the result array -- before the longer one, so raw insertion order is the reverse of the required startRun order and only a comparator that actually compares startRun correctly produces the expected sort.
…nd vestigial textStart tracking Each of the four cursor < input.length checks in scanControlWord guarded a comparison that already handles running off the end safely on its own: isAsciiLetter/isAsciiDigit against input[cursor] ?? 0 (0 matches neither), and input[cursor] === MINUS / === SPACE against a genuinely undefined value (never equal). One extra out-of-range check at each site changed nothing a test could observe. pushTextByte's own textStart === -1 guard was similarly dead: textStart's value is only ever compared against -1 (flushText's "is a run pending" check), never read as an actual byte offset back into input, so unconditionally overwriting it on every call is exactly equivalent to only setting it the first time.
…n0, and boundary fallbacks Adds the cases the existing suite left untested: a bare LF (no CR) and a bare CR (no LF) each collapsing into \par on their own, an LF-then-CR pair NOT collapsing (only CR-then-LF does), a lone minus sign with no digits after it reverting to text, a trailing backslash with nothing following it at all, a byte just past hexDigitValue's own a-f/A-F ranges falling back to an ordinary control symbol, \bin0 not being treated as a binary run (N must be strictly positive), and a parameterless control word carrying no param property at all rather than an explicit undefined one.
… partial colour entry subject, keywords, and operator (creator) were never exercised at all -- only title/author had a test. Adds them, plus a keywords list with an empty entry from a doubled delimiter, an entirely absent field staying absent rather than an empty string, and a colour table entry stating only one of red/green/blue (still a real, defaulted-to-0 colour, not the auto entry, which requires all three to be genuinely absent).
…and bookmark deletion The extent-sort comparator's own first clause compares startIndex by subtraction; a mutant summing the two values instead survived every existing fixture because sectionBlockExtents is always pushed in ascending start order for those cases, so the pre-sort array already matched the correct order and a symmetric, always-positive comparator never triggered a swap. Adds a fixture where a shorter, later-starting extent closes (and is therefore pushed) before a longer, earlier-starting one that encloses it, so the array arrives pre-sorted backwards and only a genuine subtraction-based comparator restores the correct nesting. endBookmark's own openBookmarks.delete(name) call survived because the existing "genuinely deletes" test only counted BOOKMARK_UNPAIRED diagnostics mentioning the bookmark's name, and a missing delete produces exactly one such diagnostic too -- from reportUnclosedBookmarks at the document's own end, since the never-deleted entry is still open there. Asserting the diagnostic's exact message (the bkmkend-with-no-bkmkstart wording, not the still-open-at-end wording) is what actually distinguishes a real delete from a no-op.
…edundant surrogate offset Each of applyPictureControlWord's, applyFormFieldControlWord's, and applyStructureControlWord's own switch ended in a bare default: break; clause. Falling out of a switch with no matching case, and falling out of an explicit but otherwise-empty default clause, land in the identical place -- these three functions' own end -- so the clause was pure no-op ceremony a mutation tester could never distinguish from its own removal. Dropped rather than documented as equivalent. The \uN reader's own code < 0 ? code + 0x1_00_00 : code ternary was undoing RTF's own "subtract 65536 for a negative code point" encoding by hand, but String.fromCharCode's argument coercion (ToUint16) already reduces any integer modulo 2**16 before treating it as a UTF-16 code unit -- fromCharCode(-4064) and fromCharCode(-4064 + 65536) are the same call. The conditional never changed fromCharCode's own output for any input, so it is replaced with a bare fromCharCode(code).
… own three clauses
The main token loop's own unconditional trailing flushBytes() call --
reached once the token stream is exhausted, past every other event
that could flush pending ANSI bytes on its own -- had no fixture where
input ends with buffered text and no closing brace to flush it first.
Without that call the trailing text is silently dropped: it never
reaches appendText, so the paragraph endSection force-closes comes out
empty instead of holding it.
assertRtfHeaderPresent's own three-clause OR chain had only one
fixture ("not rtf at all"), which trips its first clause alone -- the
second and third clauses independently evaluate true on that same
input regardless of the first, so neither was actually exercised on
its own. Adds a malformed-but-braceless "\rtf1\rtf1" fixture (isolates
the first clause: two bare control-word tokens with no leading brace at
all) and a properly-braced-but-wrong-name "{\ansi not rtf}" fixture
(isolates the third clause: brace and control-word shape both correct,
only the name is wrong).
closeTable's sole call site always derives columnCount as Math.max(this.tableColumnRights.length, ...), so columnCount can never be smaller than tableColumnRights.length itself. A slice bounded by columnCount can therefore never truncate the array it iterates, making rights.slice(0, columnCount) equivalent to iterating rights bare. Dropped rather than documented as equivalent.
endParagraph's own resolveBookmarkPositions call already flushes closingBookmarks with the correct inTable value whenever it actually runs -- but its own force=false early return skips that call entirely for an otherwise-empty trailing paragraph (zero runs). A \bkmkend that is the ONLY thing in such a paragraph, immediately before \cell, is therefore left pending until endCell's own explicit flushClosingBookmarks(true, ...) call two lines later -- the one remaining place that still resolves it. The existing "splices a bookmark closed inside a cell" fixture never reaches that path (its own bookmark closes in a paragraph that also flushes via resolveBookmarkPositions first, leaving closingBookmarks already empty by the time endCell's explicit call runs), so a wrong hardcoded argument there went unnoticed. Adds a fixture where \bkmkend is the paragraph's only content.
…lter Every entry in pendingRunConstructs is pushed with endRun set to this.runs.length at that exact moment (endBookmark's own same-paragraph branch, endFormField), and runs.length only ever grows between then and this call -- more text can follow within the same paragraph, but nothing ever shortens it -- so a stale, too-large endRun can never occur by construction. coalesceRunConstructs draws its own endRun values from indices into runProvenance, which is pushed in lockstep with runs (flushRun always pushes both together), so the identical bound holds for it too. The filter could never remove anything a real call actually produces, so it is dropped rather than documented as equivalent.
… reversed push order The existing "sorts by start position" fixture builds its two extents entirely from coalesceRunConstructs, which already emits them in run order -- so the pre-sort array there is already correctly ordered and never exercises the sort's own comparator logic; a broken or removed sort would pass it unnoticed. Adds two fixtures that force a genuinely reversed pre-sort array, the same technique already used for the block-extent sort: a nested bookmark pair, where the inner one's own endBookmark call (and so its own push into pendingRunConstructs) fires before the outer one's, despite the inner having the LATER startRun; and a bookmark plus a coalesced revision extent sharing one startRun, where pendingRunConstructs is always spread into the array before coalesceRunConstructs' own output regardless of which one's numeric range is actually smaller. Both catch a comparator whose clauses summed instead of subtracted (the result is then order-independent and never triggers the needed swap) and a bare sort() call being dropped outright.
…own calls The existing "flushes a run still pending" fixture opens \objdata before \result and gives it valid, decodable bytes -- \objdata's own successful decode calls addBlocks, whose own flushRun call already pushes the pending run before \result ever opens, leaving beginResultScratch's own flushRun call with nothing left to do. Restoring captured accumulator state round-trips pendingRunText unchanged either way, so the missing call was invisible to a plain "is the text still there" check. Adds a fixture with an empty \result and no \objdata at all, where no other call ever flushes the pending run, so the two are only ever kept apart by beginResultScratch's own call -- observable as run count/merging once identically-formatted text follows \object's own close. The existing "closes an open table" fixture ends \result's own table with an explicit \par outside it; that \par's own endParagraph call already closes the table via its own internal, para.inTable-gated closeTable() call, leaving endResultScratch's own trailing call redundant for that shape. Adds a fixture where \result's content ends on \row itself, with para.inTable still true and no \par to trigger that internal call -- endResultScratch's own explicit closeTable() is the only thing left that can still turn tableRows into a real block.
paragraphSerial's own += and resolveRows' own column += are each provably equivalent under -=: every reader of either value compares it for equality alone (same-paragraph, matching column) against a value captured earlier from the identical monotonic-in-one-direction sequence, never for ordering or against a literal -- a uniform sign flip preserves every such equality and inequality for any input. The root GroupState's own pictureOwner/objectDataOwner/objectOwner/ isFieldGroup all default to false, and each is only ever read paired with a sibling field (picture/objectData/object/field) that stays undefined on root and can only become defined on a freshly cloned child in the same branch that also sets its own paired flag true -- making the flag's own value unreachable through root for any input. Each is explained in place rather than left for a mutation tester to flag as unkillable, per the campaign's own rule that a documented, proven-irreducible equivalent mutant is a legitimate last resort.
…Rows' own scan loop The scan loop's own upper bound (next < rows.length) is equivalent under <=: a JavaScript array read past its own length is undefined rather than a thrown error, and the very next line's own ?./?? -1 already turns that undefined into the same matchIndex -1 an ordinary out-of-range lookup produces, immediately breaking the loop either way. columnIndices[next]'s own ?? -1 fallback is unreachable, not a defensive guard against a real case: columnIndices is built by rows.map(...) directly above, so columnIndices.length === rows.length always, and the loop's own bound already guarantees a defined entry for every next this line sees. The fallback exists only to satisfy noUncheckedIndexedAccess's own typing, never to handle a case that can occur.
Four gaps in the existing rowSpan/continuation fixtures: The verticalMergeContinuation early return was only ever exercised against an already-empty continuation cell (no text typed in the source), so a fallthrough that kept cell.blocks instead of discarding it produced the identical [] result by coincidence. Adds a "stray" run inside the continuation cell so a genuine discard is observable. The scan loop's own verticalMergeFirst guard was never tested against a plain, unmerged cell sitting directly above a real (if malformed) \clvmrg continuation at the same column -- an unconditional scan would wrongly extend that ordinary cell's rowSpan to 2. The existing "exactly two, not three" fixture has only one real continuation row, so a scan loop stepping backward instead of forward (reaching the anchor's own row again, which is never itself a continuation, and breaking there) produces the same rowSpan by coincidence. Adds a second continuation row so a reversed direction undercounts. matchIndex's own -1 sentinel check was only ever exercised with a real match at array position 0, so mistaking a genuine matchIndex of 1 for "not found" went unnoticed. Adds a fixture where the anchor is the second cell of its own row, placing a genuine match at position 1.
…call The existing "never appends an empty block list" fixture uses a picture that fails to decode, but buildPicture returning undefined is guarded by its OWN `if (image !== undefined)` check at the call site -- addBlocks is never even called there, so its own guard and flushRun call were both entirely unexercised by it. objectState.resultBlocks is the one real call site that can pass a genuinely empty array (an \object whose \result had no content). Adds a fixture using \shppict (a "body"-kind destination, not a fresh \result scratch) to type text directly into the outer paragraph's own pendingRunText AFTER \result has already closed and restored state, so it is still genuinely pending at the exact moment \object's own close calls addBlocks with that empty array -- a guard-less call would flush it regardless. Adds a second fixture for the same function's non-empty path: a successfully-decoded picture between two runs, proving flushRun's own call keeps them separate rather than letting the trailing text merge across the spliced-in image block.
endSection's own drop condition (blocks.length === 0 && this.sections.length > 0) can only ever skip pushing when sections.length is already at least 1 -- its second operand is false whenever sections.length is 0. finish() calls endSection exactly once, so that single call is unconditionally guaranteed to leave sections.length at least 1 regardless of what it was beforehand: the fallback was guarding against a state this call can never actually produce. Dropped rather than documented as equivalent.
…ype key omission
The existing "keeps the document's only section" fixture only checks
blocks, never the boundary that actually matters: endSection's own
drop condition's second clause (sections.length > 0) is what makes an
empty-but-FIRST section still get pushed here rather than silently
skipped, and a ">= 0" in its place is always true regardless of count
-- indistinguishable from "> 0" on a fixture that never states a
breakType. Adds a fixture whose only, empty section states \sbknone,
checking breakType survives -- finish()'s own fallback (now removed)
never carried it, so this also proves the push came from endSection
itself.
Adds a fixture for the breakType conditional spread's own omitted-key
case: a plain toEqual cannot tell an absent key apart from one present
with value undefined, so Object.hasOwn is what actually distinguishes
a real conditional spread from an unconditional { breakType:
section.breakType } that would leave the key present but undefined.
…ir exact message text Neither an \object with two \result children nor one with two \objdata children had any coverage at all -- the isDuplicateResult and isDuplicateObjectData guards, and their own EMBEDDED_OBJECT_UNREADABLE diagnostics, were entirely unexercised. Adds one fixture per case, checking both the diagnostic's exact message and, for the duplicate \result case, that only the first one's own fallback content survives into the document. UNKNOWN_DESTINATION_SKIPPED and CONTENT_DESTINATION_SKIPPED were only ever asserted by diagnostic code, never by message text, so a template-literal mutant that emptied either message string went unnoticed. Adds exact-message assertions to a new fixture and to the existing footnote fixture.
…d guard isResultDestination's own definition already asserts objectState !== undefined (head.destination === "result" && objectState !== undefined), so a second, separate objectState !== undefined check ahead of it can never be false when isResultDestination is true -- equivalent to bare isResultDestination alone, exactly like the sibling isDuplicateResult check two lines above whose own comment already states the identical reasoning. TypeScript's aliased-condition narrowing carries the non-undefined fact through to the block's own objectState reads the same way it already does for isDuplicateResult.
…cture recognised destination
The existing "never initialises picture/embedded-object state" fixtures
both use {\b bold}, which has no recognised destination of its own at
all (known === undefined) -- they never reach the `if (known !== undefined)
{ ... if (kind === "picture") ... }` branch this is meant to guard,
exercising an entirely different, earlier guard instead. Adds a sibling
fixture per case using \*\bkmkstart, a real, known, non-picture/
non-objectData destination that DOES reach the branch, so a `true` in
either kind check's place is now actually observable as a spurious
UNSUPPORTED_PICTURE_FORMAT/EMBEDDED_OBJECT_UNREADABLE diagnostic.
…n destination guard state.bookmark.name.trim() had no fixture with actual leading/trailing whitespace in the name -- the space right after \bkmkstart itself is consumed as the control word's own terminating delimiter, but a SECOND space before the name (or one before the group's own closing brace) is ordinary #PCDATA and survives verbatim unless trimmed. Adds a padded name, matched against an unpadded \bkmkend so a missing trim breaks the name-keyed match entirely rather than merely leaving whitespace in the output. startFormField's own destination === "fieldInstruction" guard looked structurally redundant against every fixture that nests only same-destination groups inside \*\fldinst (the real reason the guard exists, per its own comment) -- but \*\ud is a real, different, known destination that can genuinely nest there while still sharing state.field by reference. Appending more instruction text after such a nested group closes is what makes the guard's absence observable: a premature trigger at \*\ud's own close locks in a still-partial instruction, so the field's own later, complete (and no-longer- matching) instruction is read back and dropped with a FORM_FIELD_KEYWORD_LOST diagnostic that correct code -- which never opens the extent in the first place -- never produces.
…de any \field group \*\ffname/\*\ffhelptext/\*\ffl are each recognised by DESTINATION_KINDS regardless of what encloses them, so a hostile or truncated producer's own stray occurrence outside \field reaches emitText with state.field inherited from the root -- genuinely undefined, never set by anything else. Each of the three branches' own field?.formField !== undefined guard is what keeps that case a silent discard (matching the trailing comment on the whole if-chain, "every other unhandled destination already does") rather than a thrown TypeError; tsc confirms the gap directly (removing the guard leaves six "possibly undefined" errors across the three branches), and the mutation itself reproduces as a real runtime crash, not just a type-checker complaint.
…ng it Every other table fixture in this file follows its own \row with an explicit \pard, which resets para.inTable to false on its own via defaultParagraphState() -- masking whether \row's own case doing the identical reset accomplishes anything. Adds a fixture with text typed directly after \row and no \pard in between, which depends entirely on \row's own reset: without it, that text stays routed into the now-closed table's own cellBlocks instead of the section's real ones.
…llthrough as equivalent Each of applyCharacterControlWord, applyCellDefinition, applyParagraphControlWord and applySectionControlWord's own early return, if removed, would let a name the dispatcher already fully handled fall through to the next one down. Every one of the five dispatchers' own recognised name sets (character, cell-definition, paragraph, section, structure) is disjoint from all the others -- no control-word name this reader recognises appears in more than one -- so a name any earlier dispatcher already matched can never also match a later one, making every such fallthrough a genuine no-op for any input regardless of whether its own early return fires.
code !== undefined had no fixture for a malformed \u with no digits at all. String.fromCharCode(undefined) coerces via NaN's own ToUint16 result to U+0000, so calling it unconditionally would silently insert a stray NUL character into the run rather than emitting nothing. skipUnicodeFallback still runs either way, consuming the one ANSI fallback character \uN's own grammar always requires -- proving the fallback text itself was never the thing standing between correct and mutated behaviour here.
indexOf's own "not found" sentinel (-1) is not a real property on any plain array, so definitions[-1] already evaluates to undefined on its own -- the exact same result the explicit "matchIndex === -1 ? undefined : ..." ternary special-cased by hand. Indexing by matchIndex directly produces the identical outcome for both the found and not-found cases, so the special case is dropped rather than documented as equivalent.
…rmField state by reference Both a bookmark name and a formField dropdown entry are accumulated through state shared by reference across a nested group, keyed only on that state's own definedness rather than on the current group's own destination. A \listtext group nested inside \*\bkmkstart or the first \*\ffl entry inherits that shared state without genuinely being a bookmarkStart/bookmarkEnd or formFieldListItem destination, so these fixtures prove the destination check on each guard is load-bearing rather than redundant.
…ent mutants The formFieldListItem text-accumulation guard's own current !== undefined check, and the main token loop's own index < tokens.length bound, both survive their own forced-true/<= mutation because the surrounding code already guarantees the branch they guard can never actually be taken: a *\ffl group's own open always pushes an empty listItems entry in the identical branch that sets this destination, and tokens[index] reads as undefined past the array's own end, which the loop's very next line already treats as its own break condition.
Mearman
force-pushed
the
feat/100-percent-mutation-rtf-codec
branch
from
September 14, 2026 22:31
143a714 to
0c60868
Compare
…n opportunities in read.ts Replaces paragraphSerial's incrementing number with a fresh symbol per paragraph, since every reader only ever compares it for identity and never for ordering -- removing the AssignmentOperator mutation (+= vs -=) that a numeric counter left behind with no real behaviour to distinguish. Rebuilds resolveRows' own column-position tracking from a running width total (vulnerable to the identical += vs -= equivalence) into a count of pushed placeholder slots, and rewrites its vertical-merge scan's own loop bound as `next <= rows.length - 1` instead of `next < rows.length` -- algebraically identical, but this phrasing's own single mutation (<= to <) is a genuine one-row-short bug an existing three-row test already catches, rather than a harmless extra pass past the array's own end. Extracts closingBookmarkExtent, appendToLastListItem, and verticalMergeRowSpan out of ContentBuilder as standalone, exported functions, each keeping the same fail-loud invariant check the inlined code had, but now directly unit-testable with a deliberately invariant-violating input instead of leaving an unreachable throw branch with no test able to reach it. Deletes the redundant `head.destination === "objdata"` conjunct next to `known === "objectData"` (DESTINATION_KINDS maps exactly one key to that kind, so the second clause alone already states it), and drops the `kind === "bookmarkStart" || kind === "bookmarkEnd"` guard around assigning a fresh bookmark onto a newly opened child group -- every reader of state.bookmark already pairs it with its own destination check, so assigning it unconditionally changes nothing observable and also makes the group-close handler's own bookmarkStart/bookmarkEnd branch pair a real, killable check instead of a no-op. Replaces the token-processing loop's own `index < tokens.length` condition with `while (true)`, since the very next line's `if (token === undefined) break;` already terminates it on exactly that condition. Makes pictureOwner/objectDataOwner/objectOwner/isFieldGroup optional on GroupState and omits them from the root group's own literal instead of stating a `false` every reader already reads as paired with the sibling field (picture/objectData/object/field) also being undefined on root -- so there is no real `false` bit for a mutation to flip, only a field a mutation can't meaningfully touch because it isn't there.
…ly, drop their boolean returns applyControlWord's own final five-way dispatch (character, cell definition, paragraph, section, structure) gated each call behind an `if (dispatcher(...)) return;`, but the five dispatchers' own recognised control-word names share not one name across all five sets -- so a name any one of them recognises can never also be one a sibling would act on, making the early-return gating a no-op chain regardless of order. Calling all five unconditionally, in the same fixed order, produces the identical result without threading a boolean nothing outside this one call site ever reads. applyCharacterControlWord, applyParagraphControlWord, applySectionControlWord, and the ContentBuilder.applyCellDefinition wrapper (the underlying applyCellDefinitionControlWord keeps its own boolean contract, tested directly on its own terms) all become void accordingly, each keeping its own internal early exits where they still gate real behaviour (e.g. applySectionControlWord's own "no parameter, nothing to apply" check) rather than merely reporting a match back to a caller that no longer asks.
…tes against a shared reference Adds direct coverage for four of applyControlWord's own early gates whose OTHER operand is a field shared by reference across every descendant group (picture, object, bookmark, formField), each demonstrating a forced-true destination check would misroute a control word arriving from a sibling destination into the wrong handler: a \bkmkcolf1 nested inside \pict, an \objw1440 nested inside \object via a sibling bookmarkStart, an arbitrary parameterised control word inside bookmarkStart masquerading as \bkmkcoll, and a genuine \ffprot arriving from \*\ffname rather than \*\formfield itself. Each was verified against a temporarily forced-true copy of its own guard before being kept.
…er \sect's own force=true The two existing \nosupersub tests asserted runs[1]'s verticalAlign directly without first asserting the run count: if \nosupersub failed to clear verticalAlign, "up" and "base" would carry identical character state and coalesce into a single run, making runs[1] undefined and the verticalAlign assertion pass vacuously regardless of what actually happened. Both now assert length 2 first. Adds a test for \sect's own force=true argument to endParagraph, which was previously only exercised in a way indistinguishable from force=false (every existing \sect fixture already had real paragraph text before it): a bare \sectd\pard\sect with nothing accumulated must still produce one empty paragraph block, exactly as \par's own identical force=true does, rather than the section producing no blocks at all.
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.
Works through rtf-codec's survived/no-coverage Stryker mutants toward a genuine 100% mutation score, restructuring code to remove unreachable or redundant defensive branches where that's the right fix and adding isolating tests otherwise. No Stryker disable comments anywhere in src/ (verified by grep).
Measured baseline: 70.65% of 3275 valid mutants, timeout share 2.1%.
Current overall score: 81.70% (up from the 70.65% baseline), across 3275+ valid mutants.
At or genuinely 100%: cell-format.ts, list-id.ts, group.ts, bytes.ts (both the shared one and test-support's own), codepage.ts, test-support/brace-balance.ts, constructs.ts, tokenize.ts, embedded-object.ts. Several of these needed real restructuring, not just more tests, to remove genuinely equivalent mutants a test could never observe -- e.g. a DIB-writer's zero-value fields already covered by a freshly zero-initialized buffer, a brace-counting helper's escape-detection rewritten around a regex
matchAllso no placeholder string is left for Stryker to mutate harmlessly, and severalnameStart-tracking offsets in header.ts proven fully redundant by collectPlainText's own tolerant per-token skipping.header.ts: 83.66% (up from 75.48%), 66 gaps remaining across the list table, list override table, revision table, and info-group parsers -- the font table, colour table, and style sheet parsers are now either 100% or down to the file's own remaining structural gaps.
Still fully open: read.ts (71.65%, 336 gaps -- survived + no-coverage) and write.ts (78.38%, 181 gaps), the two largest files in the package, not yet started this round.
Left as draft until the full suite is verified at 100%.
pnpm typecheck/lint/testare all green on every commit in this branch.