Skip to content

Soundness regression from #225: empty-Dispose exemption trusts source emptiness, but IL weaving (Janitor.Fody) adds real cleanup — 263 silent exemptions in ClosedXML (+ explicit-interface Dispose coverage gap) #238

Description

@PhysShell

Found by: the post-#232/#233 re-measure (docs/notes/precision-remeasure-2026-07-11.md, PR #235, task by the sweep agent). ClosedXML delta came out −265 instead of the expected −5 — only 2 of those are legitimately-removed Slice points; 263 are silent, unsound exemptions.

(a) SOUNDNESS REGRESSION — source-empty Dispose() is not a runtime no-op under IL weaving

HasEmptyDisposeBody (#225 / PR #233) proves emptiness on the source syntax tree. ClosedXML's XLWorkbook.Dispose() is empty in source because Janitor.Fody weaves the real cleanup into it at compile time (the project carries FodyWeavers.xml with <Janitor/>). The exemption therefore silences every undisposed wb/workbook local across ClosedXML.Examples — 263 findings gone that were real (or at least honestly warnable) yesterday.

This inverts the analyzer's core doctrine: the worst case of an exemption must be "keeps today's honest warning", never "silently swallows a leak class".

Fix direction (primary): narrow the gate to the motivating shape

The FP class that justified #225 was enumerators (IEnumerator requires IDisposable; trivial implementations stub it). Restrict the empty-Dispose exemption to types that implement IEnumerator/IEnumerator<T> (semantic check on the type's interface list). XLWorkbook and every other weaver-augmented domain type falls outside the gate; Slice.Enumerator/ReverseEnumerator stay covered. A broader "any empty Dispose" claim needs a proof that no post-compile weaving applies, which source-level analysis cannot give.

Fix direction (optional hardening, if cheap)

If the extractor can see a FodyWeavers.xml next to/above the analyzed sources, disable the empty-Dispose exemption for that compilation entirely (defense in depth; do NOT rely on this alone — weaving can be configured in ways we won't see).

(b) Coverage gap — explicit interface implementation not recognized as empty

void IDisposable.Dispose() { } (explicit interface implementation, as on Slice.Enumerator) is not matched by HasEmptyDisposeBody, so only ReverseEnumerator (plain public void Dispose()) was exempted (−2 of the expected −5). Recognize the explicit-interface form as an empty Dispose — inside the narrowed (a) gate. The #233 DisposeAsync guard must keep applying.

Acceptance

  • Sample additions: an enumerator with an explicit-interface empty Dispose (silent), a non-enumerator type with an empty source Dispose (flagged — the XLWorkbook shape), existing controls intact; CI assertions both ways in flat and --flow-locals modes.
  • Full-sample-set extractor diff vs current main: the ONLY changes are the two intended ones above.
  • Re-measure ClosedXML on the fixed main: delta vs pre-fix(extractor): exempt locals of user types with a provably-empty Dispose() (Closes #225) #233 baseline must be exactly −5 (the five Slice.cs points), with the 263 findings restored.

Refs: #225, PR #233, PR #235 (re-measure note), docs/notes/field-notes-patterns.md entry 19.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions