Prune empty objects via token-deferring writers

[ewoutkramer]

Description

Alternative to #3557 — same bug, different mechanism. Opening this alongside rather than on top of that branch, since it replaces the approach rather than amending it. @andrzejskowronski's diagnosis and test expectations are what this builds on.

The FHIR spec does not allow empty objects or elements, but the serializers open a structure before knowing whether any members will follow. So an empty POCO — or a SerializationFilter that removes everything inside — leaves behind a {} or a <x/>. Our own deserializer rejects that output (OBJECTS_CANNOT_BE_EMPTY, ELEMENT_CANNOT_BE_EMPTY), so summarized resources did not round-trip.

The mechanism

#3557 solves it two ways at once: a predicate (hasContent/hasElementContent/memberHasContent) that re-implements the serializer's emptiness rules for the unfiltered case, and — because a stateful filter can't be dry-run — a buffer-and-splice for the filtered case, serializing each subtree into an ArrayBufferWriter<byte> and inspecting the bytes.

It turns out no buffering is needed at all. The only thing that ever has to be undone is the token that opens a structure: the property name plus the {/[, or the XML start tag. Nothing written after that point ever needs taking back — FHIR JSON has no "write content, then discover it was wrong" case.

So two new internal sealed classes, PruningJsonWriter and PruningXmlWriter, hold the opening tokens back and hand them to the real writer the moment content arrives, discarding them when the matching close comes first. Because the tokens still reach the underlying writer in document order, it keeps track of depth, separators and indentation itself.

What that buys over the buffering approach:

• No duplicated emptiness rules. "Empty" becomes "the serializer wrote nothing" rather than a predicate that must mirror the serializer. Refactor JSON serializer to handle optional and empty objects #3557's hasContent had to mirror it exactly, which is why it deleted the and not DynamicResource { DynamicTypeName: null } guard to keep its Resource => true arm honest (see Copilot's open thread there). Here nothing forced that call — but review settled it the same way: the guard is gone as a deliberate decision of its own (6711377), so an unnamed DynamicResource now writes "resourceType": "DynamicResource", matching the element name Xml has always used for one.
• One code path. The if (filter is null) … else … fork disappears from all three places.
• No indentation gymnastics. No dummy [-prologue written into a buffer to coax the right indent, no slicing it back off by scanning for the first {, no WriteRawValue(…, skipInputValidation: true), no writer.CurrentDepth + (wroteStartArray ? 0 : 1) in callers. The _elementName indentation bug fixes itself as a consequence rather than by construction.
• It also subsumes the two hand-rolled lazy-array loops (wroteStartArray + numNullsMissed, twice) that kept elementName/_elementName arrays index-aligned, via the same deferral applied to null placeholders.
• Identical filter callbacks. A hasContent pre-check skips EnterObject/LeaveObject for pruned subtrees; deferral preserves today's exact Enter/TryEnter/Leave sequence, so there is no filter-state behaviour to reason about.
• Cheaper: one pass with O(depth) pending state, instead of repeated subtree scans plus an ArrayBufferWriter + Utf8JsonWriter allocation per complex member.

Net effect on BaseFhirJsonSerializer: 163 lines changed, −36 net, and serializeInternal goes back to straight-line writing.

Also included: the class-mapping lookup is hoisted above the first write in both serializers (so a failure can't leave a half-open structure), and a leftover Console.WriteLine(e) is removed from BaseFhirXmlSerializer.serializeMemberValue. The writers hand out the underlying writer through a PrepareContent() method: getting it flushes the pending tokens, and a property with that side effect would fire whenever a debugger evaluates it.

Observable changes

All of these were invalid FHIR before:

• Empty complex members and array items are omitted. Array items shift up — there is no valid placeholder for them.
• Indentation of _elementName objects is now correct (same snapshot result as Refactor JSON serializer to handle optional and empty objects #3557).
• XML is fixed too, which Refactor JSON serializer to handle optional and empty objects #3557 does not cover: <contact/> and friends are pruned.
• Error / parse-recovery output no longer echoes empty placeholders: "language": {} and "link": [{}] disappear from the snapshot. The error list still reports what was wrong; only the echo is gone.
• An unnamed DynamicResource writes "resourceType": "DynamicResource" rather than {} — settled in review: Json now matches the element name Xml has always used.

Two asymmetries worth a reviewer's eye:

• XML prunes empty contained resources, JSON does not. In JSON a resource always writes resourceType, so it is never empty; in XML the type name is the element name, so <contained><PatientX/></contained> has genuinely nothing in it. Pinned by a test — say the word if you'd rather force-keep resources in XML.
• A retained source comment counts as content, so an element holding nothing but a comment is kept rather than pruned, and a comment preceding a pruned element stays behind in its parent. Dropping a comment the caller explicitly asked to retain seemed worse than emitting an element the source had in that shape to begin with — and such an element can only be produced from input that already had it, since the summary-filter path never sees comments (the subsetted clone carries no annotations). Pinned by a test.

Pruning deliberately stops at the document root: SerializeToString(new HumanName()) still yields {}, and in XML the root is always written because XmlWriter.WriteEndDocument() throws on a document without one.

Public API

No new public surface — both writers are internal sealed. Both SerializePrimitiveValue overloads keep their signatures on both serializers; they are protected virtual public API and SDK6 is released, so they receive the real writer after pending tokens have been flushed.

Related issues

Alternative implementation of #3557.

Testing

New EmptyStructurePruningTests (16 tests) covering: single and nested empty members collapsing in one pass; separators intact when a member in the middle is dropped (compact and pretty); _elementName omitted when a filter empties it, including when it empties every entry of a _elementName array; _elementName indented at its own depth; primitive-array alignment with leading, trailing and all-empty placeholders; alignment preserved when a filter empties an entry before and after the array opens; root written when empty, including the lone-primitive wrapper; resourceType always written, including for an unnamed DynamicResource; null placeholders kept in complex arrays; empty contained resource pruned in XML but not JSON; XML elements emptied by a filter pruned; XML elements kept alive by nothing but an attribute; retained comments keeping (or outliving) their element.

Updated: FhirJsonSerializationTests.SerializesInvalidData and its XML counterpart; three Verify snapshots (the JSON one is byte-identical to what #3557 produces).

Full runs, all green:

suite	result
Support.Poco	615
Support	500
Serialization R4 / R4B / R5	98 / 93 / 94
Model R4 / R4B / R5 / STU3	187 / 180 / 181 / 133
ElementModel R4 · Specification R4	70 · 824
LongRunner example round-trips R4 / R5	4034 / 4134

The example round-trips are the load-bearing ones here — they exercise both formats over every spec example, including the leading/trailing null placeholders in json-edge-cases.json and narrative/contained-resource comment retention. Full solution builds clean.

FirelyTeam Checklist

• Update the title of the PR to be succinct and less than 50 characters
• Mark the PR with the label breaking change when this PR introduces breaking changes

Not marked breaking: every output change is output that was invalid FHIR and that our own parser rejected. Worth a maintainer's call though — anyone diffing round-tripped invalid input will see a difference, and empty-sibling removal shifts array indices.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the FHIR JSON and XML serializers to omit empty objects/elements by introducing token-deferring “pruning” writer wrappers. This prevents the serializers from emitting invalid {} in JSON or empty elements (e.g. <contact/>) in XML when a POCO (or a SerializationFilter) produces no content, improving round-tripping with the SDK’s own deserializers.

Changes:

• Add PruningJsonWriter / PruningXmlWriter to defer structure-opening tokens until real content is written, dropping structures that remain empty.
• Refactor BaseFhirJsonSerializer and BaseFhirXmlSerializer to use the pruning writers, removing the prior buffer-and-splice approach and ensuring mapping resolution happens before any output is written.
• Add EmptyStructurePruningTests and update existing serialization tests/snapshots to reflect pruning behavior and corrected indentation/output.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Hl7.Fhir.Base/Serialization/PruningJsonWriter.cs	New writer wrapper that defers JSON object/array openings and drops empty structures (with optional null placeholders).
src/Hl7.Fhir.Base/Serialization/PruningXmlWriter.cs	New writer wrapper that defers XML start tags and drops elements that remain empty.
src/Hl7.Fhir.Base/Serialization/BaseFhirJsonSerializer.cs	Switch JSON serialization to pruning writer; simplify empty-structure handling and remove buffering code paths.
src/Hl7.Fhir.Base/Serialization/BaseFhirXmlSerializer.cs	Switch XML serialization to pruning writer; prune empty elements and remove debugging console output.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/EmptyStructurePruningTests.cs	New targeted test suite covering pruning behavior across JSON/XML and filter interactions.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/FhirJsonSerializationTests.cs	Update invalid-data test expectations to reflect omission of empty objects.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/FhirXmlSerializationTests.cs	Update invalid-data test expectations to reflect omission of empty XML elements.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/snapshots/FhirJsonDeserializationTests.SerializingErroneousResource_Should_ThrowExpectedErrors.verified.txt	Snapshot updated for pruned empty structures and formatting/indentation changes.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/snapshots/FhirXmlDeserializationTests.SerializingErroneousResource_Should_ThrowExpectedErrors.verified.txt	Snapshot updated to reflect pruning of empty contained resources/elements in XML.
src/Hl7.Fhir.Support.Poco.Tests/Serialization/snapshots/FhirJsonDeserializationTests.JsonDeserializerHandleUnexpectedObject.verified.txt	Snapshot updated for formatting/indentation changes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[andrzejskowronski]

Had some remarks at first, but after viewing rest of the code it's clear why certain choices are made, so I removed them

• Thought a Queue would fit nicely for the Pending writes - but not with LIFO checks
• JSON passing null PropertyName - while not intuitive, keeps the code overall cleaner

So approving and merging