From 0c1c2f3bab42f00f78501699d0fc61d63aec99c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 09:39:46 +0000 Subject: [PATCH 1/4] =?UTF-8?q?fix(extractor):=20confine=20the=20empty-Dis?= =?UTF-8?q?pose=20exemption=20to=20enumerators=20=E2=80=94=20source=20empt?= =?UTF-8?q?iness=20is=20not=20a=20runtime=20no-op=20(#238)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-merge re-measure caught a soundness regression from #225/PR #233: ClosedXML came out -265 instead of the expected -5. 263 of those were real findings silently swallowed — XLWorkbook.Dispose() is empty IN SOURCE only, Janitor.Fody weaves the actual cleanup into it at build time, and HasEmptyDisposeBody trusted the source tree. Three changes, all in the same gate: - SOUNDNESS (the 263): the exemption now applies ONLY to types implementing IEnumerator/IEnumerator — the shape that motivated #225, where an empty Dispose is an idiomatic interface stub, not a weaving target. Everything else keeps the honest warning; source-level analysis cannot prove the absence of IL weaving for arbitrary types. - Defense in depth: a FodyWeavers.xml anywhere above the type's source file disables the exemption even for a perfect enumerator shape (weaved/ fixture + CI assertion). Unlocatable source -> conservatively no exemption. - COVERAGE (the missing 3 of -5): `void IDisposable.Dispose() { }` (explicit interface implementation — the actual Slice.Enumerator spelling) is now recognized as an empty Dispose; the DisposeAsync refusal likewise matches explicit `IAsyncDisposable.DisposeAsync`. Sample rework: ExplicitDisposeEnumerator ('x', silent) pins the coverage fix; ScratchReader — a NON-enumerator with an empty source Dispose, the XLWorkbook shape — flips from silent to a FLAGGED control on both detector paths; flow count assertion 4 -> 5; new weaved/WeavedEmptyDispose.cs fixture pins the kill-switch. Verified with the real extractor (.NET 8): flat flags s/lr/ar, flow flags exactly s/r/lr/d/ar with e/x silent, the weaved enumerator stays flagged; full-sample-set diff vs main (minus the reworked sample) is byte-identical. Gates: run_tests 276/276, ruff, mypy green. Post-merge acceptance: re-measure ClosedXML — delta vs the pre-#233 baseline must be exactly -5 with all 263 findings restored. Closes #238 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 35 +++++++---- frontend/roslyn/OwnSharp.Extractor/Program.cs | 60 +++++++++++++++++-- frontend/roslyn/samples/EmptyDisposeSample.cs | 35 +++++++++-- .../roslyn/samples/weaved/FodyWeavers.xml | 3 + .../samples/weaved/WeavedEmptyDispose.cs | 30 ++++++++++ 5 files changed, 140 insertions(+), 23 deletions(-) create mode 100644 frontend/roslyn/samples/weaved/FodyWeavers.xml create mode 100644 frontend/roslyn/samples/weaved/WeavedEmptyDispose.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e6f423d..d61d6584 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -837,14 +837,13 @@ jobs: # ...while the same-named local in the OTHER method must still warn. echo "$out" | grep -qE "TemplatePartLocalCaptureSample\.cs:[0-9]+: warning: \[OWN001\].*'SameNameDifferentScopeSubscriber'" \ || { echo "FAIL: expected OWN001 on the same-named-but-unrelated local in a different method"; exit 1; } - # issue #225 — a user-defined type whose Dispose() body is provably EMPTY holds no resource, - # so a LOCAL of it never disposed is not a leak (extends the named-BCL no-op exemption to any - # type). Here the flat (non-flow) name path: a `*Reader`-named empty-Dispose local (ScratchReader) - # is silent, while a non-empty `*Reader` local (LeakyReader) still leaks. (The semantic enumerator - # form is exercised in the --flow-locals step below.) - if echo "$out" | grep -q "'ScratchReader'"; then - echo "FAIL: a local of a user type with a provably-empty Dispose() body was wrongly flagged (#225)"; exit 1 - fi + # issue #225 (narrowed by #238) — the empty-Dispose exemption is confined to ENUMERATOR + # types: source emptiness proves nothing about the COMPILED body once an IL weaver is in + # play (Janitor.Fody wove real cleanup into ClosedXML's XLWorkbook.Dispose — 263 silently + # swallowed findings). ScratchReader (empty source Dispose, NOT an enumerator — the + # XLWorkbook shape) must therefore STAY flagged on the flat name path... + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'s'.*'ScratchReader'" \ + || { echo "FAIL: #238: a NON-enumerator empty-source-Dispose local must stay flagged (weaver soundness)"; exit 1; } echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'lr'.*'LeakyReader'" \ || { echo "FAIL: a non-empty-Dispose local (LeakyReader) must still leak (#225 stays scoped)"; exit 1; } # Codex P2: an empty SYNC Dispose but a real DisposeAsync (a `*Reader` here) must still leak. @@ -1124,11 +1123,23 @@ jobs: # must NOT be exempted — the real cleanup is async and the flow detector treats it as a release. echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ar' is never disposed" \ || { echo "FAIL: #225 flow: a type with a real DisposeAsync (empty sync Dispose) must still leak"; exit 1; } - # exactly 4 EmptyDisposeSample findings -> the two provably-empty-Dispose locals (the enumerator - # and ScratchReader) are SILENT; any leak of them would push the count past 4. + # #238 soundness: ScratchReader (empty source Dispose, NOT an enumerator) must stay flagged + # in the flow path too — a weaver can rewrite a non-enumerator Dispose at build time. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'s' is never disposed" \ + || { echo "FAIL: #238 flow: a NON-enumerator empty-source-Dispose local must stay flagged"; exit 1; } + # exactly 5 EmptyDisposeSample findings -> the two provably-empty-Dispose ENUMERATOR locals + # ('e' plain, 'x' explicit-interface — the #238 coverage fix) are SILENT; any leak of them + # would push the count past 5. n225=$(echo "$out" | grep -cE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN00") - [ "$n225" = "4" ] \ - || { echo "FAIL: #225 flow: expected exactly 4 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + [ "$n225" = "5" ] \ + || { echo "FAIL: #225/#238 flow: expected exactly 5 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + # #238 weaver kill-switch: a directory carrying FodyWeavers.xml disables the exemption even + # for a perfect enumerator shape — the weaved fixture's local must be FLAGGED. + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + frontend/roslyn/samples/weaved/WeavedEmptyDispose.cs --flow-locals -o "$RUNNER_TEMP/weaved.json" + wout=$(python -m ownlang ownir "$RUNNER_TEMP/weaved.json" || true) + echo "$wout" | grep -qE "WeavedEmptyDispose\.cs:[0-9]+:.*\[OWN001\].*'we' is never disposed" \ + || { echo "FAIL: #238: FodyWeavers.xml present -> even an enumerator empty Dispose must stay flagged"; exit 1; } echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, #225 empty-Dispose local exempt, beyond flat)" - name: Gallery C#-native bad/ok pairs (examples/gallery/cs/) run: | diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 56ab8281..d923d0b4 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1434,6 +1434,16 @@ static bool HasEmptyDisposeBody(ITypeSymbol? t) { if (t is not INamedTypeSymbol nt) return false; + // Issue #238 SOUNDNESS gate: an empty SOURCE body proves nothing about the COMPILED body — + // IL weavers (Janitor.Fody wove the real cleanup into ClosedXML's XLWorkbook.Dispose(), + // 263 silently-swallowed findings) rewrite Dispose at build time, and source-level analysis + // cannot prove their absence. So the exemption is confined to the shape that motivated + // #225 and is idiomatically stubbed, not woven: ENUMERATORS (IEnumerator forces IDisposable + // onto types that usually hold nothing). Everything else keeps the honest warning. + if (!nt.AllInterfaces.Any(i => i.SpecialType + is SpecialType.System_Collections_IEnumerator + or SpecialType.System_Collections_Generic_IEnumerator_T)) + return false; // The base must not own a Dispose we'd be silently skipping — only `object` / a non-IDisposable base. for (var b = nt.BaseType; b is not null && b.SpecialType != SpecialType.System_Object; b = b.BaseType) if (ImplementsIDisposable(b)) @@ -1442,14 +1452,52 @@ static bool HasEmptyDisposeBody(ITypeSymbol? t) // compatibility no-op (a type implementing both IDisposable AND IAsyncDisposable). The flow // detector already treats DisposeAsync() as a release, so an undisposed local of such a type is a // real leak — conservatively refuse to exempt any type that declares its own DisposeAsync (Codex). - if (nt.GetMembers("DisposeAsync").OfType().Any(m => m.Parameters.Length == 0)) + // #238: an EXPLICIT `IAsyncDisposable.DisposeAsync` has metadata name "…DisposeAsync", so match + // by simple name OR explicit-implementation target. + if (nt.GetMembers().OfType().Any(m => m.Parameters.Length == 0 + && (m.Name == "DisposeAsync" + || m.ExplicitInterfaceImplementations.Any(x => x.Name == "DisposeAsync")))) return false; - var dispose = nt.GetMembers("Dispose").OfType().FirstOrDefault( - m => m.Parameters.Length == 0 && m.ReturnsVoid && m.TypeParameters.Length == 0); - if (dispose is null || dispose.DeclaringSyntaxReferences.Length == 0) + // #238 coverage: `void IDisposable.Dispose() { }` (explicit interface implementation — the + // actual ClosedXML Slice.Enumerator spelling) is a Dispose too; GetMembers("Dispose") misses + // it because its metadata name is "System.IDisposable.Dispose". + var disposes = nt.GetMembers().OfType().Where(m => + m.Parameters.Length == 0 && m.ReturnsVoid && m.TypeParameters.Length == 0 + && (m.Name == "Dispose" + || m.ExplicitInterfaceImplementations.Any( + x => x.ContainingType.SpecialType == SpecialType.System_IDisposable))) + .ToList(); + if (disposes.Count == 0 || disposes.Any(d => d.DeclaringSyntaxReferences.Length == 0)) return false; // no readable source Dispose (metadata-only) -> cannot prove empty - return dispose.DeclaringSyntaxReferences.All( - sref => sref.GetSyntax() is MethodDeclarationSyntax { Body.Statements.Count: 0 }); + if (!disposes.All(d => d.DeclaringSyntaxReferences.All( + sref => sref.GetSyntax() is MethodDeclarationSyntax { Body.Statements.Count: 0 }))) + return false; + // #238 defense in depth: a FodyWeavers.xml anywhere above the type's source file means a + // weaver CAN rewrite this Dispose at build time — never exempt, even an enumerator. + return !SourceTreeHasWeaverConfig(nt); +} + +// #238: walk up from the type's declaring source file looking for FodyWeavers.xml (the Fody +// weaver manifest — Janitor/Costura/etc. rewrite method bodies after compilation). A type we +// cannot locate on disk is conservatively treated as weavable (no exemption). +static bool SourceTreeHasWeaverConfig(INamedTypeSymbol nt) +{ + var path = nt.DeclaringSyntaxReferences.FirstOrDefault()?.SyntaxTree.FilePath; + if (string.IsNullOrEmpty(path)) + return true; + try + { + for (var dir = Path.GetDirectoryName(Path.GetFullPath(path)); + !string.IsNullOrEmpty(dir); + dir = Path.GetDirectoryName(dir)) + if (File.Exists(Path.Combine(dir, "FodyWeavers.xml"))) + return true; + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException or ArgumentException) + { + return true; // cannot inspect the tree -> cannot rule a weaver out -> no exemption + } + return false; } // A type that is System.Windows.Forms.Form or derives from it (semantic, walks the diff --git a/frontend/roslyn/samples/EmptyDisposeSample.cs b/frontend/roslyn/samples/EmptyDisposeSample.cs index 0608b7c7..67fe067b 100644 --- a/frontend/roslyn/samples/EmptyDisposeSample.cs +++ b/frontend/roslyn/samples/EmptyDisposeSample.cs @@ -23,12 +23,27 @@ public sealed class EmptyDisposeEnumerator : IEnumerator public void Dispose() { } // literally empty -> no resource } -// EMPTY Dispose on a name that ALSO matches the flat (non-flow) name heuristic (`*Reader`), so the -// non-flow local-disposable path exercises the same exemption. +// EMPTY Dispose spelled as an EXPLICIT interface implementation — the actual ClosedXML +// Slice.Enumerator form (#238 coverage): metadata name "System.IDisposable.Dispose", which a +// plain GetMembers("Dispose") lookup misses. +public sealed class ExplicitDisposeEnumerator : IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + void IDisposable.Dispose() { } // literally empty, explicit-interface spelling +} + +// #238 SOUNDNESS control — the XLWorkbook shape: an empty SOURCE Dispose on a NON-enumerator +// type. An IL weaver (Janitor.Fody) can inject the real cleanup at build time, and source-level +// analysis cannot prove it doesn't — so this must STAY flagged. (`*Reader` name keeps the flat +// path exercising it too.) public sealed class ScratchReader : IDisposable { public int Read() => -1; - public void Dispose() { } // empty -> no resource + public void Dispose() { } // empty IN SOURCE only -> not provably a runtime no-op } // NON-empty Dispose — a real owned resource released in Dispose. A local never disposed LEAKS. @@ -85,11 +100,21 @@ public int CountEmpty() return n; } - // SILENT: an empty-Dispose `*Reader` local (flat-path name match) never disposed. + // SILENT: the explicit-interface empty-Dispose enumerator (#238 coverage), never disposed. + public int CountExplicit() + { + var x = new ExplicitDisposeEnumerator(); + var n = 0; + while (x.MoveNext()) n++; // never disposed -> Dispose is empty -> SILENT + return n; + } + + // FLAGGED (#238 soundness control): an empty SOURCE Dispose on a NON-enumerator — a weaver + // may add the real cleanup at build time, so the exemption must not apply -> OWN001. public int UseScratch() { var s = new ScratchReader(); - return s.Read(); // never disposed -> Dispose is empty -> SILENT + return s.Read(); // non-enumerator empty-in-source Dispose -> LEAK } // FLAGGED (control): a real IDisposable local never disposed -> OWN001. diff --git a/frontend/roslyn/samples/weaved/FodyWeavers.xml b/frontend/roslyn/samples/weaved/FodyWeavers.xml new file mode 100644 index 00000000..b41ca452 --- /dev/null +++ b/frontend/roslyn/samples/weaved/FodyWeavers.xml @@ -0,0 +1,3 @@ + + + diff --git a/frontend/roslyn/samples/weaved/WeavedEmptyDispose.cs b/frontend/roslyn/samples/weaved/WeavedEmptyDispose.cs new file mode 100644 index 00000000..ce79c248 --- /dev/null +++ b/frontend/roslyn/samples/weaved/WeavedEmptyDispose.cs @@ -0,0 +1,30 @@ +// #238 weaver kill-switch fixture: this directory carries a FodyWeavers.xml, so even a +// PERFECT enumerator-shaped empty Dispose must NOT be exempted — a weaver (Janitor.Fody) +// can rewrite the body at build time and source-level emptiness proves nothing here. +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Own.Samples.Weaved; + +public sealed class WeavedEnumerator : IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // empty in source; a weaver may fill it at build time +} + +public sealed class WeavedConsumer +{ + // FLAGGED: the FodyWeavers.xml above disables the empty-Dispose exemption entirely. + public int Count() + { + var we = new WeavedEnumerator(); + var n = 0; + while (we.MoveNext()) n++; // never disposed -> must STAY OWN001 + return n; + } +} From 607773f5f4729f4302a26885010ef5af23ec41f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 10:04:07 +0000 Subject: [PATCH 2/4] fix(extractor): honor the owning project's Fody config for linked sources (Codex on #240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Fody-enabled project can compile a linked source that lives OUTSIDE its directory () — the ancestor walk from the FILE path never sees the FodyWeavers.xml sitting next to the .csproj, so a linked empty-Dispose enumerator kept the exemption the kill-switch exists to withhold. ProjectCsFiles now records every file a weaver-enabled project compiles (Program.WeaverOwnedFiles, filled while expanding items); SourceTreeHasWeaverConfig consults that registry before the ancestor walk. Directory/bare-file scans carry no project info and keep today's honest per-path behaviour. Pinned by the weaved-linked/ fixture (Proj/Linked.csproj + FodyWeavers.xml linking ../Shared/SharedEnumerator.cs) + CI assertion: analysed THROUGH the .csproj the local is flagged; verified locally both ways, full-sample flat and weaved outputs byte-identical to the pre-fix run. Refs #238 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 8 ++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 39 +++++++++++++++++-- .../weaved-linked/Proj/FodyWeavers.xml | 3 ++ .../samples/weaved-linked/Proj/Linked.csproj | 10 +++++ .../weaved-linked/Shared/SharedEnumerator.cs | 31 +++++++++++++++ 5 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 frontend/roslyn/samples/weaved-linked/Proj/FodyWeavers.xml create mode 100644 frontend/roslyn/samples/weaved-linked/Proj/Linked.csproj create mode 100644 frontend/roslyn/samples/weaved-linked/Shared/SharedEnumerator.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d61d6584..8e48c18c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1140,6 +1140,14 @@ jobs: wout=$(python -m ownlang ownir "$RUNNER_TEMP/weaved.json" || true) echo "$wout" | grep -qE "WeavedEmptyDispose\.cs:[0-9]+:.*\[OWN001\].*'we' is never disposed" \ || { echo "FAIL: #238: FodyWeavers.xml present -> even an enumerator empty Dispose must stay flagged"; exit 1; } + # ...and the LINKED-source variant (Codex on #240): the shared file lives OUTSIDE the + # weaver project's directory (no FodyWeavers.xml above it) — analysed THROUGH the + # .csproj, the project's weaver config must still disable the exemption. + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + frontend/roslyn/samples/weaved-linked/Proj/Linked.csproj --flow-locals -o "$RUNNER_TEMP/weavedlink.json" + lout=$(python -m ownlang ownir "$RUNNER_TEMP/weavedlink.json" || true) + echo "$lout" | grep -qE "SharedEnumerator\.cs:[0-9]+:.*\[OWN001\].*'se' is never disposed" \ + || { echo "FAIL: #240: a linked source of a Fody-enabled project must not get the empty-Dispose exemption"; exit 1; } echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, #225 empty-Dispose local exempt, beyond flat)" - name: Gallery C#-native bad/ok pairs (examples/gallery/cs/) run: | diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index d923d0b4..25d3dfb5 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -309,6 +309,13 @@ static List ProjectCsFiles(string csproj, EnumerationOptions opts) result.RemoveAll(f => rmMatch.Any(m => m(f))); } + // #240: if THIS project is weaver-enabled (FodyWeavers.xml in its directory or above), + // EVERY file it compiles — including linked sources outside its tree — is weavable at + // build time; record them so the empty-Dispose kill-switch sees past the file's own path. + if (AncestorsHaveWeaverConfig(dir)) + foreach (var f in result) + WeaverOwnedFiles.Add(Path.GetFullPath(f)); + return result.Distinct().ToList(); } @@ -1487,15 +1494,33 @@ static bool SourceTreeHasWeaverConfig(INamedTypeSymbol nt) return true; try { - for (var dir = Path.GetDirectoryName(Path.GetFullPath(path)); - !string.IsNullOrEmpty(dir); - dir = Path.GetDirectoryName(dir)) + var full = Path.GetFullPath(path); + // A linked source compiled by a Fody-enabled project (Codex on #240): the weaver + // config sits next to the OWNING .csproj, not above the file — ProjectCsFiles + // recorded such files while expanding the project's items. + if (WeaverOwnedFiles.Contains(full)) + return true; + return AncestorsHaveWeaverConfig(Path.GetDirectoryName(full)); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException or ArgumentException) + { + return true; // cannot inspect the tree -> cannot rule a weaver out -> no exemption + } +} + +// #238/#240: FodyWeavers.xml in `startDir` or any ancestor. Unreadable filesystem state is +// treated as "a weaver may be present" — the callers then withhold the exemption. +static bool AncestorsHaveWeaverConfig(string? startDir) +{ + try + { + for (var dir = startDir; !string.IsNullOrEmpty(dir); dir = Path.GetDirectoryName(dir)) if (File.Exists(Path.Combine(dir, "FodyWeavers.xml"))) return true; } catch (Exception e) when (e is IOException or UnauthorizedAccessException or ArgumentException) { - return true; // cannot inspect the tree -> cannot rule a weaver out -> no exemption + return true; } return false; } @@ -5190,4 +5215,10 @@ or ImplicitObjectCreationExpressionSyntax } init partial class Program { internal static bool BodyThrowEdges; + + // #240 (Codex review): full paths of files compiled by a Fody-enabled PROJECT, recorded + // while expanding its items — a linked source outside the project directory + // would otherwise dodge the ancestor-walk weaver check (the FodyWeavers.xml lives next + // to the .csproj, not above the linked file). + internal static readonly HashSet WeaverOwnedFiles = new(StringComparer.Ordinal); } diff --git a/frontend/roslyn/samples/weaved-linked/Proj/FodyWeavers.xml b/frontend/roslyn/samples/weaved-linked/Proj/FodyWeavers.xml new file mode 100644 index 00000000..b41ca452 --- /dev/null +++ b/frontend/roslyn/samples/weaved-linked/Proj/FodyWeavers.xml @@ -0,0 +1,3 @@ + + + diff --git a/frontend/roslyn/samples/weaved-linked/Proj/Linked.csproj b/frontend/roslyn/samples/weaved-linked/Proj/Linked.csproj new file mode 100644 index 00000000..1ca5762a --- /dev/null +++ b/frontend/roslyn/samples/weaved-linked/Proj/Linked.csproj @@ -0,0 +1,10 @@ + + + net8.0 + + + + + + diff --git a/frontend/roslyn/samples/weaved-linked/Shared/SharedEnumerator.cs b/frontend/roslyn/samples/weaved-linked/Shared/SharedEnumerator.cs new file mode 100644 index 00000000..0487e484 --- /dev/null +++ b/frontend/roslyn/samples/weaved-linked/Shared/SharedEnumerator.cs @@ -0,0 +1,31 @@ +// #240 linked-source weaver fixture: this file lives OUTSIDE the Fody-enabled project's +// directory and is pulled in via . No FodyWeavers.xml +// sits above THIS file — the kill-switch must learn the weaver from the OWNING project. +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Own.Samples.WeavedLinked; + +public sealed class SharedEnumerator : IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // empty in source; the OWNING project weaves at build time +} + +public sealed class SharedConsumer +{ + // FLAGGED when analysed THROUGH the .csproj: the project's FodyWeavers.xml disables + // the empty-Dispose exemption even though this file's own ancestors carry none. + public int Count() + { + var se = new SharedEnumerator(); + var n = 0; + while (se.MoveNext()) n++; // never disposed -> must STAY OWN001 + return n; + } +} From 0d1411ce81582c60a046c387ca531703c7e6dc96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 10:16:34 +0000 Subject: [PATCH 3/4] fix(extractor): close three more soundness holes in the #238 empty-Dispose gate (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up review on #240 found the hotfix still left soundness gaps. All fixed: - Inherited IAsyncDisposable (P1): the async-cleanup check only looked at members declared on the type, so an enumerator with an empty sync Dispose whose real DisposeAsync lives on a BASE (that base not implementing IDisposable) was silently exempted — the exact #238 false-negative class. Now ANY IAsyncDisposable in AllInterfaces (inherited included) disqualifies. New flagged control InheritedAsyncEnumerator ('ia'), flow count 5 -> 6. - Over-broad enumerator gate (P2): non-generic System.Collections.IEnumerator does NOT extend IDisposable, so pairing them is author-chosen, not forced — the motivating proof only covers IEnumerator. Narrowed to the generic interface, matched by name/arity/namespace (the well-known-type SpecialType is unset in the source-only extraction, so a SpecialType check would wrongly reject legitimate generic enumerators). - Sticky static registry (P2): WeaverOwnedFiles was never reset, so a file seen as weaver-owned leaked into a later in-process invocation (sticky FP). Cleared at program start next to the existing BodyThrowEdges reset. - Fail-open weaver walk (P3): File.Exists returns false on access-denied, swallowing the case the comment claimed to fail closed on. Switched to File.GetAttributes, which throws on a permission error while still reporting a plain not-found — genuinely fail-closed now. Verified with the real extractor: flow flags exactly s,r,lr,d,ar,ia with the two generic enumerators (e,x) silent; weaved + weaved-linked fixtures flagged; full-sample-set diff vs main byte-identical. run_tests 276/276, ruff, mypy green. Refs #238 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 11 +++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 63 ++++++++++++++----- frontend/roslyn/samples/EmptyDisposeSample.cs | 33 ++++++++++ 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e48c18c..bcd15d82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1123,16 +1123,21 @@ jobs: # must NOT be exempted — the real cleanup is async and the flow detector treats it as a release. echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ar' is never disposed" \ || { echo "FAIL: #225 flow: a type with a real DisposeAsync (empty sync Dispose) must still leak"; exit 1; } + # #240 review P1: an enumerator whose sync Dispose is empty but whose BASE owns a real + # DisposeAsync (inherited IAsyncDisposable) must still leak — inherited async cleanup was + # the same silent-false-negative class #238 exists to close. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ia' is never disposed" \ + || { echo "FAIL: #240 flow: an inherited-IAsyncDisposable enumerator must still leak"; exit 1; } # #238 soundness: ScratchReader (empty source Dispose, NOT an enumerator) must stay flagged # in the flow path too — a weaver can rewrite a non-enumerator Dispose at build time. echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'s' is never disposed" \ || { echo "FAIL: #238 flow: a NON-enumerator empty-source-Dispose local must stay flagged"; exit 1; } # exactly 5 EmptyDisposeSample findings -> the two provably-empty-Dispose ENUMERATOR locals # ('e' plain, 'x' explicit-interface — the #238 coverage fix) are SILENT; any leak of them - # would push the count past 5. + # would push the count past 6. n225=$(echo "$out" | grep -cE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN00") - [ "$n225" = "5" ] \ - || { echo "FAIL: #225/#238 flow: expected exactly 5 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + [ "$n225" = "6" ] \ + || { echo "FAIL: #225/#238 flow: expected exactly 6 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } # #238 weaver kill-switch: a directory carrying FodyWeavers.xml disables the exemption even # for a perfect enumerator shape — the weaved fixture's local must be FLAGGED. dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 25d3dfb5..1ffac29b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -81,6 +81,10 @@ // run that did not request it (the other config — emitEvents/flowLocals/reportStats — are locals, // re-initialized each call, so they need no reset; only this static one does). CodeRabbit. BodyThrowEdges = false; +// #240 (review P2): WeaverOwnedFiles is the other process-global mutable state — a file recorded +// as belonging to a Fody-enabled project must not stay "weaver-owned" for an unrelated later +// invocation in the same process (a sticky false positive). Same reset discipline as above. +WeaverOwnedFiles.Clear(); // `extract` verb: the tool's one job is extraction, so an optional leading `extract` // makes the advertised `ownsharp-extract extract --project App.csproj --out facts.json` // UX real while the bare form (no verb) stays the default and back-compatible. (The @@ -1444,23 +1448,36 @@ static bool HasEmptyDisposeBody(ITypeSymbol? t) // Issue #238 SOUNDNESS gate: an empty SOURCE body proves nothing about the COMPILED body — // IL weavers (Janitor.Fody wove the real cleanup into ClosedXML's XLWorkbook.Dispose(), // 263 silently-swallowed findings) rewrite Dispose at build time, and source-level analysis - // cannot prove their absence. So the exemption is confined to the shape that motivated - // #225 and is idiomatically stubbed, not woven: ENUMERATORS (IEnumerator forces IDisposable - // onto types that usually hold nothing). Everything else keeps the honest warning. - if (!nt.AllInterfaces.Any(i => i.SpecialType - is SpecialType.System_Collections_IEnumerator - or SpecialType.System_Collections_Generic_IEnumerator_T)) + // cannot prove their absence. So the exemption is confined to the ONE shape whose empty + // Dispose is FORCED by the language, not authored: `IEnumerator` — the generic enumerator + // interface is the only one that transitively requires IDisposable (IEnumerator : IEnumerator, + // IDisposable). The non-generic System.Collections.IEnumerator does NOT extend IDisposable, so a + // type that pairs them coupled the two contracts itself — the motivating proof does not apply, and + // it keeps the honest warning (review #240). + // Matched by name/arity/namespace, not SpecialType: in the source-only extraction (no wired + // corelib reference) the well-known-type SpecialType is often unset, so `IEnumerator` would + // slip through a SpecialType check. `System.Collections.Generic.IEnumerator`1` is the exact + // interface that transitively forces IDisposable. + if (!nt.AllInterfaces.Any(i => i.Name == "IEnumerator" + && i.TypeArguments.Length == 1 + && i.ContainingNamespace?.ToString() == "System.Collections.Generic")) return false; // The base must not own a Dispose we'd be silently skipping — only `object` / a non-IDisposable base. for (var b = nt.BaseType; b is not null && b.SpecialType != SpecialType.System_Object; b = b.BaseType) if (ImplementsIDisposable(b)) return false; // A non-empty DisposeAsync() can do the REAL cleanup even when the sync Dispose() is an empty - // compatibility no-op (a type implementing both IDisposable AND IAsyncDisposable). The flow - // detector already treats DisposeAsync() as a release, so an undisposed local of such a type is a - // real leak — conservatively refuse to exempt any type that declares its own DisposeAsync (Codex). - // #238: an EXPLICIT `IAsyncDisposable.DisposeAsync` has metadata name "…DisposeAsync", so match - // by simple name OR explicit-implementation target. + // compatibility no-op. The flow detector treats DisposeAsync() as a release, so an undisposed + // local of such a type is a real leak. ANY IAsyncDisposable in the interface set — declared on + // this type OR INHERITED from a base (review #240 P1: the base's real DisposeAsync would + // otherwise be silently skipped, exactly the #238 false-negative class) — disqualifies the + // exemption: a type with async disposal is no longer the "simple enumerator holding nothing" + // this narrow rule is for. + if (nt.AllInterfaces.Any(i => i.Name == "IAsyncDisposable" + && i.ContainingNamespace?.ToString() == "System")) + return false; + // Belt-and-braces for a bare `DisposeAsync()` method that the flow detector may treat as a + // release without the type formally implementing IAsyncDisposable (declared or explicit-impl). if (nt.GetMembers().OfType().Any(m => m.Parameters.Length == 0 && (m.Name == "DisposeAsync" || m.ExplicitInterfaceImplementations.Any(x => x.Name == "DisposeAsync")))) @@ -1508,19 +1525,31 @@ static bool SourceTreeHasWeaverConfig(INamedTypeSymbol nt) } } -// #238/#240: FodyWeavers.xml in `startDir` or any ancestor. Unreadable filesystem state is -// treated as "a weaver may be present" — the callers then withhold the exemption. +// #238/#240: FodyWeavers.xml in `startDir` or any ancestor. GENUINELY fail-closed on an +// unreadable ancestor (review #240 P3): `File.Exists` returns false on access-denied — swallowing +// exactly the case we must treat as "a weaver may be present" — so we probe with +// `File.GetAttributes`, which THROWS on a permission error while still reporting a plain +// not-found. Absent config -> keep walking; unreadable state -> withhold the exemption. static bool AncestorsHaveWeaverConfig(string? startDir) { try { for (var dir = startDir; !string.IsNullOrEmpty(dir); dir = Path.GetDirectoryName(dir)) - if (File.Exists(Path.Combine(dir, "FodyWeavers.xml"))) - return true; + { + var candidate = Path.Combine(dir, "FodyWeavers.xml"); + try + { + _ = File.GetAttributes(candidate); + return true; // present AND readable -> a weaver is configured here + } + catch (FileNotFoundException) { } // no config in this dir — keep walking + catch (DirectoryNotFoundException) { } // this ancestor is gone — keep walking + } } - catch (Exception e) when (e is IOException or UnauthorizedAccessException or ArgumentException) + catch (Exception e) when (e is IOException or UnauthorizedAccessException + or System.Security.SecurityException or ArgumentException) { - return true; + return true; // cannot prove the config is absent -> fail closed, no exemption } return false; } diff --git a/frontend/roslyn/samples/EmptyDisposeSample.cs b/frontend/roslyn/samples/EmptyDisposeSample.cs index 67fe067b..1d6b5e47 100644 --- a/frontend/roslyn/samples/EmptyDisposeSample.cs +++ b/frontend/roslyn/samples/EmptyDisposeSample.cs @@ -89,6 +89,29 @@ public System.Threading.Tasks.ValueTask DisposeAsync() // REAL cleanup } } +// #240 review P1 — the real async cleanup lives in a BASE class's DisposeAsync, and the base does +// NOT implement IDisposable (so the base-cascade check alone lets it through). An empty sync +// Dispose on the derived enumerator must STILL leak: an IAsyncDisposable ANYWHERE in the interface +// set (inherited included) disqualifies the exemption. +public abstract class AsyncOwnerBase : IAsyncDisposable +{ + private readonly System.Threading.CancellationTokenSource _cts = new(); + public System.Threading.Tasks.ValueTask DisposeAsync() // REAL cleanup, on the BASE + { + _cts.Dispose(); + return default; + } +} +public sealed class InheritedAsyncEnumerator : AsyncOwnerBase, IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // empty sync, but the base owns a real DisposeAsync +} + public sealed class EmptyDisposeConsumers { // SILENT: an empty-Dispose enumerator local, iterated and never disposed (the ClosedXML shape). @@ -145,4 +168,14 @@ public int LeakAsync() var ar = new AsyncReader(); return ar.Read(); // never disposed (sync or async) -> LEAK } + + // FLAGGED (control, #240 review P1): the enumerator's own sync Dispose is empty, but its BASE + // owns a real DisposeAsync -> an inherited IAsyncDisposable must still leak. + public int LeakInheritedAsync() + { + var ia = new InheritedAsyncEnumerator(); + var n = 0; + while (ia.MoveNext()) n++; // base DisposeAsync is real -> LEAK + return n; + } } From 40756e31a66f6299cdf38ab7461e9cdaf0432ca9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 10:37:09 +0000 Subject: [PATCH 4/4] fix(extractor): base-chain bare DisposeAsync + non-generic-IEnumerator regression pin (review r2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round on #240: - Bare DisposeAsync on a BASE (P2): the belt-and-braces async check used nt.GetMembers() (current type only), so a base with a bare `public ValueTask DisposeAsync()` that does NOT implement IAsyncDisposable slipped its real async cleanup through — AllInterfaces misses it (no interface) and the member scan misses it (inherited). Now walks the whole base chain. New flagged control BareInheritedAsyncEnumerator ('ba'). - Non-generic IEnumerator regression pin (P2): the IEnumerator-only narrowing had no negative fixture. Added LegacyEnumerator (IEnumerator + IDisposable, empty Dispose) -> must stay flagged ('ng'). Flow count 6 -> 8. - Stale docs (P3): the HasEmptyDisposeBody header, the sample-file header still said "generalises to any type" — the exact over-broad framing that produced #238. Rewritten to the actual IEnumerator-only scope. Verified with the real extractor: flow flags exactly s,r,lr,d,ar,ia,ba,ng with the two generic enumerators (e,x) silent; full-sample diff vs main byte-identical. run_tests 276/276, ruff, mypy green. Refs #238 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LsWw4Ay8KLTHFom1HvRu3U --- .github/workflows/ci.yml | 14 +++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 31 +++++---- frontend/roslyn/samples/EmptyDisposeSample.cs | 67 +++++++++++++++++-- 3 files changed, 91 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bcd15d82..6ea948e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1128,16 +1128,24 @@ jobs: # the same silent-false-negative class #238 exists to close. echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ia' is never disposed" \ || { echo "FAIL: #240 flow: an inherited-IAsyncDisposable enumerator must still leak"; exit 1; } + # #240 review round 2: a BASE with a bare DisposeAsync (no IAsyncDisposable interface) must + # be caught by the base-chain scan -> still leak. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ba' is never disposed" \ + || { echo "FAIL: #240 flow: a base-chain bare DisposeAsync enumerator must still leak"; exit 1; } + # #240 review round 2: a NON-generic IEnumerator (does not force IDisposable) must stay + # flagged -> pins the IEnumerator-only narrowing. + echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'ng' is never disposed" \ + || { echo "FAIL: #240 flow: a non-generic-IEnumerator empty-Dispose local must still leak"; exit 1; } # #238 soundness: ScratchReader (empty source Dispose, NOT an enumerator) must stay flagged # in the flow path too — a weaver can rewrite a non-enumerator Dispose at build time. echo "$out" | grep -qE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'s' is never disposed" \ || { echo "FAIL: #238 flow: a NON-enumerator empty-source-Dispose local must stay flagged"; exit 1; } # exactly 5 EmptyDisposeSample findings -> the two provably-empty-Dispose ENUMERATOR locals # ('e' plain, 'x' explicit-interface — the #238 coverage fix) are SILENT; any leak of them - # would push the count past 6. + # would push the count past 8 (controls: s,r,lr,d,ar,ia,ba,ng). n225=$(echo "$out" | grep -cE "EmptyDisposeSample\.cs:[0-9]+:.*\[OWN00") - [ "$n225" = "6" ] \ - || { echo "FAIL: #225/#238 flow: expected exactly 6 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } + [ "$n225" = "8" ] \ + || { echo "FAIL: #225/#238 flow: expected exactly 8 EmptyDisposeSample findings (controls only), got $n225"; exit 1; } # #238 weaver kill-switch: a directory carrying FodyWeavers.xml disables the exemption even # for a perfect enumerator shape — the weaved fixture's local must be FLAGGED. dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 1ffac29b..b89abfe2 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1431,14 +1431,17 @@ static bool IsNoOpDisposeWrapper(BaseObjectCreationExpressionSyntax oce, Semanti return arg0 is not null && IsInMemoryDisposableBacking(model.GetTypeInfo(arg0).Type); } -// Issue #225 — a user-defined type whose parameterless `Dispose()` body is provably EMPTY holds no -// resource behind the interface: it implements IDisposable only to satisfy a contract (e.g. -// `IEnumerator`), so never calling Dispose() cannot leak. Recognised from SOURCE: the type -// declares `void Dispose()` with a BLOCK body of ZERO statements, and no base in its chain carries a -// Dispose to cascade to (the base is `object` / not IDisposable), so nothing real is skipped. A body -// we cannot read (third-party metadata-only, or expression-bodied doing work) is NOT provably empty -// and stays flagged — never a guessed drop. This generalises the named-BCL IsNoOpDisposeWrapper / -// IsDisposeOptional idea to any type. Deliberately scoped to LOCAL disposables (the #201-sweep +// Issue #225 (narrowed by #238) — a user-defined `IEnumerator` whose parameterless `Dispose()` +// body is provably EMPTY holds no resource behind the interface: `IEnumerator` is the one +// interface that FORCES IDisposable onto types that usually hold nothing, so never calling its +// empty Dispose() cannot leak. Recognised from SOURCE: the type declares `void Dispose()` (plain or +// explicit-interface) with a BLOCK body of ZERO statements, and no base in its chain carries a +// Dispose to cascade to, no IAsyncDisposable / bare DisposeAsync anywhere in the type or its bases, +// and no FodyWeavers.xml above it (an IL weaver could rewrite the "empty" body at build time — the +// #238 soundness lesson). A body we cannot read (metadata-only, or expression-bodied doing work) is +// NOT provably empty and stays flagged — never a guessed drop. Deliberately NOT "any type": that +// over-broad framing was the #238 regression (263 silently-swallowed ClosedXML findings). Scoped to +// LOCAL disposables (the #201-sweep // evidence — ClosedXML's `Slice.Enumerator` locals): a FIELD keeps its own disposal contract, so an // empty-Dispose field (the OwnIgnoreSample `Handle` stand-in) is unaffected. static bool HasEmptyDisposeBody(ITypeSymbol? t) @@ -1478,10 +1481,14 @@ static bool HasEmptyDisposeBody(ITypeSymbol? t) return false; // Belt-and-braces for a bare `DisposeAsync()` method that the flow detector may treat as a // release without the type formally implementing IAsyncDisposable (declared or explicit-impl). - if (nt.GetMembers().OfType().Any(m => m.Parameters.Length == 0 - && (m.Name == "DisposeAsync" - || m.ExplicitInterfaceImplementations.Any(x => x.Name == "DisposeAsync")))) - return false; + // Walk the ENTIRE base chain (review #240 round 2): GetMembers() returns only members declared + // on the current type, so a base with a bare DisposeAsync (no IAsyncDisposable, so AllInterfaces + // above misses it) would otherwise slip its real async cleanup through as an exemption. + for (var cur = nt; cur is not null && cur.SpecialType != SpecialType.System_Object; cur = cur.BaseType) + if (cur.GetMembers().OfType().Any(m => m.Parameters.Length == 0 + && (m.Name == "DisposeAsync" + || m.ExplicitInterfaceImplementations.Any(x => x.Name == "DisposeAsync")))) + return false; // #238 coverage: `void IDisposable.Dispose() { }` (explicit interface implementation — the // actual ClosedXML Slice.Enumerator spelling) is a Dispose too; GetMembers("Dispose") misses // it because its metadata name is "System.IDisposable.Dispose". diff --git a/frontend/roslyn/samples/EmptyDisposeSample.cs b/frontend/roslyn/samples/EmptyDisposeSample.cs index 1d6b5e47..cd7d85bb 100644 --- a/frontend/roslyn/samples/EmptyDisposeSample.cs +++ b/frontend/roslyn/samples/EmptyDisposeSample.cs @@ -4,12 +4,13 @@ namespace Own.Samples; -// Issue #225 — a user-defined type whose Dispose() body is provably EMPTY holds no resource behind -// the interface (it implements IDisposable only to satisfy a contract, e.g. IEnumerator), so a -// LOCAL of it that is never disposed cannot leak. Generalises the named-BCL no-op exemption -// (field-notes entry 9 / IsNoOpDisposeWrapper) to any type. Mirrors ClosedXML's Slice.Enumerator. -// Negative controls prove it does not over-widen. A FIELD keeps its own disposal contract (out of -// scope here), so the OwnIgnoreSample `Handle` field stand-in is unaffected. +// Issue #225 (narrowed by #238) — a user-defined `IEnumerator` whose Dispose() body is provably +// EMPTY holds no resource behind the interface (IEnumerator is the one interface that FORCES +// IDisposable onto a type that usually holds nothing), so a LOCAL of it never disposed cannot leak. +// Mirrors ClosedXML's Slice.Enumerator. The exemption is deliberately NOT "any type" (that was the +// #238 regression) and NOT the non-generic IEnumerator (which does not extend IDisposable). The +// negative controls below pin every edge of the narrowing. A FIELD keeps its own disposal contract +// (out of scope here), so the OwnIgnoreSample `Handle` field stand-in is unaffected. // EMPTY Dispose — implements IDisposable only because IEnumerator requires it (the ClosedXML // shape). Its Dispose does literally nothing. @@ -112,6 +113,40 @@ public sealed class InheritedAsyncEnumerator : AsyncOwnerBase, IEnumerator public void Dispose() { } // empty sync, but the base owns a real DisposeAsync } +// #240 review round 2 P2 — the base owns a BARE DisposeAsync() and does NOT implement +// IAsyncDisposable, so neither AllInterfaces nor a current-type-only member scan catches it; only +// walking the whole base chain does. An empty sync Dispose here must STILL leak. +public abstract class BareAsyncBase +{ + private readonly System.Threading.CancellationTokenSource _cts = new(); + public System.Threading.Tasks.ValueTask DisposeAsync() // bare, no interface, on the BASE + { + _cts.Dispose(); + return default; + } +} +public sealed class BareInheritedAsyncEnumerator : BareAsyncBase, IEnumerator +{ + private int _i; + public int Current => _i; + object IEnumerator.Current => Current; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // empty sync, but the base owns a bare real DisposeAsync +} + +// #240 review round 2 P2 — a NON-generic System.Collections.IEnumerator does NOT extend IDisposable, +// so pairing it with IDisposable is author-chosen, not forced: the motivating proof does not apply +// and an empty-Dispose local must STAY flagged. Pins the IEnumerator-only narrowing. +public sealed class LegacyEnumerator : IEnumerator, IDisposable +{ + private int _i; + public object Current => _i; + public bool MoveNext() => ++_i <= 3; + public void Reset() => _i = 0; + public void Dispose() { } // empty, but the type was not FORCED to implement IDisposable +} + public sealed class EmptyDisposeConsumers { // SILENT: an empty-Dispose enumerator local, iterated and never disposed (the ClosedXML shape). @@ -178,4 +213,24 @@ public int LeakInheritedAsync() while (ia.MoveNext()) n++; // base DisposeAsync is real -> LEAK return n; } + + // FLAGGED (control, #240 review round 2): base owns a BARE DisposeAsync (no interface) -> the + // base-chain scan must catch it -> LEAK. + public int LeakBareInheritedAsync() + { + var ba = new BareInheritedAsyncEnumerator(); + var n = 0; + while (ba.MoveNext()) n++; // base bare DisposeAsync is real -> LEAK + return n; + } + + // FLAGGED (control, #240 review round 2): non-generic IEnumerator + IDisposable is not forced -> + // the IEnumerator-only narrowing must keep it flagged -> LEAK. + public int LeakLegacy() + { + var ng = new LegacyEnumerator(); + var n = 0; + while (ng.MoveNext()) n++; // non-generic enumerator, author-coupled IDisposable -> LEAK + return n; + } }