diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52278707..13b3fdaf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,8 @@ jobs: frontend/roslyn/samples/SampleTypes.cs \ frontend/roslyn/samples/PipeFieldsSample.cs \ frontend/roslyn/samples/AppLifetimeSample.cs \ + frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs \ + frontend/roslyn/samples/InjectedDcViewSample.xaml.cs \ -o "$RUNNER_TEMP/facts.json" cat "$RUNNER_TEMP/facts.json" - name: Check facts through the core @@ -292,6 +294,18 @@ jobs: if echo "$out" | grep -q "AppLifetimeSample.cs"; then echo "FAIL: a process-lived App static-event subscription was wrongly reported (OWN014 FP)"; exit 1 fi + # P-004 WPF MVVM ownership (mined: ScreenToGif VideoSource): a view that + # CONSTRUCTS its view-model in its own XAML (`` — read from + # the sibling .xaml) owns it, so a field assigned from `DataContext` is + # self-owned and subscribing to its events is a collectable cycle -> SILENT. + if echo "$out" | grep -q "ViewOwnsVmSample"; then + echo "FAIL: a view that owns its VM via its own XAML DataContext was wrongly reported"; exit 1 + fi + # negative control: a view whose XAML BINDS its DataContext (``) does + # NOT own the VM (it may be externally supplied), so the subscription must + # still WARN — proving the gate keys off proven construction, not every cast. + echo "$out" | grep -qE "InjectedDcViewSample\.xaml\.cs:[0-9]+: warning: \[OWN001\].*injected dependency whose lifetime is unknown" \ + || { echo "FAIL: a bound (unowned) DataContext subscription must still warn with the injected-source wording"; exit 1; } # P-006 DI001 (captive dependency): the registration + constructor graph # extracted from DiCaptiveSample.cs feeds ownlang/di.py. A singleton that # captures a scoped service — directly, transitively through a transient, diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 091baec2..a05fb7aa 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -24,6 +24,7 @@ // repo (this is what the `own-check` script / GitHub Action do). using System.Text.Json; +using System.Xml.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -220,6 +221,57 @@ static bool IsProcessLivedApplication(TypeDeclarationSyntax cls) && cls.Modifiers.Any(m => m.IsKind(SyntaxKind.PartialKeyword)); } +// 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. +static bool ReadsDataContext(ExpressionSyntax expr) +{ + expr = expr switch + { + BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left, + CastExpressionSyntax c => c.Expression, + _ => expr, + }; + return expr switch + { + IdentifierNameSyntax id => id.Identifier.Text == "DataContext", + MemberAccessExpressionSyntax m => m.Name.Identifier.Text == "DataContext" + && m.Expression is ThisExpressionSyntax, + _ => false, + }; +} + +// P-004 WPF MVVM ownership: does this XAML construct its own DataContext inline — +// `` — so the view OWNS its view-model +// (a collectable view<->VM cycle)? Parsed structurally (XAML is XML), restricted to the +// ROOT element's own DataContext: the code-behind's `this.DataContext` is the root's, so +// a nested `` (a different element's) must NOT exempt the +// view (Codex / CodeRabbit) — else a real leak on the root's injected VM is dropped. +// True only when the root's DataContext child is a constructed object that the view owns: +// NOT a binding / resource reference, and NOT an `x:`-namespace language object +// (`x:Static`, `x:Null`, `x:Reference`, ...), which name an external/shared value. A +// malformed `.xaml` yields false (conservative — no exemption, never a dropped leak). +static bool XamlDeclaresOwnedDataContext(string xaml) +{ + XDocument doc; + try { doc = XDocument.Parse(xaml); } + catch (System.Xml.XmlException) { return false; } + var root = doc.Root; + if (root is null) + return false; + // The root's OWN `` property-element (a direct child of the root, + // in the root's namespace) — not a nested element's, not another property. + var dc = root.Element(root.Name.Namespace + (root.Name.LocalName + ".DataContext")); + var child = dc?.Elements().FirstOrDefault(); + if (child is null) + return false; + if (child.Name.NamespaceName == "http://schemas.microsoft.com/winfx/2006/xaml") + return false; // x:Static / x:Null / x:Reference / x:Type / ... + return child.Name.LocalName is not ("Binding" or "MultiBinding" or "PriorityBinding" + or "StaticResource" or "DynamicResource" or "RelativeSource" + or "TemplateBinding" or "Reference" or "Null"); +} + // P-004 severity tiering: of the subscriptions that survive the self-owned and // static-handler exemptions (and are not timers), how long-lived is the event // SOURCE? A static event lives for the whole process, so an undetached handler is @@ -1805,6 +1857,12 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) // it under), then build ONE compilation over all of them so the SemanticModel // resolves cross-file and cross-project symbols (P-014 Tier A). var parsed = new List<(string file, SyntaxTree tree)>(); +// P-004 WPF MVVM: source-file paths (`Foo.xaml.cs`) whose sibling `Foo.xaml` constructs +// its own DataContext (``) — the view OWNS that VM. The +// extractor only parses `.cs`, so XAML-declared ownership is invisible from the C# +// alone; we read the sibling `.xaml` here and the subscription detector then treats a +// field assigned from `this.DataContext` as self-owned. Keyed by the tree's FilePath. +var viewsOwningDataContext = new HashSet(StringComparer.Ordinal); foreach (var path in inputs) { // Defensive: an explicit input that is not a readable file (a directory @@ -1827,6 +1885,19 @@ static bool IsPublicCtor(SyntaxTokenList modifiers) continue; } parsed.Add((Rel(path), CSharpSyntaxTree.ParseText(text, path: path))); + if (path.EndsWith(".xaml.cs", StringComparison.OrdinalIgnoreCase)) + { + var xamlPath = path[..^3]; // strip ".cs" -> "....xaml" + try + { + if (File.Exists(xamlPath) && XamlDeclaresOwnedDataContext(File.ReadAllText(xamlPath))) + viewsOwningDataContext.Add(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // An unreadable sibling `.xaml` just means "ownership unknown" — no exemption. + } + } } // Project-local compilation (P-014 Tier A): the framework reference set is this @@ -1936,6 +2007,17 @@ or ImplicitObjectCreationExpressionSyntax && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol tf && IsTemplatePartFetch(a.Right)) selfOwned.Add(tf.Name); + // * WPF MVVM view-model — `_vm = DataContext as VM`: when THIS view's own + // XAML constructs its DataContext (recorded in viewsOwningDataContext from + // the sibling `.xaml`), the view owns that VM, so the view<->VM cycle is + // collectable and subscribing to its events is not a leak. (Mined from + // ScreenToGif's VideoSource: 4 FP subscriptions to its own declared VM.) + if (viewsOwningDataContext.Contains(tree.FilePath)) + foreach (var a in assigns) + if (a.IsKind(SyntaxKind.SimpleAssignmentExpression) + && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol dcf + && ReadsDataContext(a.Right)) + selfOwned.Add(dcf.Name); // Is this class the process-lived WPF application object? Used to drop the // static-source region escape (OWN014) — `App` cannot be over-promoted. @@ -1975,8 +2057,10 @@ or ImplicitObjectCreationExpressionSyntax continue; // Process-lived subscriber (the WPF `App` singleton): a static-source // subscription promotes nothing — `App` already lives for the whole - // process — so the region escape (OWN014) is a false positive. - if (source == "static" && clsIsApp) + // process — so the region escape (OWN014) is a false positive. Scoped + // to NON-timers: a timer is forced to source "static" above, but a + // never-stopped timer in `App` is still a real leak (CodeRabbit). + if (!isTimer && source == "static" && clsIsApp) continue; var released = unsub.Contains($"{a.Left}|{a.Right}") || (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv)); diff --git a/frontend/roslyn/samples/InjectedDcViewSample.xaml b/frontend/roslyn/samples/InjectedDcViewSample.xaml new file mode 100644 index 00000000..a070b9f4 --- /dev/null +++ b/frontend/roslyn/samples/InjectedDcViewSample.xaml @@ -0,0 +1,18 @@ + + + + + + + + + + + + diff --git a/frontend/roslyn/samples/InjectedDcViewSample.xaml.cs b/frontend/roslyn/samples/InjectedDcViewSample.xaml.cs new file mode 100644 index 00000000..b9ba1bd0 --- /dev/null +++ b/frontend/roslyn/samples/InjectedDcViewSample.xaml.cs @@ -0,0 +1,31 @@ +using System; + +namespace Own.Samples.Wpf; + +// Negative control for the XAML-ownership exemption. This view's XAML BINDS its +// DataContext (`` — see the sibling +// InjectedDcViewSample.xaml): it does NOT construct the VM, the DataContext is +// inherited / externally supplied and may outlive the view. So the field is NOT owned +// and the subscription must still WARN (OWN001). This proves the gate suppresses only +// PROVEN construction, not every `DataContext as T`. +public partial class InjectedDcView +{ + private readonly InjectedVm _vm; + + public InjectedDcView() + { + _vm = DataContext as InjectedVm; + } + + public void OnLoaded() + { + _vm.Changed += OnChanged; // unowned source -> possible leak -> warns + } + + private void OnChanged(object sender, EventArgs e) { } +} + +public class InjectedVm +{ + public event EventHandler Changed; +} diff --git a/frontend/roslyn/samples/ViewOwnsVmSample.xaml b/frontend/roslyn/samples/ViewOwnsVmSample.xaml new file mode 100644 index 00000000..344654a1 --- /dev/null +++ b/frontend/roslyn/samples/ViewOwnsVmSample.xaml @@ -0,0 +1,9 @@ + + + + + + diff --git a/frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs b/frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs new file mode 100644 index 00000000..c704b623 --- /dev/null +++ b/frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs @@ -0,0 +1,30 @@ +using System; + +namespace Own.Samples.Wpf; + +// P-004 WPF MVVM ownership (mined: ScreenToGif VideoSource). This view constructs its +// view-model in its OWN XAML (`<...DataContext>` — see the +// sibling ViewOwnsVmSample.xaml), so it OWNS the VM: the view<->VM reference cycle is +// GC-collectable and subscribing to the VM's events is NOT a leak. The extractor reads +// the sibling `.xaml` to see this (it parses only `.cs` otherwise). Must be SILENT. +public partial class ViewOwnsVm +{ + private readonly OwnedVm _vm; + + public ViewOwnsVm() + { + _vm = DataContext as OwnedVm; + } + + public void OnLoaded() + { + _vm.Changed += OnChanged; // owned source -> collectable cycle -> silent + } + + private void OnChanged(object sender, EventArgs e) { } +} + +public class OwnedVm +{ + public event EventHandler Changed; +}