Skip to content

Commit ec4ce5c

Browse files
committed
fix(extractor): keep dispose-optional exemption in resolve-aware path (Codex); tighten CI assertion (CodeRabbit)
Codex P2: the resolve-aware IsOwnedDisposableType flagged EVERY real IDisposable, so dispose-optional types the flat name path skipped for free — Task / ValueTask / DataTable / DataSet / DataView — would re-FP as undisposed fields. Reuse the existing ImplementsIDisposable + IsDisposeOptional helpers (shared with the flow detector): `ImplementsIDisposable(sym) && !IsDisposeOptional(sym)`. Dropped the duplicate ImplementsDisposable/IsDisposableInterface helpers I had added. Sample gains a HolderWithDisposeOptional control (a new'd, undisposed Task + DataTable) that must stay silent. CI: strengthen the positive assertion to OWN001 + the disposable-field resource on ResolvedDisposableSample (CodeRabbit), kept severity-agnostic since the disposable-field leak renders as error, not warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent d2c0f92 commit ec4ce5c

3 files changed

Lines changed: 30 additions & 18 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,18 @@ jobs:
211211
echo "FAIL: a resolved non-IDisposable Writer/Reader field was wrongly flagged"; exit 1
212212
fi
213213
# control: resolved IDisposable fields (MemoryStream / CancellationTokenSource) the
214-
# class new's but never disposes must STILL warn — real detection intact.
214+
# class new's but never disposes must STILL warn — real detection intact. (CodeRabbit:
215+
# tie the assertion to OWN001 + the disposable-field resource, not just the class name.
216+
# Severity-agnostic on purpose — the disposable-field leak renders as error, not warning.)
217+
echo "$out" | grep -qE "ResolvedDisposableSample\.cs:[0-9]+:.*\[OWN001\].*resource: disposable field" \
218+
|| { echo "FAIL: expected the OWN001 disposable-field finding on the resolved IDisposable control"; exit 1; }
215219
echo "$out" | grep -q "HolderWithRealDisposable" \
216-
|| { echo "FAIL: a resolved IDisposable field (MemoryStream/CTS) must still warn"; exit 1; }
220+
|| { echo "FAIL: the resolved IDisposable control (MemoryStream/CTS) must be flagged by owner name"; exit 1; }
221+
# dispose-optional control (Codex): Task / DataTable ARE IDisposable but disposal is
222+
# optional (IsDisposeOptional) — a new'd, undisposed field of these must stay SILENT.
223+
if echo "$out" | grep -q "HolderWithDisposeOptional"; then
224+
echo "FAIL: a dispose-optional (Task/DataTable) field was wrongly flagged"; exit 1
225+
fi
217226
# WPF004: an ignored `X.Subscribe(...)` result leaks; the captured+
218227
# disposed one stays silent. "ignored" is unique to the WPF004 message.
219228
echo "$out" | grep -q "MessengerViewModel.cs" \

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,24 +1537,14 @@ static bool IsOwnedDisposableType(TypeSyntax type, SemanticModel model)
15371537
var sym = model.GetTypeInfo(type).Type;
15381538
if (sym is null or IErrorTypeSymbol)
15391539
return IsDisposableType(type.ToString());
1540-
return ImplementsDisposable(sym);
1540+
// Resolved: demand a REAL IDisposable, but keep the optional-dispose exemption
1541+
// (Task/ValueTask/DataTable/DataSet/DataView) that the flat name path got for free —
1542+
// their Dispose is a no-op / they hold only a lazy wait handle, so an undisposed field
1543+
// of these is not a leak and must stay silent (Codex). Both helpers already exist and
1544+
// are shared with the flow detector.
1545+
return ImplementsIDisposable(sym) && !IsDisposeOptional(sym);
15411546
}
15421547

1543-
static bool ImplementsDisposable(ITypeSymbol sym)
1544-
{
1545-
if (IsDisposableInterface(sym))
1546-
return true;
1547-
foreach (var i in sym.AllInterfaces)
1548-
if (IsDisposableInterface(i))
1549-
return true;
1550-
return false;
1551-
}
1552-
1553-
static bool IsDisposableInterface(ITypeSymbol t) =>
1554-
t.SpecialType == SpecialType.System_IDisposable
1555-
|| (t.Name == "IAsyncDisposable"
1556-
&& t.ContainingNamespace is { Name: "System", ContainingNamespace.IsGlobalNamespace: true });
1557-
15581548
// --- P-006: DI registration + constructor graph (DI001 captive dependency) ---
15591549
// A syntactic pass over the same trees, independent of the event/disposable
15601550
// detectors: collect each class's constructor parameter types (the dependency

‎frontend/roslyn/samples/ResolvedDisposableSample.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
using System.Data;
12
using System.IO;
23
using System.Threading;
4+
using System.Threading.Tasks;
35

46
namespace Own.Samples;
57

@@ -39,3 +41,14 @@ public sealed class HolderWithRealDisposable
3941

4042
public long Use() => this.stream.Length + this.cts.Token.GetHashCode();
4143
}
44+
45+
// Dispose-optional control (Codex): Task and DataTable ARE IDisposable, but disposal is
46+
// conventionally optional (IsDisposeOptional) — a `new`'d, undisposed field of these must
47+
// stay SILENT even though it resolves as IDisposable.
48+
public sealed class HolderWithDisposeOptional
49+
{
50+
private readonly Task task = new(() => { }); // Task: IDisposable but dispose-optional
51+
private readonly DataTable table = new(); // DataTable: IDisposable but dispose-optional
52+
53+
public int Use() => this.task.Id + this.table.Columns.Count;
54+
}

0 commit comments

Comments
 (0)