fix(extractor): recognize WinForms Controls/IContainer disposal channels (Closes #219)

.github/workflows/ci.yml

@@ -213,6 +213,7 @@ jobs:
             frontend/roslyn/samples/TemplatePartLocalCaptureSample.cs \
             frontend/roslyn/samples/EmptyDisposeSample.cs \
             frontend/roslyn/samples/AppScopedSourceSample.cs \
+            frontend/roslyn/samples/WinFormsDisposalSample.cs \
             frontend/roslyn/samples/AssociatedObjectSourceSample.cs \
             -o "$RUNNER_TEMP/facts.json"
           cat "$RUNNER_TEMP/facts.json"
@@ -879,6 +880,32 @@ jobs:
           # curated initializer — the stale declaration binding must not exempt.
           echo "$out" | grep -qE "AppScopedSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'ReassignApp'" \
             || { echo "FAIL: expected OWN001 when the resolver-bound local is reassigned before the +="; exit 1; }
+          # issue #219 — WinForms disposal CHANNELS: (a) a Control/ToolStripItem field added to THIS's
+          # own Controls/Items collection is disposed transitively by base.Dispose(disposing); (b) a
+          # component built with `new T(components)` / registered via `components.Add(x)` is disposed by
+          # components.Dispose(). ~30 ShareX FPs. Both require the class to reach a disposal root.
+          # SILENT: (a) direct Controls.Add, AddRange, a transitive ToolStrip Items add, a ComboBox
+          # added to Controls; (b) both IContainer-registration shapes incl. the named Add(x,"name").
+          if echo "$out" | grep -qE "WinFormsDisposalSample\.cs.*'(lblStatus|btnOk|menu|menuItem|trayIcon|icon|namedIcon|combo)'"; then
+            echo "FAIL: a WinForms Controls/Items-membership or IContainer-registered field was wrongly flagged (#219)"; exit 1
+          fi
+          # ...and the negative controls MUST stay flagged: an add into a FOREIGN (param) container;
+          # a plain owner with NO Dispose at all (owned container never disposed); a component NOT
+          # container-registered; and a real disposable stored as a ComboBox item (ObjectCollection
+          # does not dispose its items — Codex).
+          echo "$out" | grep -qE "WinFormsDisposalSample\.cs:[0-9]+:.*\[OWN001\].*'lblForeign'" \
+            || { echo "FAIL: #219 (a): a control added to a FOREIGN container must stay flagged"; exit 1; }
+          echo "$out" | grep -qE "WinFormsDisposalSample\.cs:[0-9]+:.*\[OWN001\].*'item'" \
+            || { echo "FAIL: #219 (a): a control in an owned container with NO Dispose must stay flagged"; exit 1; }
+          echo "$out" | grep -qE "WinFormsDisposalSample\.cs:[0-9]+:.*\[OWN001\].*'unregisteredIcon'" \
+            || { echo "FAIL: #219 (b): a component NOT registered in the IContainer must stay flagged"; exit 1; }
+          echo "$out" | grep -qE "WinFormsDisposalSample\.cs:[0-9]+:.*\[OWN001\].*'comboItem'" \
+            || { echo "FAIL: #219 (a): a disposable stored as a ComboBox item (ObjectCollection) must stay flagged"; exit 1; }
+          # exactly 5 WinFormsDisposalSample findings (lblForeign, cms, item, unregisteredIcon,
+          # comboItem) — the eight channel-disposed fields are all silent; any leak pushes it past 5.
+          nwf=$(echo "$out" | grep -cE "WinFormsDisposalSample\.cs:[0-9]+:.*\[OWN001\]")
+          [ "$nwf" = "5" ] \
+            || { echo "FAIL: #219 expected exactly 5 WinFormsDisposalSample findings (controls only), got $nwf"; exit 1; }
           # issue #227 — a `Behavior`-derived subscriber whose event source is (an element
           # reached from) its own base-class `AssociatedObject` must be SILENT: the
           # behavior cannot outlive being attached, so the source is co-lifetimed with the
@@ -919,7 +946,7 @@ jobs:
           # population scan must span every partial of the type.
           echo "$out" | grep -qE "AssociatedObjectSourceSample\.cs:[0-9]+: warning: \[OWN001\].*'PartialFieldBehavior'" \
             || { echo "FAIL: expected OWN001 when a sibling partial injects the AssociatedObject field (#227)"; exit 1; }
-          echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) + #228 curated app-scoped source in App (silent; controls flagged) + #227 self-owned Behavior.AssociatedObject source (silent; controls flagged) at the C# location"
+          echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) + [OwnIgnore] suppression (silent-but-counted, SARIF suppressions) + #218 DP old->new subscription rotation (silent; controls flagged) + #225 empty-Dispose local exemption (silent; controls flagged) + #228 curated app-scoped source in App (silent; controls flagged) + #227 self-owned Behavior.AssociatedObject source (silent; controls flagged) + #219 WinForms Controls/IContainer disposal channels (silent; controls flagged) at the C# location"
       - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
         run: |
           # Path-sensitive flow analysis of local IDisposables — bugs the flat D1

docs/notes/field-notes-patterns.md

@@ -462,6 +462,25 @@ construction through (or explicit registration into) the designer's `IContainer`
 is a release, exactly like `Controls.Add` — the container's `Dispose()` is the
 real sink.**
 
+**Shipped (issue #219 — covers both entries 12 and 13).** The field-disposal scan
+now credits both channels, but **only when the class reaches the framework disposal
+root** — a designer `Dispose(bool)` that calls `base.Dispose(disposing)`
+(`ClassReachesDisposalRoot`). A class with no such Dispose (the ShareX
+`HistoryItemManager` true-positive) is not rooted, so its owned controls stay
+flagged. (a) A `Control`/`ToolStripItem` field added to **this object's own**
+`Controls`/`Items` collection (`this.Controls.Add`/`AddRange`, or `this.<field>.
+Items.Add` where `<field>` itself reaches the root) is released transitively — a
+fixpoint so a `ToolStrip` added to `this.Controls` cascades to its items. An add
+into a **foreign** container (a parameter/local — resolved semantically, not by
+name) yields no credit and stays flagged. (b) A field **constructed with** the
+designer `IContainer` (`new T(components)`) or explicitly `components.Add(x)` is
+released by `components.Dispose()`; the sink is a disposed field of type
+`System.ComponentModel.IContainer`. A component **not** registered (a bare
+`new NotifyIcon()`) stays flagged. Extractor-only, no OwnIR change. Pinned by
+`frontend/roslyn/samples/WinFormsDisposalSample.cs` (six channel-disposed fields
+silent; `lblForeign`/`cms`/`item`/`unregisteredIcon` controls flagged) in the
+`wpf-extractor` CI job.
+
 ## 14. `using (field = new T())` — a field as the direct `using` acquisition target
 
 **Seen in:** ShareX `ShareX.HelpersLib/Cryptographic/HashChecker.cs:59`,

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -1515,6 +1515,85 @@
         || t.AllInterfaces.Any(i => i.Name == "IDisposable"
                                     && i.ContainingNamespace?.ToString() == "System"));
 
+// --- Issue #219: WinForms disposal CHANNELS ---
+
+// The class reaches the framework disposal root via a designer `Dispose(bool)` that calls
+// `base.Dispose(disposing)` — `Control.Dispose(bool)` recursively disposes its `Controls` collection
+// and the `IContainer components`. Only such a class is treated as rooted; a class with no Dispose at
+// all (the ShareX HistoryItemManager true-positive) is NOT, so its owned controls stay flagged.
+static bool ClassReachesDisposalRoot(ClassDeclarationSyntax cls)
+{
+    foreach (var m in cls.Members.OfType<MethodDeclarationSyntax>())
+        if (m.Identifier.Text == "Dispose")
+            foreach (var inv in m.DescendantNodes().OfType<InvocationExpressionSyntax>())
+                if (inv.Expression is MemberAccessExpressionSyntax
+                    { Expression: BaseExpressionSyntax, Name.Identifier.Text: "Dispose" })
+                    return true;
+    return false;
+}
+
+// A type that is (or implements) `IContainer` — the designer `components` sink. Matched by simple
+// name (like the other WinForms type recognitions), so `System.ComponentModel.IContainer` and a
+// stand-in of the same name both resolve.
+static bool ImplementsIContainer(ITypeSymbol? t) =>
+    t is not null
+    && (t.Name == "IContainer" || t.AllInterfaces.Any(i => i.Name == "IContainer"));
+
+// The container an `X.Controls.Add(..)` / `X.Items.Add(..)` collection belongs to, given the
+// collection expression `X.Controls` / `X.Items` (or a bare inherited `Controls`/`Items`): "" (a
+// sentinel) for THIS object, the FIELD name for a `this.<field>`/`<field>` container, or null when it
+// is not a disposing collection, or its container is FOREIGN (a parameter / local / another
+// instance). A `Controls` collection (Control.ControlCollection) disposes its children, and a
+// ToolStrip's `Items` (ToolStripItemCollection) disposes its items — but a ComboBox/ListBox `Items`
+// (ObjectCollection) does NOT dispose arbitrary items, so an `Items` add is a channel only when the
+// collection's type is a ToolStripItemCollection (Codex). Semantic on the receiver so a parameter
+// named like a field is not mistaken for one.
+static string? CollectionContainerKey(ExpressionSyntax collExpr, SemanticModel model)
+{
+    string member;
+    ExpressionSyntax? containerObj;
+    switch (collExpr)
+    {
+        case IdentifierNameSyntax { Identifier.Text: "Controls" or "Items" } bare:
+            member = bare.Identifier.Text; containerObj = null; break;   // inherited member -> this
+        case MemberAccessExpressionSyntax { Name.Identifier.Text: "Controls" or "Items" } ma:
+            member = ma.Name.Identifier.Text; containerObj = ma.Expression; break;
+        default:
+            return null;
+    }
+    // `Items` disposes its items only for a ToolStripItemCollection; a ComboBox/ListBox
+    // ObjectCollection does not, so it is not a disposal channel.
+    if (member == "Items" && model.GetTypeInfo(collExpr).Type?.Name != "ToolStripItemCollection")
+        return null;
+    if (containerObj is null or ThisExpressionSyntax)
+        return "";
+    return model.GetSymbolInfo(containerObj).Symbol is IFieldSymbol { IsStatic: false } f ? f.Name : null;
+}
+
+// The THIS-field names added by an `.Add(this.child)` / `.AddRange(new T[]{ this.a, this.b })` call —
+// flattening an array-creation initializer (the designer `AddRange` form). Semantic (IFieldSymbol) so
+// only real fields of this object are credited.
+static IEnumerable<string> AddedFieldNames(ArgumentListSyntax args, SemanticModel model)
+{
+    foreach (var a in args.Arguments)
+    {
+        var items = a.Expression switch
+        {
+            ArrayCreationExpressionSyntax { Initializer: { } ai } => ai.Expressions,
+            ImplicitArrayCreationExpressionSyntax { Initializer: { } ii } => ii.Expressions,
+            _ => default,
+        };
+        if (items.Count > 0)
+        {
+            foreach (var e in items)
+                if (model.GetSymbolInfo(e).Symbol is IFieldSymbol { IsStatic: false } af)
+                    yield return af.Name;
+        }
+        else if (model.GetSymbolInfo(a.Expression).Symbol is IFieldSymbol { IsStatic: false } sf)
+            yield return sf.Name;
+    }
+}
+
 // The ignored result of a member `.Subscribe(...)` is a leakable IDisposable token (WPF004) only
 // when the call RETURNS an IDisposable — the Rx `IObservable<T>.Subscribe()` shape. A RESOLVED void
 // / non-IDisposable return has no token to leak: StackExchange.Redis's `ISubscriber.Subscribe(channel,
@@ -4095,7 +4174,7 @@
     .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
     .Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
     .ToList();
 var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);
 var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
 // P-004 WPF profile: widen the reference set with assemblies named by the
 // OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
@@ -4551,6 +4630,83 @@
                 && ThisFieldName(uae.Left) is { } uf)
                 disposed.Add(uf);
 
+        // Issue #219 — WinForms disposal CHANNELS. Only when the class reaches the framework disposal
+        // root (a designer `Dispose(bool)` -> `base.Dispose(disposing)`): a class with no such Dispose
+        // (the HistoryItemManager true-positive shape) is never rooted, so its owned controls stay
+        // flagged.
+        if (ClassReachesDisposalRoot(cls))
+        {
+            // (b) IContainer registration. The sink is a field of type IContainer that is disposed in
+            // this class (already in `disposed` via `components.Dispose()` / `components?.Dispose()`).
+            // A field CONSTRUCTED with that sink (`new T(components)`) or explicitly `sink.Add(this.G)`
+            // is disposed by `components.Dispose()`.
+            var containerSinks = new HashSet<string>(StringComparer.Ordinal);
+            foreach (var fdecl in cls.Members.OfType<FieldDeclarationSyntax>())
+                foreach (var v in fdecl.Declaration.Variables)
+                    if (disposed.Contains(v.Identifier.Text)
+                        && model.GetDeclaredSymbol(v) is IFieldSymbol fsym
+                        && ImplementsIContainer(fsym.Type))
+                        containerSinks.Add(v.Identifier.Text);
+            if (containerSinks.Count > 0)
+            {
+                bool CtorTakesSink(BaseObjectCreationExpressionSyntax oce) =>
+                    oce.ArgumentList is { } al && al.Arguments.Any(
+                        a => model.GetSymbolInfo(a.Expression).Symbol is IFieldSymbol af
+                             && containerSinks.Contains(af.Name));
+                // `G = new T(..., components, ...)` — assignment form.
+                foreach (var a in assigns)
+                    if (a.IsKind(SyntaxKind.SimpleAssignmentExpression)
+                        && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol gf
+                        && a.Right is BaseObjectCreationExpressionSyntax gnew && CtorTakesSink(gnew))
+                        disposed.Add(gf.Name);
+                // ...and the field-initializer form `T G = new T(..., components, ...)`.
+                foreach (var fdecl in cls.Members.OfType<FieldDeclarationSyntax>())
+                    foreach (var v in fdecl.Declaration.Variables)
+                        if (v.Initializer?.Value is BaseObjectCreationExpressionSyntax inew
+                            && CtorTakesSink(inew))
+                            disposed.Add(v.Identifier.Text);
+                // explicit `components.Add(this.G)` — and the named overload `Add(this.G, "name")`
+                // (IContainer.Add(IComponent, string)); the FIRST argument is the registered component
+                // in both, so credit it whenever the receiver is a container sink (Codex).
+                foreach (var inv in cls.DescendantNodes().OfType<InvocationExpressionSyntax>())
+                    if (inv.Expression is MemberAccessExpressionSyntax { Name.Identifier.Text: "Add" } addMa
+                        && inv.ArgumentList.Arguments.Count >= 1
+                        && model.GetSymbolInfo(addMa.Expression).Symbol is IFieldSymbol sinkf
+                        && containerSinks.Contains(sinkf.Name)
+                        && model.GetSymbolInfo(inv.ArgumentList.Arguments[0].Expression).Symbol is IFieldSymbol regf)
+                        disposed.Add(regf.Name);
+            }
+
+            // (a) Controls/Items membership. `Control.Dispose(bool)` disposes every child in its own
+            // `Controls` collection (and a ToolStrip disposes its `Items`). Collect each add edge
+            // `(containerKey, childField)`, then fixpoint from the rooted set — the child of a rooted
+            // container is itself rooted, so a ToolStrip added to `this.Controls` cascades to its items.
+            // An add into a FOREIGN container (param/local) yields no edge and stays flagged.
+            var edges = new List<(string container, string child)>();
+            foreach (var inv in cls.DescendantNodes().OfType<InvocationExpressionSyntax>())
+                if (inv.Expression is MemberAccessExpressionSyntax { Name.Identifier.Text: "Add" or "AddRange" } addColl
+                    && CollectionContainerKey(addColl.Expression, model) is { } ckey)
+                    foreach (var child in AddedFieldNames(inv.ArgumentList, model))
+                        edges.Add((ckey, child));
+            if (edges.Count > 0)
+            {
+                // rooted = THIS ("") plus every field already released (direct dispose / registered above),
+                // any of which is a container whose Controls/Items disposal reaches the root.
+                var rooted = new HashSet<string>(disposed, StringComparer.Ordinal) { "" };
+                var changed = true;
+                while (changed)
+                {
+                    changed = false;
+                    foreach (var (container, child) in edges)
+                        if (rooted.Contains(container) && disposed.Add(child))
+                        {
+                            rooted.Add(child);   // the child is now itself a rooted container
+                            changed = true;
+                        }
+                }
+            }
+        }
+
         // P-004 EventSource counter exemption: inside an EventSource, a DiagnosticCounter field
         // constructed with `this` is registered to (and lifetime-owned by) the source — a
         // process-lived diagnostic the source never field-disposes (see IsEventSourceOwnedCounter).