Skip to content

Commit 72438b9

Browse files
committed
fix(extractor): restrict XAML DataContext-ownership to the ROOT element (Codex P2)
XamlDeclaresOwnedDataContext matched ANY `*.DataContext` property-element in the sibling XAML. But the code-behind's `this.DataContext` is the ROOT element's, so a nested `<Grid.DataContext><ChildVm/>` (a different element's DataContext) would wrongly mark the whole `.xaml.cs` as owning — silently dropping a real OWN001 on the root's injected/bound VM. Now: strip comments / processing-instructions, find the root element tag, and match only `<Root.DataContext>` with a constructed (type-instantiation) child. The InjectedDcView control XAML gains exactly this shape — a BOUND root DataContext plus a nested `<Grid.DataContext><local:NestedChildVm/>` — so the existing "must warn" assertion now also guards the root-restriction (without it the nested construction would make the view look owned and the subscription would be wrongly suppressed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent cb2bcc3 commit 72438b9

2 files changed

Lines changed: 36 additions & 11 deletions

File tree

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

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,17 +242,32 @@ BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left,
242242
}
243243

244244
// P-004 WPF MVVM ownership: does this XAML construct its own DataContext inline —
245-
// `<Owner.DataContext><vm:Foo/></Owner.DataContext>` — so the view OWNS its
246-
// view-model (a collectable view<->VM cycle)? True only when the property-element's
247-
// child is a TYPE instantiation, not a binding / resource reference (those point at
248-
// an external or inherited value the view does NOT own, so a subscription to it can
249-
// still leak). Matched textually because the extractor has no XAML parser: the
250-
// property-element form `.DataContext>` is what denotes inline construction (the
251-
// `DataContext="{Binding}"` attribute form never matches). Conservative — an
252-
// unrecognised shape yields false (no exemption), never a wrongly-suppressed leak.
245+
// `<Root.DataContext><vm:Foo/></Root.DataContext>` — so the view OWNS its view-model
246+
// (a collectable view<->VM cycle)? Restricted to the ROOT element's DataContext: the
247+
// code-behind's `this.DataContext` is the root's, so only the root's own constructed
248+
// DataContext proves ownership. A NESTED element's `<Grid.DataContext><ChildVm/>` sets
249+
// a DIFFERENT element's DataContext and must NOT exempt the view (Codex) — else a real
250+
// leak on the root's injected VM is silently dropped. True only when the root's
251+
// property-element child is a TYPE instantiation, not a binding / resource reference
252+
// (those point at an external or inherited value the view does NOT own). Matched
253+
// textually (no XAML parser): the property-element form `.DataContext>` denotes inline
254+
// construction; the `DataContext="{Binding}"` attribute form never matches.
255+
// Conservative — an unrecognised shape yields false (no exemption), never a
256+
// wrongly-suppressed leak.
253257
static bool XamlDeclaresOwnedDataContext(string xaml)
254258
{
255-
foreach (Match m in Regex.Matches(xaml, @"\.DataContext\s*>\s*<\s*(?:[\w]+:)?([\w.]+)"))
259+
// Strip comments / processing instructions so `<Foo.DataContext>` text inside a
260+
// comment (or `<?xml?>`) cannot be mistaken for markup, nor skew the root search.
261+
xaml = Regex.Replace(xaml, @"<!--.*?-->", " ", RegexOptions.Singleline);
262+
xaml = Regex.Replace(xaml, @"<\?.*?\?>", " ", RegexOptions.Singleline);
263+
// The root element is the view type (its x:Class is the code-behind). Find its tag
264+
// (the first element open — not `</close>`), then match ONLY `<Root.DataContext>`.
265+
var rootMatch = Regex.Match(xaml, @"<\s*([A-Za-z_][\w:]*)");
266+
if (!rootMatch.Success)
267+
return false;
268+
var root = Regex.Escape(rootMatch.Groups[1].Value);
269+
foreach (Match m in Regex.Matches(xaml,
270+
$@"<\s*{root}\s*\.\s*DataContext\s*>\s*<\s*(?:[\w]+:)?([\w.]+)"))
256271
{
257272
var name = m.Groups[1].Value;
258273
var local = name.Contains('.') ? name[(name.LastIndexOf('.') + 1)..] : name;
Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,18 @@
11
<Window x:Class="Own.Samples.Wpf.InjectedDcView"
22
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
3-
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
4-
<!-- The view BINDS (does not construct) its DataContext: it does NOT own the VM. -->
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. -->
57
<Window.DataContext>
68
<Binding Path="Shared"/>
79
</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>
818
</Window>

0 commit comments

Comments
 (0)