From b2d818a615172dfdf33fa937ff5e58f450d1dc6f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 09:25:30 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(extractor):=20resolve-aware=20ignored-S?= =?UTF-8?q?ubscribe=20=E2=80=94=20only=20flag=20when=20Subscribe=20returns?= =?UTF-8?q?=20IDisposable=20(mined=20StackExchange.Redis)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WPF004 flagged any bare member `x.Subscribe(...)` as an ignored IDisposable subscription token. But that token only exists for the Rx shape (`IObservable.Subscribe()` -> IDisposable). StackExchange.Redis's `ISubscriber.Subscribe(channel, handler, flags)` returns VOID — there is nothing to dispose — so flagging it (ConnectionMultiplexer.Sentinel, twice) was a false positive, and the same applies to the many event-bus `Subscribe(handler)` APIs. Make it resolve-aware (mirrors IsOwnedDisposableType / #83): require the call's return type to implement IDisposable; a RESOLVED void / non-IDisposable return is silenced, while an UNRESOLVED return keeps the syntactic benefit of the doubt (so ReactiveUI's WhenAnyValue chains, whose type doesn't bind on the Linux runner, still fire). Regression sample VoidSubscribeSample.cs: VoidSubscriber (void Subscribe) is SILENT; the control DisposableSubscriber (IDisposable-returning Subscribe, ignored) still WARNs. The existing IDisposable control (MessengerViewModel. InboxViewModel) is unchanged. Mined: StackExchange.Redis (first fresh target after the Npgsql arc). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 9 +++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 16 +++++++- .../roslyn/samples/VoidSubscribeSample.cs | 39 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 frontend/roslyn/samples/VoidSubscribeSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25bfd2a6..dec51ddd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -147,6 +147,7 @@ jobs: frontend/roslyn/samples/AliasDisposeSample.cs \ frontend/roslyn/samples/CloseReleaseSample.cs \ frontend/roslyn/samples/SemaphoreFieldSample.cs \ + frontend/roslyn/samples/VoidSubscribeSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -300,6 +301,14 @@ jobs: || { echo "FAIL: expected the InboxViewModel ignored-Subscribe leak"; exit 1; } echo "$out" | grep -q "is ignored" \ || { echo "FAIL: expected the ignored-Subscribe message"; exit 1; } + # P-004 resolve-aware ignored-Subscribe (mined: StackExchange.Redis): a bare `x.Subscribe(...)` + # whose call returns VOID (the Redis `ISubscriber.Subscribe(channel, handler, flags)` shape) has + # no IDisposable token -> must be SILENT; the IDisposable-returning Subscribe still WARNs. + if echo "$out" | grep -q "leaking 'VoidSubscriber'"; then + echo "FAIL: a void-returning .Subscribe(...) was wrongly flagged as an ignored IDisposable subscription"; exit 1 + fi + echo "$out" | grep -q "leaking 'DisposableSubscriber'" \ + || { echo "FAIL: an ignored IDisposable-returning .Subscribe(...) must still warn (resolve-aware stays scoped)"; exit 1; } if echo "$out" | grep -q "CleanInboxViewModel"; then echo "FAIL: captured+disposed subscription wrongly reported"; exit 1 fi diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 45b4adb4..3bb9d988 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -459,6 +459,15 @@ t is not null || t.AllInterfaces.Any(i => i.Name == "IDisposable" && i.ContainingNamespace?.ToString() == "System")); +// The ignored result of a member `.Subscribe(...)` is a leakable IDisposable token (WPF004) only +// when the call RETURNS an IDisposable — the Rx `IObservable.Subscribe()` shape. A RESOLVED void +// / non-IDisposable return has no token to leak: StackExchange.Redis's `ISubscriber.Subscribe(channel, +// handler, flags)` returns void, as do many event-bus `Subscribe(handler)` APIs. Only an UNRESOLVED +// return type keeps the syntactic benefit of the doubt (mirrors IsOwnedDisposableType, #83). Mined: +// StackExchange.Redis ConnectionMultiplexer.Sentinel `sub.Subscribe(channel, handler, FireAndForget)`. +static bool SubscribeResultIsDisposable(ITypeSymbol? rt) => + rt is null or IErrorTypeSymbol || ImplementsIDisposable(rt); + // Types that implement IDisposable but whose disposal is conventionally OPTIONAL — // the .NET guidance / Roslyn CA2000 exempt them: Task/ValueTask only hold a // lazily-allocated wait handle, and the System.Data containers' Dispose() is a @@ -2541,11 +2550,14 @@ or ImplicitObjectCreationExpressionSyntax // WPF004: a `X.Subscribe(...)` whose IDisposable result is ignored — the // call stands as a bare statement (not assigned/returned/added), so the // token is dropped and never disposed. Member-access only (`x.Subscribe`), - // to avoid flagging bare void `Subscribe(...)` helpers. + // and RESOLVE-AWARE: the call must return an IDisposable (Rx). A resolved void / + // non-IDisposable `Subscribe` (StackExchange.Redis's `ISubscriber.Subscribe(channel, + // handler, flags)` is void) has no token to leak; an unresolved return still counts. foreach (var inv in cls.DescendantNodes().OfType()) if (inv.Expression is MemberAccessExpressionSyntax m && m.Name.Identifier.Text == "Subscribe" - && inv.Parent is ExpressionStatementSyntax) + && inv.Parent is ExpressionStatementSyntax + && SubscribeResultIsDisposable(model.GetTypeInfo(inv).Type)) subs.Add(new { @event = m.ToString(), diff --git a/frontend/roslyn/samples/VoidSubscribeSample.cs b/frontend/roslyn/samples/VoidSubscribeSample.cs new file mode 100644 index 00000000..99790063 --- /dev/null +++ b/frontend/roslyn/samples/VoidSubscribeSample.cs @@ -0,0 +1,39 @@ +using System; + +namespace Own.Samples; + +// P-004 resolve-aware ignored-Subscribe (WPF004), mined: StackExchange.Redis ConnectionMultiplexer.Sentinel. +// A bare `x.Subscribe(...)` is a leak ONLY when the call returns an IDisposable token (the Rx +// `IObservable.Subscribe()` shape). StackExchange.Redis's `ISubscriber.Subscribe(channel, handler, +// flags)` returns VOID — there is no token to leak — so it must NOT be flagged. The IDisposable-returning +// case stays flagged (DisposableSubscriber below, and MessengerViewModel.InboxViewModel). + +// a void-returning Subscribe (the handler overload) -> no IDisposable token -> must be SILENT. +public sealed class VoidSubscriber +{ + public VoidSubscriber(IRedisSubscriber sub) + { + sub.Subscribe("+switch-master", (_, _) => { }); // returns void -> nothing to dispose -> SILENT + } +} + +// control: a Subscribe that DOES return an IDisposable token, ignored -> STILL a leak -> WARN. +public sealed class DisposableSubscriber +{ + public DisposableSubscriber(IObservableBus bus) + { + bus.Subscribe(_ => { }); // returns IDisposable, ignored -> WARN (resolve-aware still fires) + } +} + +// StackExchange.Redis-style: the handler overload returns void. +public interface IRedisSubscriber +{ + void Subscribe(string channel, Action handler); +} + +// Rx-style: Subscribe hands back an IDisposable unsubscribe token. +public interface IObservableBus +{ + IDisposable Subscribe(Action handler); +} From 0419f98ccba0ac9c2bf5e8fde03d1c2ab7b5f461 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 09:30:01 +0000 Subject: [PATCH 2/2] fix(extractor): treat a dynamic Subscribe return as unknown, not silenced (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `dynamic` receiver gives `x.Subscribe(...)` a dynamic return type, which is neither null/IErrorTypeSymbol nor an IDisposable implementation — so the new resolve-aware gate wrongly silenced ignored dynamic Subscribe calls even though the runtime overload can't be proven void/non-disposable. Treat TypeKind.Dynamic as unresolved (benefit of the doubt -> still flag), alongside null/error returns. Regression: VoidSubscribeSample gains DynamicSubscriber (a `dynamic bus.Subscribe(handler)`) which must STILL warn. VoidSubscriber stays silent; DisposableSubscriber still warns. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 3 +++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 4 +++- frontend/roslyn/samples/VoidSubscribeSample.cs | 11 +++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dec51ddd..44faa8a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -309,6 +309,9 @@ jobs: fi echo "$out" | grep -q "leaking 'DisposableSubscriber'" \ || { echo "FAIL: an ignored IDisposable-returning .Subscribe(...) must still warn (resolve-aware stays scoped)"; exit 1; } + # Codex control: a `dynamic` receiver's Subscribe has a dynamic return -> unprovable -> still WARN. + echo "$out" | grep -q "leaking 'DynamicSubscriber'" \ + || { echo "FAIL: an ignored dynamic .Subscribe(...) must still warn (dynamic return is unknown, not silenced)"; exit 1; } if echo "$out" | grep -q "CleanInboxViewModel"; then echo "FAIL: captured+disposed subscription wrongly reported"; exit 1 fi diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 3bb9d988..0d783b4d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -466,7 +466,9 @@ t is not null // return type keeps the syntactic benefit of the doubt (mirrors IsOwnedDisposableType, #83). Mined: // StackExchange.Redis ConnectionMultiplexer.Sentinel `sub.Subscribe(channel, handler, FireAndForget)`. static bool SubscribeResultIsDisposable(ITypeSymbol? rt) => - rt is null or IErrorTypeSymbol || ImplementsIDisposable(rt); + rt is null or IErrorTypeSymbol + || rt.TypeKind == TypeKind.Dynamic // a `dynamic` receiver -> dynamic return; can't prove non-disposable (Codex) + || ImplementsIDisposable(rt); // Types that implement IDisposable but whose disposal is conventionally OPTIONAL — // the .NET guidance / Roslyn CA2000 exempt them: Task/ValueTask only hold a diff --git a/frontend/roslyn/samples/VoidSubscribeSample.cs b/frontend/roslyn/samples/VoidSubscribeSample.cs index 99790063..fa1e6e2b 100644 --- a/frontend/roslyn/samples/VoidSubscribeSample.cs +++ b/frontend/roslyn/samples/VoidSubscribeSample.cs @@ -26,6 +26,17 @@ public DisposableSubscriber(IObservableBus bus) } } +// Codex control: a `dynamic` receiver gives Subscribe a dynamic return type — we cannot prove it is +// void / non-IDisposable, so it keeps the benefit of the doubt and STILL warns. +public sealed class DynamicSubscriber +{ + public DynamicSubscriber(dynamic bus) + { + Action handler = _ => { }; + bus.Subscribe(handler); // dynamic return -> unknown -> WARN + } +} + // StackExchange.Redis-style: the handler overload returns void. public interface IRedisSubscriber {