Skip to content

Commit eba8c9a

Browse files
authored
Merge pull request #82 from PhysShell/claude/fp-view-owns-vm
fix(extractor): WPF view-owns-VM exemption via sibling XAML (mined VideoSource FPs)
2 parents fffa098 + bed4868 commit eba8c9a

6 files changed

Lines changed: 188 additions & 2 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ jobs:
137137
frontend/roslyn/samples/SampleTypes.cs \
138138
frontend/roslyn/samples/PipeFieldsSample.cs \
139139
frontend/roslyn/samples/AppLifetimeSample.cs \
140+
frontend/roslyn/samples/ViewOwnsVmSample.xaml.cs \
141+
frontend/roslyn/samples/InjectedDcViewSample.xaml.cs \
140142
-o "$RUNNER_TEMP/facts.json"
141143
cat "$RUNNER_TEMP/facts.json"
142144
- name: Check facts through the core
@@ -292,6 +294,18 @@ jobs:
292294
if echo "$out" | grep -q "AppLifetimeSample.cs"; then
293295
echo "FAIL: a process-lived App static-event subscription was wrongly reported (OWN014 FP)"; exit 1
294296
fi
297+
# P-004 WPF MVVM ownership (mined: ScreenToGif VideoSource): a view that
298+
# CONSTRUCTS its view-model in its own XAML (`<X.DataContext><VM/>` — read from
299+
# the sibling .xaml) owns it, so a field assigned from `DataContext` is
300+
# self-owned and subscribing to its events is a collectable cycle -> SILENT.
301+
if echo "$out" | grep -q "ViewOwnsVmSample"; then
302+
echo "FAIL: a view that owns its VM via its own XAML DataContext was wrongly reported"; exit 1
303+
fi
304+
# negative control: a view whose XAML BINDS its DataContext (`<Binding/>`) does
305+
# NOT own the VM (it may be externally supplied), so the subscription must
306+
# still WARN — proving the gate keys off proven construction, not every cast.
307+
echo "$out" | grep -qE "InjectedDcViewSample\.xaml\.cs:[0-9]+: warning: \[OWN001\].*injected dependency whose lifetime is unknown" \
308+
|| { echo "FAIL: a bound (unowned) DataContext subscription must still warn with the injected-source wording"; exit 1; }
295309
# P-006 DI001 (captive dependency): the registration + constructor graph
296310
# extracted from DiCaptiveSample.cs feeds ownlang/di.py. A singleton that
297311
# captures a scoped service — directly, transitively through a transient,

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

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
// repo (this is what the `own-check` script / GitHub Action do).
2525

2626
using System.Text.Json;
27+
using System.Xml.Linq;
2728
using Microsoft.CodeAnalysis;
2829
using Microsoft.CodeAnalysis.CSharp;
2930
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -220,6 +221,57 @@ static bool IsProcessLivedApplication(TypeDeclarationSyntax cls)
220221
&& cls.Modifiers.Any(m => m.IsKind(SyntaxKind.PartialKeyword));
221222
}
222223

224+
// P-004 WPF MVVM ownership: a field read from `this.DataContext`, optionally through
225+
// an `as`/cast (`DataContext as VM`, `(VM)DataContext`). Combined with a view whose
226+
// own XAML CONSTRUCTS its DataContext, such a field is the view's owned view-model.
227+
static bool ReadsDataContext(ExpressionSyntax expr)
228+
{
229+
expr = expr switch
230+
{
231+
BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left,
232+
CastExpressionSyntax c => c.Expression,
233+
_ => expr,
234+
};
235+
return expr switch
236+
{
237+
IdentifierNameSyntax id => id.Identifier.Text == "DataContext",
238+
MemberAccessExpressionSyntax m => m.Name.Identifier.Text == "DataContext"
239+
&& m.Expression is ThisExpressionSyntax,
240+
_ => false,
241+
};
242+
}
243+
244+
// P-004 WPF MVVM ownership: does this XAML construct its own DataContext inline —
245+
// `<Root.DataContext><vm:Foo/></Root.DataContext>` — so the view OWNS its view-model
246+
// (a collectable view<->VM cycle)? Parsed structurally (XAML is XML), restricted to the
247+
// ROOT element's own DataContext: the code-behind's `this.DataContext` is the root's, so
248+
// a nested `<Grid.DataContext><ChildVm/>` (a different element's) must NOT exempt the
249+
// view (Codex / CodeRabbit) — else a real leak on the root's injected VM is dropped.
250+
// True only when the root's DataContext child is a constructed object that the view owns:
251+
// NOT a binding / resource reference, and NOT an `x:`-namespace language object
252+
// (`x:Static`, `x:Null`, `x:Reference`, ...), which name an external/shared value. A
253+
// malformed `.xaml` yields false (conservative — no exemption, never a dropped leak).
254+
static bool XamlDeclaresOwnedDataContext(string xaml)
255+
{
256+
XDocument doc;
257+
try { doc = XDocument.Parse(xaml); }
258+
catch (System.Xml.XmlException) { return false; }
259+
var root = doc.Root;
260+
if (root is null)
261+
return false;
262+
// The root's OWN `<Root.DataContext>` property-element (a direct child of the root,
263+
// in the root's namespace) — not a nested element's, not another property.
264+
var dc = root.Element(root.Name.Namespace + (root.Name.LocalName + ".DataContext"));
265+
var child = dc?.Elements().FirstOrDefault();
266+
if (child is null)
267+
return false;
268+
if (child.Name.NamespaceName == "http://schemas.microsoft.com/winfx/2006/xaml")
269+
return false; // x:Static / x:Null / x:Reference / x:Type / ...
270+
return child.Name.LocalName is not ("Binding" or "MultiBinding" or "PriorityBinding"
271+
or "StaticResource" or "DynamicResource" or "RelativeSource"
272+
or "TemplateBinding" or "Reference" or "Null");
273+
}
274+
223275
// P-004 severity tiering: of the subscriptions that survive the self-owned and
224276
// static-handler exemptions (and are not timers), how long-lived is the event
225277
// SOURCE? A static event lives for the whole process, so an undetached handler is
@@ -1805,6 +1857,12 @@ static bool IsPublicCtor(SyntaxTokenList modifiers)
18051857
// it under), then build ONE compilation over all of them so the SemanticModel
18061858
// resolves cross-file and cross-project symbols (P-014 Tier A).
18071859
var parsed = new List<(string file, SyntaxTree tree)>();
1860+
// P-004 WPF MVVM: source-file paths (`Foo.xaml.cs`) whose sibling `Foo.xaml` constructs
1861+
// its own DataContext (`<Foo.DataContext><VM/></...>`) — the view OWNS that VM. The
1862+
// extractor only parses `.cs`, so XAML-declared ownership is invisible from the C#
1863+
// alone; we read the sibling `.xaml` here and the subscription detector then treats a
1864+
// field assigned from `this.DataContext` as self-owned. Keyed by the tree's FilePath.
1865+
var viewsOwningDataContext = new HashSet<string>(StringComparer.Ordinal);
18081866
foreach (var path in inputs)
18091867
{
18101868
// Defensive: an explicit input that is not a readable file (a directory
@@ -1827,6 +1885,19 @@ static bool IsPublicCtor(SyntaxTokenList modifiers)
18271885
continue;
18281886
}
18291887
parsed.Add((Rel(path), CSharpSyntaxTree.ParseText(text, path: path)));
1888+
if (path.EndsWith(".xaml.cs", StringComparison.OrdinalIgnoreCase))
1889+
{
1890+
var xamlPath = path[..^3]; // strip ".cs" -> "....xaml"
1891+
try
1892+
{
1893+
if (File.Exists(xamlPath) && XamlDeclaresOwnedDataContext(File.ReadAllText(xamlPath)))
1894+
viewsOwningDataContext.Add(path);
1895+
}
1896+
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
1897+
{
1898+
// An unreadable sibling `.xaml` just means "ownership unknown" — no exemption.
1899+
}
1900+
}
18301901
}
18311902

18321903
// Project-local compilation (P-014 Tier A): the framework reference set is this
@@ -1936,6 +2007,17 @@ or ImplicitObjectCreationExpressionSyntax
19362007
&& model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol tf
19372008
&& IsTemplatePartFetch(a.Right))
19382009
selfOwned.Add(tf.Name);
2010+
// * WPF MVVM view-model — `_vm = DataContext as VM`: when THIS view's own
2011+
// XAML constructs its DataContext (recorded in viewsOwningDataContext from
2012+
// the sibling `.xaml`), the view owns that VM, so the view<->VM cycle is
2013+
// collectable and subscribing to its events is not a leak. (Mined from
2014+
// ScreenToGif's VideoSource: 4 FP subscriptions to its own declared VM.)
2015+
if (viewsOwningDataContext.Contains(tree.FilePath))
2016+
foreach (var a in assigns)
2017+
if (a.IsKind(SyntaxKind.SimpleAssignmentExpression)
2018+
&& model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol dcf
2019+
&& ReadsDataContext(a.Right))
2020+
selfOwned.Add(dcf.Name);
19392021

19402022
// Is this class the process-lived WPF application object? Used to drop the
19412023
// static-source region escape (OWN014) — `App` cannot be over-promoted.
@@ -1975,8 +2057,10 @@ or ImplicitObjectCreationExpressionSyntax
19752057
continue;
19762058
// Process-lived subscriber (the WPF `App` singleton): a static-source
19772059
// subscription promotes nothing — `App` already lives for the whole
1978-
// process — so the region escape (OWN014) is a false positive.
1979-
if (source == "static" && clsIsApp)
2060+
// process — so the region escape (OWN014) is a false positive. Scoped
2061+
// to NON-timers: a timer is forced to source "static" above, but a
2062+
// never-stopped timer in `App` is still a real leak (CodeRabbit).
2063+
if (!isTimer && source == "static" && clsIsApp)
19802064
continue;
19812065
var released = unsub.Contains($"{a.Left}|{a.Right}")
19822066
|| (isTimer && Receiver(a.Left) is { } recv && stopped.Contains(recv));
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<Window x:Class="Own.Samples.Wpf.InjectedDcView"
2+
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
4+
xmlns:local="clr-namespace:Own.Samples.Wpf">
5+
<!-- The ROOT view BINDS (does not construct) its DataContext: it does NOT own the
6+
VM that the code-behind reads via this.DataContext. -->
7+
<Window.DataContext>
8+
<Binding Path="Shared"/>
9+
</Window.DataContext>
10+
<!-- Codex case: a NESTED element constructs its OWN child VM. This is a DIFFERENT
11+
element's DataContext, not the view's this.DataContext, so it must NOT make the
12+
view look owned — the subscription must still warn. -->
13+
<Grid>
14+
<Grid.DataContext>
15+
<local:NestedChildVm/>
16+
</Grid.DataContext>
17+
</Grid>
18+
</Window>
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using System;
2+
3+
namespace Own.Samples.Wpf;
4+
5+
// Negative control for the XAML-ownership exemption. This view's XAML BINDS its
6+
// DataContext (`<Window.DataContext><Binding/></...>` — see the sibling
7+
// InjectedDcViewSample.xaml): it does NOT construct the VM, the DataContext is
8+
// inherited / externally supplied and may outlive the view. So the field is NOT owned
9+
// and the subscription must still WARN (OWN001). This proves the gate suppresses only
10+
// PROVEN construction, not every `DataContext as T`.
11+
public partial class InjectedDcView
12+
{
13+
private readonly InjectedVm _vm;
14+
15+
public InjectedDcView()
16+
{
17+
_vm = DataContext as InjectedVm;
18+
}
19+
20+
public void OnLoaded()
21+
{
22+
_vm.Changed += OnChanged; // unowned source -> possible leak -> warns
23+
}
24+
25+
private void OnChanged(object sender, EventArgs e) { }
26+
}
27+
28+
public class InjectedVm
29+
{
30+
public event EventHandler Changed;
31+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Window x:Class="Own.Samples.Wpf.ViewOwnsVm"
2+
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
4+
xmlns:local="clr-namespace:Own.Samples.Wpf">
5+
<!-- The view CONSTRUCTS its own view-model here: it owns it. -->
6+
<Window.DataContext>
7+
<local:OwnedVm/>
8+
</Window.DataContext>
9+
</Window>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using System;
2+
3+
namespace Own.Samples.Wpf;
4+
5+
// P-004 WPF MVVM ownership (mined: ScreenToGif VideoSource). This view constructs its
6+
// view-model in its OWN XAML (`<...DataContext><OwnedVm/></...DataContext>` — see the
7+
// sibling ViewOwnsVmSample.xaml), so it OWNS the VM: the view<->VM reference cycle is
8+
// GC-collectable and subscribing to the VM's events is NOT a leak. The extractor reads
9+
// the sibling `.xaml` to see this (it parses only `.cs` otherwise). Must be SILENT.
10+
public partial class ViewOwnsVm
11+
{
12+
private readonly OwnedVm _vm;
13+
14+
public ViewOwnsVm()
15+
{
16+
_vm = DataContext as OwnedVm;
17+
}
18+
19+
public void OnLoaded()
20+
{
21+
_vm.Changed += OnChanged; // owned source -> collectable cycle -> silent
22+
}
23+
24+
private void OnChanged(object sender, EventArgs e) { }
25+
}
26+
27+
public class OwnedVm
28+
{
29+
public event EventHandler Changed;
30+
}

0 commit comments

Comments
 (0)