fix(extractor): resolve-aware ignored-Subscribe — only flag when Subscribe returns IDisposable (mined StackExchange.Redis) by PhysShell · Pull Request #93 · PhysShell/Own.NET

PhysShell · 2026-06-23T09:25:43Z

First fix from the fresh StackExchange.Redis mine.

The FP

WPF004 flagged any bare member x.Subscribe(...) as an ignored IDisposable subscription token. But that token only exists for the Rx shape — IObservable<T>.Subscribe() → IDisposable. StackExchange.Redis's handler overload:

sub.Subscribe(RedisChannel.Literal("+switch-master"), (__, message) => { … }, CommandFlags.FireAndForget);

is ISubscriber.Subscribe(RedisChannel, Action<…>, CommandFlags) → void. There's nothing to dispose, so flagging it (twice, in ConnectionMultiplexer.Sentinel) was a false positive — and the same applies to the many event-bus Subscribe(handler) APIs. (Verified against the Redis source.)

The fix

Make it resolve-aware (mirrors IsOwnedDisposableType / #83): require the call's return type to implement IDisposable.

resolved void / non-IDisposable return → silenced (Redis, event buses)
resolved IDisposable return → still flagged (Rx, IMessenger.Subscribe)
unresolved return → keeps the syntactic benefit of the doubt, so ReactiveUI's WhenAnyValue(...).Subscribe(...) (whose type doesn't bind on the Linux runner) still fires

Regression sample (`VoidSubscribeSample.cs`)

VoidSubscriber (void Subscribe) → silent
DisposableSubscriber (IDisposable-returning Subscribe, ignored) → still warns

The existing IDisposable controls (MessengerViewModel.InboxViewModel warns; CleanInboxViewModel captures+disposes → silent) are unchanged.

🤖 Generated with Claude Code

Generated by Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved leak detection for ignored Subscribe(...) patterns by correctly handling void-returning/unknown-return shapes versus IDisposable-returning ones, reducing false positives while preserving expected warnings where a disposable subscription token can be produced.
Tests
- Expanded CI self-tests to cover an additional void-returning Subscribe scenario and to verify continued correct behavior for both IDisposable-returning and dynamic/unknown cases.
Documentation
- Updated the “ignored-Subscribe” sample to include the new void and dynamic scenarios.

…cribe returns IDisposable (mined StackExchange.Redis) WPF004 flagged any bare member `x.Subscribe(...)` as an ignored IDisposable subscription token. But that token only exists for the Rx shape (`IObservable<T>.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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED

coderabbitai · 2026-06-23T09:25:56Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88619864-f950-42bb-902f-4334bb516bfe

📥 Commits

Reviewing files that changed from the base of the PR and between b2d818a and 0419f98.

📒 Files selected for processing (3)

.github/workflows/ci.yml
frontend/roslyn/OwnSharp.Extractor/Program.cs
frontend/roslyn/samples/VoidSubscribeSample.cs

🚧 Files skipped from review as they are similar to previous changes (2)

.github/workflows/ci.yml
frontend/roslyn/OwnSharp.Extractor/Program.cs

📝 Walkthrough

Walkthrough

Adds a SubscribeResultIsDisposable helper to the WPF004 ignored-Subscribe leak detector in Program.cs, gating leak reporting on whether the Subscribe(...) call's resolved return type is IDisposable (or unresolved). A new sample fixture VoidSubscribeSample.cs defines void-returning and IDisposable-returning Subscribe contracts alongside a dynamic-receiver control case, and CI assertions verify all three cases.

Changes

Resolve-aware void-Subscribe suppression (WPF004)

Layer / File(s)	Summary
Sample fixture: void vs. IDisposable Subscribe contracts `frontend/roslyn/samples/VoidSubscribeSample.cs`	Adds `IRedisSubscriber` (void `Subscribe`) and `IObservableBus` (`IDisposable` `Subscribe`) interfaces, plus `VoidSubscriber`, `DisposableSubscriber`, and `DynamicSubscriber` sample classes exercising void-return, IDisposable-return, and dynamic-receiver scenarios.
`SubscribeResultIsDisposable` helper and WPF004 guard `frontend/roslyn/OwnSharp.Extractor/Program.cs`	Introduces `SubscribeResultIsDisposable(ITypeSymbol?)` returning true for unresolved/error types or `IDisposable`-implementing returns, and updates the WPF004 detector to require that predicate before recording an ignored-Subscribe leak candidate.
CI fixture inclusion and selftest assertions `.github/workflows/ci.yml`	Adds `VoidSubscribeSample.cs` to the extractor invocation and appends grep assertions verifying `VoidSubscriber` is absent from reported leaks while `DisposableSubscriber` and `DynamicSubscriber` remain present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

PhysShell/Own.NET#40: Also modifies WPF004 ignored-Subscribe logic in Program.cs, adding a different filtering layer (this.WhenAnyValue(...).Subscribe(...) self-rooted suppression) to the same detection path this PR refines.

Poem

🐇 A void Subscribe returns no token to hold,
So the rabbit won't flag what can't leak any gold.
IDisposable? Now that gets a stare!
With resolve-aware checks, we warn only with care.
Hop-hop, the tests pass — both silent and loud! 🎉

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically identifies the main change: fixing resolve-aware detection for ignored Subscribe calls to only flag when Subscribe returns IDisposable. It accurately reflects the core purpose of the PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/fp-subscribe-resolve-aware

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2d818a615

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-23T09:28:15Z

+// 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);


Treat dynamic Subscribe results as unresolved

When the receiver is dynamic, Roslyn gives x.Subscribe(...) a dynamic return type, which is neither null/IErrorTypeSymbol nor an IDisposable interface implementation. This now suppresses ignored dynamic Subscribe calls even though the extractor cannot prove the runtime overload is void or non-disposable, so dynamic Rx/event-bus subscriptions that used to be reported disappear; treat TypeKind.Dynamic as unknown here unless the call is proven non-IDisposable.

Useful? React with 👍 / 👎.

…nced (Codex) 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED

+{
+    public DisposableSubscriber(IObservableBus bus)
+    {
+        bus.Subscribe(_ => { });   // returns IDisposable, ignored -> WARN (resolve-aware still fires)


+    public DynamicSubscriber(dynamic bus)
+    {
+        Action<object> handler = _ => { };
+        bus.Subscribe(handler);   // dynamic return -> unknown -> WARN


github-advanced-security AI found potential problems Jun 23, 2026

View reviewed changes

Comment thread frontend/roslyn/samples/VoidSubscribeSample.cs Fixed

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

github-advanced-security AI found potential problems Jun 23, 2026

View reviewed changes

PhysShell merged commit b6f3cc7 into main Jun 23, 2026
22 checks passed

coderabbitai Bot mentioned this pull request Jul 16, 2026

feat(P-035): declared weak-subscribe wrapper = accepted release (B0+B1) #284

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(extractor): resolve-aware ignored-Subscribe — only flag when Subscribe returns IDisposable (mined StackExchange.Redis)#93

fix(extractor): resolve-aware ignored-Subscribe — only flag when Subscribe returns IDisposable (mined StackExchange.Redis)#93
PhysShell merged 2 commits into
mainfrom
claude/fp-subscribe-resolve-aware

PhysShell commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

PhysShell commented Jun 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The FP

The fix

Regression sample (VoidSubscribeSample.cs)

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PhysShell commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

Regression sample (`VoidSubscribeSample.cs`)

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading