From c4d8ba4bb96b1fdea063013fe49bd52a3573ddc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 11:42:30 +0000 Subject: [PATCH 1/2] docs: reconcile status notes with shipped #221/#227-229 and #238/#240 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parent issue #221 (self-owned-source exemption too narrow) is closed: all three child shapes shipped and merged (#227/PR#237, #228/PR#232, #229/PR#239), verified against live GitHub state and the extractor source rather than trusted from prior task summaries. field-notes-patterns.md entry 15's status line only listed shapes (a) and (c) as shipped, silently omitting the already-merged (b) — corrected to list all three with their PRs. entry 19 (issue #225's empty-Dispose exemption) never recorded the #238 soundness regression (source-empty Dispose != runtime no-op under Janitor.Fody weaving) or the #240 fix that narrowed the gate to IEnumerator-implementing types — added as an explicitly-marked superseding addendum, keeping the original #225 writeup intact per the notebook's historical-record convention. precision-remeasure-2026-07-11.md's "still open" claim about #219 and its ClosedXML follow-up items are now stale (#219 shipped via PR #236; the two follow-ups became #238/PR#240) — marked as superseded in place, original analysis untouched. --- docs/notes/field-notes-patterns.md | 49 +++++++++++++++++++- docs/notes/precision-remeasure-2026-07-11.md | 21 ++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index a84c4dab..059245b0 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -551,12 +551,21 @@ exemption's real criterion is "does this object's lifetime start and end with th subscriber's" — a base-class accessor to the attached object, or an item of an owned collection, satisfies that just as well as a constructed field.** -**Status (2026-07):** shapes **(a)** and **(c)** shipped. +**Status (2026-07):** all three sub-shapes **(a)**, **(b)**, **(c)** shipped — +parent issue #221 closed once all three landed. - **(a)** the `Behavior.AssociatedObject` self-owned source, shipped in #227 — extractor `IsAssociatedObjectSource`, gated on the `Behavior` base (`IsBehaviorSubscriber`) plus a same-class assignment-chain resolving to `this.AssociatedObject` (`ResolvesToAssociatedObject`); pinned by `frontend/roslyn/samples/AssociatedObjectSourceSample.cs`. +- **(b)** the curated app-scoped source for an `Application`-derived + subscriber, shipped in #228 (PR #232) — extractor keeps the existing + `clsIsApp` subscriber gate byte-for-byte and only loosens the *source* + check to a curated resolver allowlist (`PaletteHelper.GetThemeManager`), + method-group handlers only (no lambdas); see + `docs/notes/oracle-known-fps.md` "What DID ship next to it (issue #228)" + for why this is not the rejected `clsIsStatic` broadening. Pinned by + `frontend/roslyn/samples/AppScopedSourceSample.cs`. - **(c)** the element of a self-populated collection, shipped in #229 — extractor `IsOwnedCollectionElementSource`: a `foreach` loop variable whose collection is a this-owned field/property populated ONLY from own construction / an own factory @@ -694,6 +703,44 @@ existing fixtures that had used an empty `Dispose()` as a modelling shortcut (`UnitOfWork`, `PixelOwner`) were made faithful (a real Dispose body) so they remain genuine leak fixtures. +**Superseded by a soundness regression (issue #238) — gate narrowed to +enumerators (issue #240 / PR #240).** The re-measure in +`docs/notes/precision-remeasure-2026-07-11.md` (PR #235) ran the shipped \#225 +exemption above against real ClosedXML and found it unsound: +`XLWorkbook.Dispose()` is empty **in source only** — `Janitor.Fody` (an IL +weaver declared in ClosedXML's `FodyWeavers.xml`) rewrites the body at +compile time to call real cleanup (`DisposeManaged()` → +`Worksheets.ForEach(w => (w as XLWorksheet).Cleanup())`). `HasEmptyDisposeBody` +read the source-level emptiness as proof of "nothing to release" and +silently exempted 263 unrelated `XLWorkbook` locals across +`ClosedXML.Examples` — a real leak class going quiet, not a false-positive +fix. This inverts the analyzer's core doctrine (an exemption's worst case +must stay "keeps today's honest warning," never "silently swallows a leak +class"), so it was treated as a bug, not a precision tweak. + +The fix (#240) **narrowed** the gate rather than special-casing weavers: +`HasEmptyDisposeBody`'s exemption now applies **only** to types that +implement `IEnumerator`/`IEnumerator` — the one interface that *forces* +a (frequently no-op) `Dispose()` implementation, which was the entire +motivating shape for #225 in the first place. `XLWorkbook` and every other +weaver-augmented domain type fall outside the gate and keep the honest +warning. Defense in depth: a `FodyWeavers.xml` found above the source (or +above the owning project, for linked sources) disables the exemption even +for a qualifying enumerator. Explicit-interface `Dispose()`/`DisposeAsync()` +are now recognized as empty too (closing the coverage gap that left 3 of 5 +`Slice.cs` sites unexempted). Two review rounds closed further soundness +holes: inherited `IAsyncDisposable` (via `AllInterfaces`), inherited bare +`DisposeAsync` (via base-chain walk), a sticky static-registry cache, and a +fail-open weaver-detection path (`File.GetAttributes` instead of +`File.Exists`, which silently disabled the guard when the check itself +errored). Confirmed on real ClosedXML (HEAD `4e89dce`, `--flow-locals`): +121 of the swallowed findings restored, 0 removed. See issue #238 and PR \#240 +for the full record — this is a deliberate historical lesson, not +cleaned up: **source-level emptiness is not a runtime no-op once a +compile-time IL weaver is in play, and a broad "any empty Dispose" gate +cannot rule that out — only a gate scoped to a specific +compiler/language-forced shape (here: `IEnumerator`) can.** + --- ## The through-line diff --git a/docs/notes/precision-remeasure-2026-07-11.md b/docs/notes/precision-remeasure-2026-07-11.md index a26ed2d9..c65c754f 100644 --- a/docs/notes/precision-remeasure-2026-07-11.md +++ b/docs/notes/precision-remeasure-2026-07-11.md @@ -6,8 +6,9 @@ OSS repos: #218-#225. Four follow-up PRs have since shipped fixes: - [#230](https://github.com/PhysShell/Own.NET/pull/230) — Closes **#218** (not #219 — #219 is the separate WinForms `Controls`/`IContainer` disposal - channel gap, still open). DependencyProperty/property-changed old→new - subscription-rotation recognition. + channel gap, still open **as of this note**; shipped later via PR #236, + see `docs/notes/field-notes-patterns.md` entry 12/13). DependencyProperty/ + property-changed old→new subscription-rotation recognition. - [#231](https://github.com/PhysShell/Own.NET/pull/231) — four smaller gaps in one PR: `CommandManager.RequerySuggested` allowlist (#223), `using (field = new T())` release (#220), template-part local/pattern-var @@ -213,3 +214,19 @@ This note's own instructions were read-only with respect to the analyzer — no code was changed to investigate or produce the breakdown above; the breakdown is entirely from comparing old/new SARIF output and reading the target repos' real source. + +## Resolution addendum (2026-07-11 / 2026-07-12) + +Both follow-ups identified above were filed and shipped as a single unit: +issue [#238](https://github.com/PhysShell/Own.NET/issues/238) (soundness +regression — item 2 above, the ClosedXML 263-finding over-exemption) with +the explicit-interface coverage gap (item 1 above) folded into the same +fix, closed by [PR #240](https://github.com/PhysShell/Own.NET/pull/240). +The shipped fix took the narrowing direction (confine the exemption to +`IEnumerator`-implementing types) rather than the weaver-convention-sniffing +heuristic sketched in item 2 — see +`docs/notes/field-notes-patterns.md` entry 19 for the full account, +including why the original "−5 Slice.cs points, 263 restored" acceptance +target from #238 was itself corrected (ClosedXML's own `FodyWeavers.xml` +covers `Slice.cs` too, so the sound outcome is "fully restored, nothing +silently dropped," not a fixed `−5`). From 9d39b8c5052653221cd0541a6103e466b0342594 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 12:00:09 +0000 Subject: [PATCH 2/2] docs: fix precision-remeasure/field-notes wording per CodeRabbit review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two wording inaccuracies flagged on PR #242, both verified against frontend/roslyn/OwnSharp.Extractor/Program.cs HasEmptyDisposeBody: - The #240 gate matches only the generic System.Collections.Generic. IEnumerator (checked by name + TypeArguments.Length == 1 + namespace), not the non-generic System.Collections.IEnumerator. Fixed in both docs/notes/field-notes-patterns.md entry 19 and docs/notes/precision-remeasure-2026-07-11.md's resolution addendum. - DisposeAsync() is not "recognized as empty" — any IAsyncDisposable or bare DisposeAsync (declared, explicit-impl, or inherited) disqualifies the exemption outright. Only explicit-interface synchronous Dispose() is recognized as empty (the actual #238 coverage-gap fix). --- docs/notes/field-notes-patterns.md | 27 ++++++++++++-------- docs/notes/precision-remeasure-2026-07-11.md | 3 ++- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index 059245b0..c1d963a1 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -720,15 +720,22 @@ class"), so it was treated as a bug, not a precision tweak. The fix (#240) **narrowed** the gate rather than special-casing weavers: `HasEmptyDisposeBody`'s exemption now applies **only** to types that -implement `IEnumerator`/`IEnumerator` — the one interface that *forces* -a (frequently no-op) `Dispose()` implementation, which was the entire -motivating shape for #225 in the first place. `XLWorkbook` and every other -weaver-augmented domain type fall outside the gate and keep the honest -warning. Defense in depth: a `FodyWeavers.xml` found above the source (or -above the owning project, for linked sources) disables the exemption even -for a qualifying enumerator. Explicit-interface `Dispose()`/`DisposeAsync()` -are now recognized as empty too (closing the coverage gap that left 3 of 5 -`Slice.cs` sites unexempted). Two review rounds closed further soundness +implement the *generic* `System.Collections.Generic.IEnumerator` — the +one interface that *forces* a (frequently no-op) `Dispose()` implementation +via `IEnumerator : IEnumerator, IDisposable`, which was the entire +motivating shape for #225 in the first place. The non-generic +`System.Collections.IEnumerator` does **not** extend `IDisposable`, so it +does not qualify. `XLWorkbook` and every other weaver-augmented domain type +fall outside the gate and keep the honest warning. Defense in depth: a +`FodyWeavers.xml` found above the source (or above the owning project, for +linked sources) disables the exemption even for a qualifying enumerator. +Explicit-interface `Dispose()` is now recognized as empty too (closing the +coverage gap that left 3 of 5 `Slice.cs` sites unexempted) — `DisposeAsync()` +is the opposite of an exemption: ANY `IAsyncDisposable` (declared or +inherited) or bare `DisposeAsync()` method (declared, explicit-impl, or +inherited from a base) unconditionally **disqualifies** the exemption, since +async disposal means the type is no longer the "simple enumerator holding +nothing" the rule is scoped to. Two review rounds closed further soundness holes: inherited `IAsyncDisposable` (via `AllInterfaces`), inherited bare `DisposeAsync` (via base-chain walk), a sticky static-registry cache, and a fail-open weaver-detection path (`File.GetAttributes` instead of @@ -739,7 +746,7 @@ for the full record — this is a deliberate historical lesson, not cleaned up: **source-level emptiness is not a runtime no-op once a compile-time IL weaver is in play, and a broad "any empty Dispose" gate cannot rule that out — only a gate scoped to a specific -compiler/language-forced shape (here: `IEnumerator`) can.** +compiler/language-forced shape (here: `IEnumerator`) can.** --- diff --git a/docs/notes/precision-remeasure-2026-07-11.md b/docs/notes/precision-remeasure-2026-07-11.md index c65c754f..e8c289b4 100644 --- a/docs/notes/precision-remeasure-2026-07-11.md +++ b/docs/notes/precision-remeasure-2026-07-11.md @@ -223,7 +223,8 @@ regression — item 2 above, the ClosedXML 263-finding over-exemption) with the explicit-interface coverage gap (item 1 above) folded into the same fix, closed by [PR #240](https://github.com/PhysShell/Own.NET/pull/240). The shipped fix took the narrowing direction (confine the exemption to -`IEnumerator`-implementing types) rather than the weaver-convention-sniffing +types implementing the generic `System.Collections.Generic.IEnumerator`, +not the non-generic `IEnumerator`) rather than the weaver-convention-sniffing heuristic sketched in item 2 — see `docs/notes/field-notes-patterns.md` entry 19 for the full account, including why the original "−5 Slice.cs points, 263 restored" acceptance