diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16bcfc44..3f7c1d7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -215,6 +215,7 @@ jobs: frontend/roslyn/samples/AppScopedSourceSample.cs \ frontend/roslyn/samples/WinFormsDisposalSample.cs \ frontend/roslyn/samples/AssociatedObjectSourceSample.cs \ + frontend/roslyn/samples/OwnedCollectionElementSample.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -946,7 +947,42 @@ 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) + #219 WinForms Controls/IContainer disposal channels (silent; controls flagged) at the C# location" + # issue #229 — subscribing to an element of a collection the class itself + # populated (a `foreach` over a this-owned field/property assigned from the + # class's OWN construction/factory) must be SILENT: the collection and its + # elements share the constructing object's lifetime (a collectable self-cycle, + # not a leak). Three population forms: an own factory into a property + # (OwnedFactoryViewModel), a collection-initializer field (InlineNewViewModel), + # and a `new` assigned in the ctor (DirectNewViewModel). + if echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+:.*('OwnedFactoryViewModel'|'InlineNewViewModel'|'DirectNewViewModel')"; then + echo "FAIL: a self-populated owned-collection element subscription was wrongly reported (#229)"; exit 1 + fi + # ...and the exemption must NOT over-widen — four controls STAY flagged: + # (1, REQUIRED) the collection is a FIELD assigned from a ctor PARAMETER; + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'InjectedCollectionViewModel'" \ + || { echo "FAIL: expected OWN001 on the injected-collection field (#229)"; exit 1; } + # (2, REQUIRED) the `foreach` iterates a ctor PARAMETER collection directly; + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'ParamCollectionViewModel'" \ + || { echo "FAIL: expected OWN001 on the parameter-collection foreach (#229)"; exit 1; } + # (3) the member is populated from a SERVICE-LOCATED call (injected receiver); + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'ServiceLocatedViewModel'" \ + || { echo "FAIL: expected OWN001 on the service-located collection source (#229)"; exit 1; } + # (4) the member is own-populated in the ctor but ALSO reassigned an injected + # value elsewhere — every population site must be own-produced. + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'MixedCollectionViewModel'" \ + || { echo "FAIL: expected OWN001 when the collection is also assigned an injected value (#229)"; exit 1; } + # (5, Codex P1) a freshly-`new`d collection SEEDED from an injected parameter — + # the elements (which carry the events) are injected, so it is not owned; + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'SeededNewViewModel'" \ + || { echo "FAIL: expected OWN001 on a new collection seeded from injected elements (#229)"; exit 1; } + # (6, Codex P1) an own-class factory that FORWARDS a service-located collection; + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'ForwardingFactoryViewModel'" \ + || { echo "FAIL: expected OWN001 when the own factory forwards injected data (#229)"; exit 1; } + # (7, CodeRabbit) a PARTIAL class with a disqualifying injected assignment in a + # sibling partial — the population scan must span every partial of the type. + echo "$out" | grep -qE "OwnedCollectionElementSample\.cs:[0-9]+: warning: \[OWN001\].*'PartialInjectedViewModel'" \ + || { echo "FAIL: expected OWN001 when a sibling partial injects the collection (#229)"; 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) + #219 WinForms Controls/IContainer disposal channels (silent; controls flagged) + #229 self-populated owned-collection element (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 diff --git a/docs/notes/field-notes-patterns.md b/docs/notes/field-notes-patterns.md index 0099ac1a..a84c4dab 100644 --- a/docs/notes/field-notes-patterns.md +++ b/docs/notes/field-notes-patterns.md @@ -551,12 +551,18 @@ exemption's real criterion is "does this object's lifetime start and end with th subscriber's" — a base-class accessor to the attached object, or an item of an owned collection, satisfies that just as well as a constructed field.** -**Status (2026-07):** shape **(a)**, the `Behavior.AssociatedObject` self-owned -source, shipped in #227 — extractor `IsAssociatedObjectSource`, gated on the -`Behavior` base (`IsBehaviorSubscriber`) plus a same-class assignment-chain -resolving to `this.AssociatedObject` (`ResolvesToAssociatedObject`); pinned by -`frontend/roslyn/samples/AssociatedObjectSourceSample.cs`. Shape **(c)**, the -owned-collection element, is tracked separately by #229. +**Status (2026-07):** shapes **(a)** and **(c)** shipped. +- **(a)** the `Behavior.AssociatedObject` self-owned source, shipped in #227 — + extractor `IsAssociatedObjectSource`, gated on the `Behavior` base + (`IsBehaviorSubscriber`) plus a same-class assignment-chain resolving to + `this.AssociatedObject` (`ResolvesToAssociatedObject`); pinned by + `frontend/roslyn/samples/AssociatedObjectSourceSample.cs`. +- **(c)** the element of a self-populated collection, shipped in #229 — extractor + `IsOwnedCollectionElementSource`: a `foreach` loop variable whose collection is a + this-owned field/property populated ONLY from own construction / an own factory + (`MemberPopulatedByOwnFactory` + `IsOwnProducedValue`), never a ctor parameter or + an injected/service-located value; pinned by + `frontend/roslyn/samples/OwnedCollectionElementSample.cs`. ## 16. Template part fetched via `FindName`/`GetTemplateChild`, stored as a local diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index f96a9a3f..47155a5b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1173,6 +1173,174 @@ static bool IsAssociatedObjectSource(ExpressionSyntax left, SemanticModel model, => left is MemberAccessExpressionSyntax m && ResolvesToAssociatedObject(m.Expression, model, clsNode, depth: 0); +// P-004 / issue #229: the `+=` receiver is an element of a collection the class OWNS — +// a `foreach (var x in FieldOrProperty)` loop variable whose collection is a this-owned +// field/property the class ITSELF populates (own construction / own factory / collection +// initializer), never an injected or ctor-parameter value. The collection and its +// elements then share the constructing object's lifetime, so the subscription is a +// collectable self-cycle, not a leak. All checks are same-class and syntactic: +// * the receiver must bind to a `foreach` variable (matched by SYMBOL); +// * the loop's collection must be a bare/`this`-qualified member of THIS class; +// * that member's every population site must be an own-produced value (below) — a +// single injected/parameter assignment denies the proof (precision-first). +static bool IsOwnedCollectionElementSource(ExpressionSyntax left, SemanticModel model, + INamedTypeSymbol? cls) +{ + if (cls is null || left is not MemberAccessExpressionSyntax m) + return false; + if (model.GetSymbolInfo(m.Expression).Symbol is not ILocalSymbol loopVar) + return false; + // The receiver's symbol must be the variable DECLARED by an enclosing `foreach`. + var forEach = m.Expression.Ancestors().OfType() + .FirstOrDefault(f => SymbolEqualityComparer.Default.Equals( + model.GetDeclaredSymbol(f), loopVar)); + if (forEach is null) + return false; + // The iterated collection must be a bare `Field` / `this.Field` of THIS class — a + // `other.Items` reaches another instance's collection (unknown element lifetime). + var collExpr = StripCasts(forEach.Expression); + if (collExpr is not (IdentifierNameSyntax + or MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax })) + return false; + var member = model.GetSymbolInfo(collExpr).Symbol; + if (member is not (IFieldSymbol or IPropertySymbol)) + return false; + if (!SymbolEqualityComparer.Default.Equals(member.ContainingType, cls)) + return false; + return MemberPopulatedByOwnFactory(member, model, cls); +} + +// #229: is EVERY population of this field/property an own-produced value — and is there +// at least one? Scans EVERY partial declaration of the containing type (a disqualifying +// injected assignment may live in a sibling partial FILE — the merged compilation makes +// them all reachable through the symbol's DeclaringSyntaxReferences; CodeRabbit), each +// with its own tree's semantic model. An initializer at a declaration, or any simple +// assignment to the member in a ctor/method, is a population site. A single site we +// cannot prove is own-produced (a ctor parameter, an injected field, a service-located +// call) denies the proof, so an injected collection keeps today's honest warning. +static bool MemberPopulatedByOwnFactory(ISymbol member, SemanticModel model, + INamedTypeSymbol cls) +{ + var any = false; + bool Note(ExpressionSyntax value, SemanticModel m) + { + any = true; + return IsOwnProducedValue(value, m, cls, depth: 0); + } + foreach (var decl in EnumerateTypeDeclarations(cls)) + { + var m = model.Compilation.GetSemanticModel(decl.SyntaxTree); + // Declaration-site initializer: `= CreateData()` / `= new(){ ... }` on the field + // or auto-property. + foreach (var v in decl.DescendantNodes().OfType()) + if (v.Initializer is { } vi + && SymbolEqualityComparer.Default.Equals(m.GetDeclaredSymbol(v), member) + && !Note(vi.Value, m)) + return false; + foreach (var p in decl.DescendantNodes().OfType()) + if (p.Initializer is { } pi + && SymbolEqualityComparer.Default.Equals(m.GetDeclaredSymbol(p), member) + && !Note(pi.Value, m)) + return false; + // Assignment sites: `Items1 = CreateData();` + foreach (var asg in decl.DescendantNodes().OfType()) + if (asg.IsKind(SyntaxKind.SimpleAssignmentExpression) + && SymbolEqualityComparer.Default.Equals(m.GetSymbolInfo(asg.Left).Symbol, member) + && !Note(asg.Right, m)) + return false; + } + return any; +} + +// #229: a value the class PRODUCES itself, not one that FLOWS IN — and whose ELEMENTS are +// themselves own-produced (the subscribed events live on those elements, so a collection +// object that is freshly `new`d but SEEDED with injected elements is NOT owned). Two +// forms, checked recursively (Codex P1): +// * an object creation whose constructor arguments AND collection-initializer elements +// are each own-produced — `new List()`, `new() { new Model(), new Model() }`; +// `new List(injectedSource)` and `new() { injectedModel }` are rejected; +// * an invocation of an OWN factory (a method of THIS class with an implicit/`this` +// receiver) whose body PROVABLY returns an own-produced value — so a wrapper +// `List GetData() => _service.GetData()` that forwards injected elements is +// rejected. A call through any other receiver (`_service.GetData()`) is likewise out. +// Casts/`!` are peeled first; depth-bounded so a self-referential factory cannot spin. +static bool IsOwnProducedValue(ExpressionSyntax expr, SemanticModel model, + INamedTypeSymbol cls, int depth) +{ + if (depth > 3) + return false; + expr = StripCasts(expr); + if (expr is BaseObjectCreationExpressionSyntax oce) + { + // Constructor arguments may SEED the collection with external elements. + if (oce.ArgumentList is { } al) + foreach (var arg in al.Arguments) + if (!IsOwnProducedValue(arg.Expression, model, cls, depth + 1)) + return false; + // A collection/object initializer's elements must each be own-produced. + if (oce.Initializer is { } init) + foreach (var e in init.Expressions) + if (!IsOwnProducedValue(e, model, cls, depth + 1)) + return false; + return true; + } + if (expr is InvocationExpressionSyntax inv) + { + var callee = model.GetSymbolInfo(inv).Symbol as IMethodSymbol; + if (callee is null + || !SymbolEqualityComparer.Default.Equals(callee.ContainingType, cls)) + return false; + var receiverIsOwn = inv.Expression switch + { + IdentifierNameSyntax => true, // CreateData() + MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } => true, // this.CreateData() + _ => false, + }; + return receiverIsOwn && FactoryReturnsOwnProduced(callee, model, cls, depth); + } + return false; +} + +// #229: does this own-class factory PROVABLY return only own-produced values? Inspects +// its body (same-class, so within the class-level scan the guardrail allows): an +// expression body `=> expr`, or every `return expr` in a block body, must be own-produced. +// No visible body, a bodiless return, or any non-own-produced return denies the proof — +// so a wrapper that forwards an injected/service-located collection keeps the warning. +// (Returns nested in lambdas/local functions only make the check MORE conservative.) +static bool FactoryReturnsOwnProduced(IMethodSymbol factory, SemanticModel model, + INamedTypeSymbol cls, int depth) +{ + if (factory.DeclaringSyntaxReferences.Length == 0) + return false; + var any = false; + foreach (var r in factory.DeclaringSyntaxReferences) + { + if (r.GetSyntax() is not MethodDeclarationSyntax md) + return false; + var m = model.Compilation.GetSemanticModel(md.SyntaxTree); + if (md.ExpressionBody is { } eb) + { + any = true; + if (!IsOwnProducedValue(eb.Expression, m, cls, depth + 1)) + return false; + continue; + } + if (md.Body is null) + return false; + var returns = md.Body.DescendantNodes().OfType().ToList(); + if (returns.Count == 0) + return false; + foreach (var ret in returns) + { + any = true; + if (ret.Expression is null + || !IsOwnProducedValue(ret.Expression, m, cls, depth + 1)) + return false; + } + } + return any; +} + // P-004 WPF MVVM ownership: a field read from `this.DataContext`, optionally through // an `as`/cast (`DataContext as VM`, `(VM)DataContext`). Combined with a view whose // own XAML CONSTRUCTS its DataContext, such a field is the view's owned view-model. @@ -4400,6 +4568,16 @@ or ImplicitObjectCreationExpressionSyntax if (!isTimer && clsIsBehavior && IsAssociatedObjectSource(a.Left, model, cls)) continue; + // P-004 / issue #229: the `+=` receiver is an element of a collection the + // class itself populated — a `foreach (var x in OwnField)` loop variable + // whose collection is a this-owned field/property assigned from the class's + // OWN construction/factory (not a ctor parameter, not an injected value). + // The collection and its elements share the constructing object's lifetime, + // so the subscription is the same collectable self-cycle as a constructed + // field; a `foreach` over an injected/parameter collection keeps today's + // warning (its population sites are not own-produced). + if (!isTimer && IsOwnedCollectionElementSource(a.Left, model, clsSymbol)) + continue; // P-004 (issue #223): the curated weak-referenced-static-event allowlist — // unconditional (unlike the AppDomain exemption above, this does NOT gate on // HandlerRetainsNoInstance: the whole point of a weak-referenced source is diff --git a/frontend/roslyn/samples/OwnedCollectionElementSample.cs b/frontend/roslyn/samples/OwnedCollectionElementSample.cs new file mode 100644 index 00000000..ed6813a5 --- /dev/null +++ b/frontend/roslyn/samples/OwnedCollectionElementSample.cs @@ -0,0 +1,215 @@ +// issue #229 — subscribing to an element of a collection the class itself populated. +// A constructor (or a method it calls) fills a this-owned collection FIELD/PROPERTY +// from the class's own factory / a collection initializer, then subscribes to each +// element in a loop. The collection and its elements share the constructing object's +// lifetime, so `element.PropertyChanged += ...` is a collectable self-cycle, not a leak. +// Real-world shape: MaterialDesignInXamlToolkit MainDemo.Wpf ListsAndGridsViewModel.cs:16-17 +// (found by the issue #201 oracle sweep). +// +// The exemption is deliberately NARROW: the loop variable must be provably drawn from a +// this-owned member the class populates ITSELF (own construction / own factory), never a +// ctor parameter or an injected/service-located value. The negative controls below pin +// each edge. +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace OwnSamples.OwnedCollection +{ + public class Model + { + public event EventHandler? PropertyChanged; + public void Bump() => PropertyChanged?.Invoke(this, EventArgs.Empty); + } + + // Injected/service-located source of models — its elements' lifetimes are unknown. + public interface IModelService + { + List GetData(); + } + + public class ViewModelBase + { + protected void OnPropertyChanged(string name) { } + } + + // POSITIVE (silent): the ListsAndGridsViewModel shape — an own FACTORY fills the + // property, then a `foreach` over it subscribes each element. + public class OwnedFactoryViewModel : ViewModelBase + { + public ObservableCollection Items1 { get; set; } + + public OwnedFactoryViewModel() + { + Items1 = CreateData(); // own factory fills the member + foreach (var model in Items1) + model.PropertyChanged += (s, a) => OnPropertyChanged("Items1"); // silent (#229) + } + + ObservableCollection CreateData() => new() { new Model(), new Model() }; + } + + // POSITIVE (silent): a FIELD populated by a collection initializer at its own + // declaration, iterated in the constructor. + public class InlineNewViewModel : ViewModelBase + { + readonly List _items = new() { new Model() }; + + public InlineNewViewModel() + { + foreach (var m in _items) + m.PropertyChanged += OnChanged; // silent (#229) + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // POSITIVE (silent): the member is assigned `new ObservableCollection(...)` + // directly (own construction), then iterated. `this.`-qualified collection access. + public class DirectNewViewModel : ViewModelBase + { + ObservableCollection _rows; + + public DirectNewViewModel() + { + _rows = new ObservableCollection(); + foreach (var r in this._rows) + r.PropertyChanged += OnChanged; // silent (#229) + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 1 (flagged, REQUIRED): the collection is a FIELD assigned from a ctor + // PARAMETER — injected, unknown element lifetime — so the subscription stays flagged. + public class InjectedCollectionViewModel : ViewModelBase + { + readonly IEnumerable _injected; + + public InjectedCollectionViewModel(IEnumerable injected) + { + _injected = injected; + foreach (var m in _injected) + m.PropertyChanged += OnChanged; // OWN001: injected collection + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 2 (flagged, REQUIRED): the `foreach` iterates a ctor PARAMETER collection + // directly (not a this-owned member at all). + public class ParamCollectionViewModel : ViewModelBase + { + public ParamCollectionViewModel(IEnumerable models) + { + foreach (var m in models) + m.PropertyChanged += OnChanged; // OWN001: parameter collection + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 3 (flagged): the member is populated from a SERVICE-LOCATED call + // (`_service.GetData()`) — the elements are produced by an injected service, not the + // class's own factory, so their lifetime is unknown. + public class ServiceLocatedViewModel : ViewModelBase + { + readonly IModelService _service; + List _items; + + public ServiceLocatedViewModel(IModelService service) + { + _service = service; + _items = _service.GetData(); + foreach (var m in _items) + m.PropertyChanged += OnChanged; // OWN001: service-located source + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 4 (flagged): the member is populated by the own factory in the ctor but + // ALSO reassigned from an injected value elsewhere — its contents at the `+=` are + // ambiguous, so every population site must be own-produced or the proof is denied. + public class MixedCollectionViewModel : ViewModelBase + { + List _items; + + public MixedCollectionViewModel() + { + _items = CreateData(); + foreach (var m in _items) + m.PropertyChanged += OnChanged; // OWN001: _items also injected below + } + + public void Replace(List injected) => _items = injected; + + List CreateData() => new() { new Model() }; + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 5 (flagged, Codex P1): the member is a freshly-`new`d collection but SEEDED + // from an injected parameter — the collection object is own-constructed, yet its + // ELEMENTS (which carry the subscribed events) are injected, so it is not owned. + public class SeededNewViewModel : ViewModelBase + { + readonly List _items; + + public SeededNewViewModel(IEnumerable injected) + { + _items = new List(injected); + foreach (var m in _items) + m.PropertyChanged += OnChanged; // OWN001: seeded from injected elements + } + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 6 (flagged, Codex P1): an own-class factory that FORWARDS an injected + // service's collection — a same-class call, but its body returns service-located + // elements of unknown lifetime, so the factory result is not own-produced. + public class ForwardingFactoryViewModel : ViewModelBase + { + readonly IModelService _service; + List _items; + + public ForwardingFactoryViewModel(IModelService service) + { + _service = service; + _items = MakeData(); + foreach (var m in _items) + m.PropertyChanged += OnChanged; // OWN001: factory forwards injected data + } + + List MakeData() => _service.GetData(); + + void OnChanged(object? sender, EventArgs e) { } + } + + // CONTROL 7 (flagged, CodeRabbit): a PARTIAL class whose own-factory population lives + // in one declaration but a DISQUALIFYING injected assignment lives in the sibling + // partial — the population scan must cover every partial of the type, not just the + // declaration holding the `foreach`. + public partial class PartialInjectedViewModel : ViewModelBase + { + List _items; + + public PartialInjectedViewModel() + { + _items = CreateData(); + foreach (var m in _items) + m.PropertyChanged += OnChanged; // OWN001: _items also injected in the sibling partial + + } + + List CreateData() => new() { new Model() }; + + void OnChanged(object? sender, EventArgs e) { } + } + + public partial class PartialInjectedViewModel + { + public void Seed(List injected) => _items = injected; // sibling-partial injected write + } +}