Skip to content

Commit bed4868

Browse files
committed
refactor(extractor): parse XAML structurally for DataContext ownership (CodeRabbit)
Replace the textual regex in XamlDeclaresOwnedDataContext with structural XML parsing (XDocument), per CodeRabbit's recommendation. Strictly more correct: it inspects only the ROOT element's own `<Root.DataContext>` (a direct child, in the root's namespace) and excludes the entire `x:` language namespace — so x:Static / x:Null / x:Reference (external or shared values the view does NOT own) no longer count as construction, closing a residual false-negative the regex left open. A malformed `.xaml` parses to false (conservative). Drops the now-unused Regex import. Also tighten the InjectedDcView CI assertion to require the OWN001 warning AND the injected-source wording on that file's line specifically (CodeRabbit nitpick), so the negative control cannot be satisfied by an unrelated finding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent 72438b9 commit bed4868

2 files changed

Lines changed: 27 additions & 34 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,8 @@ jobs:
304304
# negative control: a view whose XAML BINDS its DataContext (`<Binding/>`) does
305305
# NOT own the VM (it may be externally supplied), so the subscription must
306306
# still WARN — proving the gate keys off proven construction, not every cast.
307-
echo "$out" | grep -qE "InjectedDcViewSample\.xaml\.cs:[0-9]+: warning: \[OWN001\]" \
308-
|| { echo "FAIL: a bound (unowned) DataContext subscription must still warn"; exit 1; }
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; }
309309
# P-006 DI001 (captive dependency): the registration + constructor graph
310310
# extracted from DiCaptiveSample.cs feeds ownlang/di.py. A singleton that
311311
# captures a scoped service — directly, transitively through a transient,

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

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

2626
using System.Text.Json;
27-
using System.Text.RegularExpressions;
27+
using System.Xml.Linq;
2828
using Microsoft.CodeAnalysis;
2929
using Microsoft.CodeAnalysis.CSharp;
3030
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -243,40 +243,33 @@ BinaryExpressionSyntax b when b.IsKind(SyntaxKind.AsExpression) => b.Left,
243243

244244
// P-004 WPF MVVM ownership: does this XAML construct its own DataContext inline —
245245
// `<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.
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).
257254
static bool XamlDeclaresOwnedDataContext(string xaml)
258255
{
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)
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)
267261
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.]+)"))
271-
{
272-
var name = m.Groups[1].Value;
273-
var local = name.Contains('.') ? name[(name.LastIndexOf('.') + 1)..] : name;
274-
if (local is not ("Binding" or "MultiBinding" or "PriorityBinding"
275-
or "StaticResource" or "DynamicResource" or "RelativeSource"
276-
or "TemplateBinding" or "Reference" or "Null"))
277-
return true;
278-
}
279-
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");
280273
}
281274

282275
// P-004 severity tiering: of the subscriptions that survive the self-owned and

0 commit comments

Comments
 (0)