Skip to content

Commit d2c0f92

Browse files
committed
fix(extractor): resolve-aware field disposability — ask the real IDisposable (mined ImageSharp)
The field-disposable leak detector classified a field's type with a pure NAME heuristic (EndsWith Stream/Reader/Writer/Subscription), so any in-project type whose name happens to end that way was flagged even when it is NOT IDisposable. Mining SixLabors/ImageSharp surfaced the FPs: undisposed `Vp8BitWriter`/`Vp8LBitWriter` (`: BitWriterBase`) and `JpegBitReader` (a struct) fields — none of them IDisposable. IsOwnedDisposableType now asks the RESOLVED type's real IDisposable/IAsyncDisposable interface; it falls back to the name heuristic only when the type does NOT resolve (an unreferenced external assembly — WPF/DevExpress on a Linux runner — which is the whole reason that heuristic exists). This generalises the #79 PipeReader/PipeWriter exclusion: any resolved non-IDisposable type is now excluded, no curated list needed. Regression sample ResolvedDisposableSample.cs: a resolved non-IDisposable `…Writer` field and a non-IDisposable `…Reader` struct stay silent; resolved IDisposable controls (MemoryStream, CancellationTokenSource) still warn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent eba8c9a commit d2c0f92

3 files changed

Lines changed: 88 additions & 1 deletion

File tree

‎.github/workflows/ci.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ jobs:
139139
frontend/roslyn/samples/AppLifetimeSample.cs \
140140
frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs \
141141
frontend/roslyn/samples/InjectedDcViewSample.xaml.cs \
142+
frontend/roslyn/samples/ResolvedDisposableSample.cs \
142143
-o "$RUNNER_TEMP/facts.json"
143144
cat "$RUNNER_TEMP/facts.json"
144145
- name: Check facts through the core
@@ -203,6 +204,16 @@ jobs:
203204
if echo "$out" | grep -q "SharedTokenHolder"; then
204205
echo "FAIL: a static singleton IDisposable field was wrongly reported"; exit 1
205206
fi
207+
# P-004 resolve-aware disposability (mined: ImageSharp Vp8BitWriter/JpegBitReader):
208+
# a field whose type NAME ends in Writer/Reader/Stream but is NOT IDisposable (and
209+
# RESOLVES) must NOT be flagged — IsOwnedDisposableType asks the real interface.
210+
if echo "$out" | grep -q "EncoderWithNonDisposableWriter"; then
211+
echo "FAIL: a resolved non-IDisposable Writer/Reader field was wrongly flagged"; exit 1
212+
fi
213+
# control: resolved IDisposable fields (MemoryStream / CancellationTokenSource) the
214+
# class new's but never disposes must STILL warn — real detection intact.
215+
echo "$out" | grep -q "HolderWithRealDisposable" \
216+
|| { echo "FAIL: a resolved IDisposable field (MemoryStream/CTS) must still warn"; exit 1; }
206217
# WPF004: an ignored `X.Subscribe(...)` result leaks; the captured+
207218
# disposed one stays silent. "ignored" is unique to the WPF004 message.
208219
echo "$out" | grep -q "MessengerViewModel.cs" \

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1524,6 +1524,37 @@ static bool IsNonDisposableReaderWriter(string t) =>
15241524
t is "PipeReader" or "PipeWriter"
15251525
or "System.IO.Pipelines.PipeReader" or "System.IO.Pipelines.PipeWriter";
15261526

1527+
// Is this field/local type an OWNED disposable? Prefer the RESOLVED type's real
1528+
// IDisposable/IAsyncDisposable interface: an in-project `…Writer`/`…Reader`/`…Stream`
1529+
// that is NOT actually IDisposable (ImageSharp's `Vp8BitWriter : BitWriterBase`, the
1530+
// `JpegBitReader` struct) must not be flagged by name alone. Generalises the #79
1531+
// PipeReader/PipeWriter exclusion — any resolved non-IDisposable type is excluded, no
1532+
// curated list needed. Only when the type does NOT resolve (an unreferenced external
1533+
// assembly — WPF/DevExpress on the Linux runner) do we fall back to the syntactic name
1534+
// heuristic, which is the whole reason that heuristic exists.
1535+
static bool IsOwnedDisposableType(TypeSyntax type, SemanticModel model)
1536+
{
1537+
var sym = model.GetTypeInfo(type).Type;
1538+
if (sym is null or IErrorTypeSymbol)
1539+
return IsDisposableType(type.ToString());
1540+
return ImplementsDisposable(sym);
1541+
}
1542+
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+
15271558
// --- P-006: DI registration + constructor graph (DI001 captive dependency) ---
15281559
// A syntactic pass over the same trees, independent of the event/disposable
15291560
// detectors: collect each class's constructor parameter types (the dependency
@@ -2116,7 +2147,11 @@ or ImplicitObjectCreationExpressionSyntax
21162147
if (fd.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword)))
21172148
continue;
21182149
var tname = fd.Declaration.Type.ToString();
2119-
if (!IsDisposableType(tname))
2150+
// Resolve-aware: when the field type binds to a real symbol, demand an actual
2151+
// IDisposable; only an UNRESOLVED type falls back to the name heuristic. (Stops
2152+
// the #1 ImageSharp FP class: undisposed `Vp8BitWriter`/`JpegBitReader` fields
2153+
// whose types merely end in Writer/Reader but are not IDisposable.)
2154+
if (!IsOwnedDisposableType(fd.Declaration.Type, model))
21202155
continue;
21212156
foreach (var v in fd.Declaration.Variables)
21222157
{
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using System.IO;
2+
using System.Threading;
3+
4+
namespace Own.Samples;
5+
6+
// P-004 resolve-aware disposability (mined: ImageSharp Vp8BitWriter / JpegBitReader).
7+
//
8+
// A field whose type NAME ends in Writer/Reader/Stream but does NOT actually implement
9+
// IDisposable must NOT be flagged: the field-disposable detector now asks the RESOLVED
10+
// type's real interface, not the name. The IDisposable control must still warn, proving
11+
// real detection is intact (and the name heuristic still covers UNRESOLVED types, which
12+
// the existing WPF samples exercise).
13+
14+
// Resolved, NOT IDisposable, name ends in "Writer" (like ImageSharp's Vp8BitWriter).
15+
public sealed class FancyBitWriter
16+
{
17+
public void Write(int x) { }
18+
}
19+
20+
// Resolved, NOT IDisposable, a struct named "...Reader" (like ImageSharp's JpegBitReader).
21+
public struct TokenReader
22+
{
23+
public int Position;
24+
}
25+
26+
public sealed class EncoderWithNonDisposableWriter
27+
{
28+
private readonly FancyBitWriter writer = new(); // resolved, not IDisposable -> SILENT
29+
private TokenReader reader = new(); // resolved struct, not IDisposable -> SILENT
30+
31+
public void Encode() => this.writer.Write(this.reader.Position);
32+
}
33+
34+
// Control: resolved IDisposable fields the class new's but never disposes -> must WARN.
35+
public sealed class HolderWithRealDisposable
36+
{
37+
private readonly MemoryStream stream = new(); // MemoryStream IS IDisposable
38+
private readonly CancellationTokenSource cts = new(); // CancellationTokenSource IS IDisposable
39+
40+
public long Use() => this.stream.Length + this.cts.Token.GetHashCode();
41+
}

0 commit comments

Comments
 (0)