diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25bfd2a6..44faa8a0 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,17 @@ 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; } + # 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 45b4adb4..0d783b4d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -459,6 +459,17 @@ 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 + || 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 // lazily-allocated wait handle, and the System.Data containers' Dispose() is a @@ -2541,11 +2552,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..fa1e6e2b --- /dev/null +++ b/frontend/roslyn/samples/VoidSubscribeSample.cs @@ -0,0 +1,50 @@ +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) + } +} + +// 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 +{ + void Subscribe(string channel, Action handler); +} + +// Rx-style: Subscribe hands back an IDisposable unsubscribe token. +public interface IObservableBus +{ + IDisposable Subscribe(Action handler); +}