Skip to content

Commit 682e3d1

Browse files
authored
Merge pull request #242 from PhysShell/claude/ownnet-cleanup-release-r5sr8g
docs: close #221, reconcile status notes with shipped #227-229 and #238/#240
2 parents 4c5a86b + 9d39b8c commit 682e3d1

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

‎docs/notes/field-notes-patterns.md‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,21 @@ exemption's real criterion is "does this object's lifetime start and end with th
551551
subscriber's" — a base-class accessor to the attached object, or an item of an
552552
owned collection, satisfies that just as well as a constructed field.**
553553

554-
**Status (2026-07):** shapes **(a)** and **(c)** shipped.
554+
**Status (2026-07):** all three sub-shapes **(a)**, **(b)**, **(c)** shipped —
555+
parent issue #221 closed once all three landed.
555556
- **(a)** the `Behavior.AssociatedObject` self-owned source, shipped in #227 —
556557
extractor `IsAssociatedObjectSource`, gated on the `Behavior` base
557558
(`IsBehaviorSubscriber`) plus a same-class assignment-chain resolving to
558559
`this.AssociatedObject` (`ResolvesToAssociatedObject`); pinned by
559560
`frontend/roslyn/samples/AssociatedObjectSourceSample.cs`.
561+
- **(b)** the curated app-scoped source for an `Application`-derived
562+
subscriber, shipped in #228 (PR #232) — extractor keeps the existing
563+
`clsIsApp` subscriber gate byte-for-byte and only loosens the *source*
564+
check to a curated resolver allowlist (`PaletteHelper.GetThemeManager`),
565+
method-group handlers only (no lambdas); see
566+
`docs/notes/oracle-known-fps.md` "What DID ship next to it (issue #228)"
567+
for why this is not the rejected `clsIsStatic` broadening. Pinned by
568+
`frontend/roslyn/samples/AppScopedSourceSample.cs`.
560569
- **(c)** the element of a self-populated collection, shipped in #229 — extractor
561570
`IsOwnedCollectionElementSource`: a `foreach` loop variable whose collection is a
562571
this-owned field/property populated ONLY from own construction / an own factory
@@ -694,6 +703,51 @@ existing fixtures that had used an empty `Dispose()` as a modelling shortcut
694703
(`UnitOfWork`, `PixelOwner`) were made faithful (a real Dispose body) so they
695704
remain genuine leak fixtures.
696705

706+
**Superseded by a soundness regression (issue #238) — gate narrowed to
707+
enumerators (issue #240 / PR #240).** The re-measure in
708+
`docs/notes/precision-remeasure-2026-07-11.md` (PR #235) ran the shipped \#225
709+
exemption above against real ClosedXML and found it unsound:
710+
`XLWorkbook.Dispose()` is empty **in source only** — `Janitor.Fody` (an IL
711+
weaver declared in ClosedXML's `FodyWeavers.xml`) rewrites the body at
712+
compile time to call real cleanup (`DisposeManaged()` →
713+
`Worksheets.ForEach(w => (w as XLWorksheet).Cleanup())`). `HasEmptyDisposeBody`
714+
read the source-level emptiness as proof of "nothing to release" and
715+
silently exempted 263 unrelated `XLWorkbook` locals across
716+
`ClosedXML.Examples` — a real leak class going quiet, not a false-positive
717+
fix. This inverts the analyzer's core doctrine (an exemption's worst case
718+
must stay "keeps today's honest warning," never "silently swallows a leak
719+
class"), so it was treated as a bug, not a precision tweak.
720+
721+
The fix (#240) **narrowed** the gate rather than special-casing weavers:
722+
`HasEmptyDisposeBody`'s exemption now applies **only** to types that
723+
implement the *generic* `System.Collections.Generic.IEnumerator<T>` — the
724+
one interface that *forces* a (frequently no-op) `Dispose()` implementation
725+
via `IEnumerator<T> : IEnumerator, IDisposable`, which was the entire
726+
motivating shape for #225 in the first place. The non-generic
727+
`System.Collections.IEnumerator` does **not** extend `IDisposable`, so it
728+
does not qualify. `XLWorkbook` and every other weaver-augmented domain type
729+
fall outside the gate and keep the honest warning. Defense in depth: a
730+
`FodyWeavers.xml` found above the source (or above the owning project, for
731+
linked sources) disables the exemption even for a qualifying enumerator.
732+
Explicit-interface `Dispose()` is now recognized as empty too (closing the
733+
coverage gap that left 3 of 5 `Slice.cs` sites unexempted) — `DisposeAsync()`
734+
is the opposite of an exemption: ANY `IAsyncDisposable` (declared or
735+
inherited) or bare `DisposeAsync()` method (declared, explicit-impl, or
736+
inherited from a base) unconditionally **disqualifies** the exemption, since
737+
async disposal means the type is no longer the "simple enumerator holding
738+
nothing" the rule is scoped to. Two review rounds closed further soundness
739+
holes: inherited `IAsyncDisposable` (via `AllInterfaces`), inherited bare
740+
`DisposeAsync` (via base-chain walk), a sticky static-registry cache, and a
741+
fail-open weaver-detection path (`File.GetAttributes` instead of
742+
`File.Exists`, which silently disabled the guard when the check itself
743+
errored). Confirmed on real ClosedXML (HEAD `4e89dce`, `--flow-locals`):
744+
121 of the swallowed findings restored, 0 removed. See issue #238 and PR \#240
745+
for the full record — this is a deliberate historical lesson, not
746+
cleaned up: **source-level emptiness is not a runtime no-op once a
747+
compile-time IL weaver is in play, and a broad "any empty Dispose" gate
748+
cannot rule that out — only a gate scoped to a specific
749+
compiler/language-forced shape (here: `IEnumerator<T>`) can.**
750+
697751
---
698752

699753
## The through-line

‎docs/notes/precision-remeasure-2026-07-11.md‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ OSS repos: #218-#225. Four follow-up PRs have since shipped fixes:
66

77
- [#230](https://github.com/PhysShell/Own.NET/pull/230) — Closes **#218**
88
(not #219 — #219 is the separate WinForms `Controls`/`IContainer` disposal
9-
channel gap, still open). DependencyProperty/property-changed old→new
10-
subscription-rotation recognition.
9+
channel gap, still open **as of this note**; shipped later via PR #236,
10+
see `docs/notes/field-notes-patterns.md` entry 12/13). DependencyProperty/
11+
property-changed old→new subscription-rotation recognition.
1112
- [#231](https://github.com/PhysShell/Own.NET/pull/231) — four smaller gaps
1213
in one PR: `CommandManager.RequerySuggested` allowlist (#223),
1314
`using (field = new T())` release (#220), template-part local/pattern-var
@@ -213,3 +214,20 @@ This note's own instructions were read-only with respect to the analyzer —
213214
no code was changed to investigate or produce the breakdown above; the
214215
breakdown is entirely from comparing old/new SARIF output and reading the
215216
target repos' real source.
217+
218+
## Resolution addendum (2026-07-11 / 2026-07-12)
219+
220+
Both follow-ups identified above were filed and shipped as a single unit:
221+
issue [#238](https://github.com/PhysShell/Own.NET/issues/238) (soundness
222+
regression — item 2 above, the ClosedXML 263-finding over-exemption) with
223+
the explicit-interface coverage gap (item 1 above) folded into the same
224+
fix, closed by [PR #240](https://github.com/PhysShell/Own.NET/pull/240).
225+
The shipped fix took the narrowing direction (confine the exemption to
226+
types implementing the generic `System.Collections.Generic.IEnumerator<T>`,
227+
not the non-generic `IEnumerator`) rather than the weaver-convention-sniffing
228+
heuristic sketched in item 2 — see
229+
`docs/notes/field-notes-patterns.md` entry 19 for the full account,
230+
including why the original "−5 Slice.cs points, 263 restored" acceptance
231+
target from #238 was itself corrected (ClosedXML's own `FodyWeavers.xml`
232+
covers `Slice.cs` too, so the sound outcome is "fully restored, nothing
233+
silently dropped," not a fixed `−5`).

0 commit comments

Comments
 (0)