diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 064ea0f3..34d2c0cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,7 @@ jobs: python audit/aggregate/normalize.py --selftest python audit/aggregate/score.py --selftest python audit/aggregate/report.py --selftest + python audit/static/tools/xaml_check.py --selftest python audit/static/run_static.py --selftest python audit/runtime/ingest.py --selftest diff --git a/audit/README.md b/audit/README.md index 8f21ad68..a8a4fe09 100644 --- a/audit/README.md +++ b/audit/README.md @@ -47,6 +47,7 @@ audit/ tools/ owncheck.py # build-free runner: own-check.sh --format sarif (needs dotnet) codeql.sh # build-free runner: CodeQL build-mode=none, security-and-quality + xaml_check.py # build-free runner: markup-only XAML perf/lifetime pass (stdlib XML, no SDK) roslyn_pack.ps1 # build-required runner (local Windows): NetAnalyzers/Roslynator/... infersharp.sh # build-required runner: Infer# over built binaries inject/ # OwnAudit.Directory.Build.props/.targets (analyzer injection, gated) @@ -65,7 +66,7 @@ audit/ | Tier | Tools | Needs a successful build of the target? | |---|---|---| -| **build-free** | own-check, CodeQL (`build-mode: none`) | no — works on a solution that does not compile | +| **build-free** | own-check, CodeQL (`build-mode: none`), XAML markup pass | no — works on a solution that does not compile | | **build-required** | Roslyn analyzer packs, Infer# | yes | The entire audit of the target runs on a **local Windows machine** (VS Build Tools @@ -108,6 +109,7 @@ Linux CI: python audit/aggregate/normalize.py --selftest python audit/aggregate/score.py --selftest python audit/aggregate/report.py --selftest +python audit/static/tools/xaml_check.py --selftest # XAML rules + line preservation + SARIF round-trip python audit/static/run_static.py --selftest # full pipeline end-to-end on fixtures ``` @@ -122,6 +124,16 @@ python audit/static/run_static.py --selftest # full pipeline end-to-end on fix to the coverage ledger), DevExpress baseline-suppress, cross-tool agreement scoring, the pain heatmap, **all four renderers (markdown / json / merged SARIF / HTML)**, the analyzer-injection props/targets, and selftests. +- **XAML analyzer (Phase 1, markup-only) — done:** a build-free, stdlib-XML pass + (`static/tools/xaml_check.py`) feeding the same pipeline as a second fact source — + line-preserving parse, the canonical SARIF record, and rules XAML101/102/103/104/ + 106/107/108/109/110/111/112/113 (virtualization-off, per-keystroke binding, template + complexity, Freezable/x:Shared/DynamicResource/merged-dictionary perf, image + decode-at-full-size, LayoutTransform cost, TemplateBinding opportunities, and inline + Freezable duplication). This makes category 8 (broken virtualization) statically + covered, not NO-TOOL. Design + the full rule catalogue, phasing, and the Phase-2 + binding-path join: [`../docs/notes/xaml-analyzer-design.md`](../docs/notes/xaml-analyzer-design.md). + Phase 2 (Roslyn-linked XAML2xx) and Phase 3 (runtime correlation) are deferred. - **Runtime (Phase 2) — started:** the runtime→pipeline bridge (`runtime/ingest.py`, CI-gated), the leak-harness scenario schema + one scenario, runtime rule mappings in the taxonomy (categories 2/3/4/11), and the C# leak-harness skeleton. See diff --git a/audit/config/profiles/desktop-wpf.yml b/audit/config/profiles/desktop-wpf.yml index ac1451b8..62d6d03f 100644 --- a/audit/config/profiles/desktop-wpf.yml +++ b/audit/config/profiles/desktop-wpf.yml @@ -19,6 +19,8 @@ tiers: build_free: - own-check # error-tolerant SemanticModel; works on a broken solution - codeql # build-mode: none, security-and-quality suite + - xaml # markup-only XAML pass (stdlib XML, no SDK needed); + # XAML perf/lifetime rules — docs/notes/xaml-analyzer-design.md build_required: - roslyn-pack # NetAnalyzers, Meziantou, Roslynator, AsyncFixer, # SonarAnalyzer, IDisposableAnalyzers, WpfAnalyzers, @@ -43,9 +45,11 @@ roslyn_packs: no_tool_static: - 4 # DependencyPropertyDescriptor.AddValueChanged leak -> runtime leak-harness - 6 # PropertyChanged storms / expensive getters -> runtime + # (XAML108 gives a static per-keystroke-binding suspicion; storms stay runtime) - 7 # WPF binding errors -> runtime - - 8 # broken/disabled virtualization -> runtime - - 10 # allocations in converters/getters -> runtime + # NOTE: category 8 (broken/disabled virtualization) is no longer NO-TOOL: + # the build-free XAML pass covers it statically (XAML107/XAML109). + - 10 # allocations in converters/getters -> runtime (XAML phase 2) - 11 # duplicated immutable data (the project's "gold") -> runtime - 12 # heavy reference data / LOH / Gen2 bloat -> runtime - 13 # cross-thread ObjectDisposedException / INPC -> runtime diff --git a/audit/static/run_static.py b/audit/static/run_static.py index f9a80949..ed3bfada 100755 --- a/audit/static/run_static.py +++ b/audit/static/run_static.py @@ -36,6 +36,7 @@ from owncheck import run_own_check # noqa: E402 from report import render_html, render_json, render_markdown, render_sarif # noqa: E402 from score import score # noqa: E402 +from xaml_check import run_xaml_check # noqa: E402 try: from oracle_compare import parse_sarif @@ -123,6 +124,14 @@ def run(target: str, profile: dict[str, Any], out_dir: Path, target_name: str = tiers.append(st) if st["available"] and st["sarif"]: sarif_inputs.append(("codeql", st["sarif"])) + if "xaml" in build_free: + # The markup-only XAML pass (docs/notes/xaml-analyzer-design.md, phase 1): + # pure stdlib XML, so it has no toolchain prerequisite and always runs here, + # emitting the same SARIF record into the same aggregate pipeline. + st = run_xaml_check(target, out_dir) + tiers.append(st) + if st["available"] and st["sarif"]: + sarif_inputs.append(("xaml", st["sarif"])) # Pick up any build-required SARIFs already dropped here by the Windows runners. # Roslyn writes ONE SARIF PER PROJECT under roslyn/ (see the injected props's @@ -305,6 +314,27 @@ def check(ok: bool, msg: str) -> None: # total derives from the call count check(res2["totals"]["high_confidence"] >= 1, "runtime leak + static finding in one file must form a high-confidence cluster") + # The build-free XAML tier must wire in like own-check/codeql: with "xaml" in + # build_free and a .xaml under the target, run() reports a xaml tier and the + # markup finding rides the pipeline through to a scored cluster — all on Linux, + # no SDK (the whole point of the markup-only phase). + with tempfile.TemporaryDirectory() as td3: + out3 = Path(td3) / "out" + src3 = Path(td3) / "src" / "Views" + src3.mkdir(parents=True) + (src3 / "Main.xaml").write_text( + '\n' + ' \n' + '\n', encoding="utf-8") + profile3 = {"name": "t", "severity_floor": "warning", + "tiers": {"build_free": ["xaml"]}} + res3 = run(str(Path(td3) / "src"), profile3, out3, target_name="t/p") + check(any(t["tool"] == "xaml" and t["available"] for t in res3["tiers"]), + "xaml build-free tier must run and be reported by run()") + check(res3["totals"]["candidates"] >= 1, + "a XAML107 markup finding must flow through to a scored cluster") + fails = [c for c in checks if c] for f in fails: print(f"RUN_STATIC SELFTEST FAIL: {f}") diff --git a/audit/static/taxonomy/categories.yml b/audit/static/taxonomy/categories.yml index 380b804f..878a7d21 100644 --- a/audit/static/taxonomy/categories.yml +++ b/audit/static/taxonomy/categories.yml @@ -43,6 +43,26 @@ rules: # ── Category 9: WPF Freezable / per-instance brush-geometry (partial) ──────── "WPF0*": {category: 9, name: wpf-freezable} # WpfAnalyzers (subset) + # ── XAML analyzer (build-free markup pass) — docs/notes/xaml-analyzer-design.md. + # XAML is a SECOND fact source, not a parallel linter: each rule maps to one of + # the same Plan.md §2 categories so a markup finding rides the same fingerprint + # -> baseline -> ratchet path as a .cs finding. Exact ids win over the XAML1* + # glob, which is a safety net so a not-yet-mapped XAML rule never lands in + # `uncategorized` (the design note's "nothing quietly falls through"). + "XAML107": {category: 8, name: broken-virtualization} # virtualization disabled + "XAML109": {category: 8, name: template-complexity} # visual-tree inflation + "XAML108": {category: 6, name: binding-update-frequency} # per-keystroke source flood + "XAML106": {category: 9, name: wpf-freezable} # Freezable not frozen + "XAML101": {category: 9, name: per-instance-resource} # duplicate converter + "XAML102": {category: 9, name: dynamic-resource-misuse} # DynamicResource for a static key + "XAML103": {category: 9, name: per-instance-resource} # x:Shared=False + "XAML104": {category: 9, name: merged-dictionary-waste} # duplicate merged dict + "XAML110": {category: 9, name: image-decode} # full-size thumbnail decode + "XAML111": {category: 8, name: layout-cost} # LayoutTransform layout pass + "XAML112": {category: 9, name: template-binding-opportunity} # cheaper compiled binding + "XAML113": {category: 9, name: per-instance-resource} # duplicated inline freezable + "XAML1*": {category: 9, name: xaml-markup-perf} # safety net for future rules + # ── Category 14: general bugs / perf / best-practice / async ────────────────── "CA1*": {category: 14, name: general-quality} # NetAnalyzers design/perf "CA2*": {category: 14, name: general-quality} # (CA2000/CA2213 above win by exactness) @@ -78,6 +98,7 @@ category_severity: 4: P1 # DependencyPropertyDescriptor.AddValueChanged leak (runtime-confirmed) 5: P2 6: P2 # PropertyChanged storms/cascades — runtime raise-frequency, perf-tier + 8: P2 # broken/disabled virtualization — now statically covered by the XAML pass 9: P2 11: P2 # duplicated immutable data — memory bloat (the project's "gold"), perf-tier 14: P2 diff --git a/audit/static/tools/xaml_check.py b/audit/static/tools/xaml_check.py new file mode 100644 index 00000000..5b0cf2ab --- /dev/null +++ b/audit/static/tools/xaml_check.py @@ -0,0 +1,980 @@ +#!/usr/bin/env python3 +""" +Own.NET Audit — XAML analyzer runner (build-free tier). + +Phase 1 of the XAML analyzer described in ``docs/notes/xaml-analyzer-design.md``: +a *markup-only* static pass over ``.xaml`` / ``.axaml`` that needs **no .NET build +and no stand**. It is a second fact source feeding the existing ``audit/`` pipeline +— it emits the same SARIF record the own-check / CodeQL runners do, so a XAML +finding rides the existing normalize → score → SARIF → baseline → ratchet path for +free (the one architectural decision in the design note: *XAML is another fact +source, not a parallel linter*). + +Because it is pure stdlib XML (no ``dotnet``, no ``lxml``), it runs on Linux in CI +like the other build-free runners here, and — unlike own-check — it has **no +toolchain prerequisite**, so it is never NO-TOOL for lack of an SDK. + +Line preservation is a hard requirement, not a detail (design note): a naive +``ElementTree.parse`` drops source positions and ``report/sarif.py`` maps a +missing/0 line to SARIF ``startLine=1``, which would point *every* XAML alert at +the top of the file. We therefore build the tree through ``expat`` directly, +stamping each element with ``CurrentLineNumber`` — stdlib, no third-party dep. A +rule that can only locate a *file-level* issue emits line 0 on purpose (parse_sarif +records 0; report/sarif.py then omits the region rather than fabricating line 1), +so it stays honestly file-level instead of mis-pinning. + +Phase-1 rules implemented here (see the catalogue in the design note). Each is the +perf/lifetime axis that WpfAnalyzers / PropertyChangedAnalyzers do **not** cover — +we deliberately do not re-implement their correctness rules: + + XAML101 DuplicateStatelessConverterResource (cat 9) per-instance resource churn + XAML102 DynamicResourceLikelyStatic (cat 9) WPF-only, deferred-lookup cost + XAML103 SuspiciousSharedFalse (cat 9) WPF-only, x:Shared opt-out + XAML104 DuplicateMergedDictionaryInclude (cat 9) wasted load + order ambiguity + XAML106 FreezableResourceShouldFreeze (cat 9) WPF-only, change-notify overhead + XAML107 VirtualizationExplicitlyDisabled (cat 8) virtualization accidentally killed + XAML108 PerKeystrokeBindingWithoutDelay (cat 6) per-keystroke source flooding + XAML109 TemplateComplexityHigh (cat 8) visual-tree / layout inflation + XAML110 ImageDecodedAtFullSize (cat 9) WPF-only, thumbnail full-size decode + XAML111 LayoutTransformSuspicious (cat 8) WPF-only, layout-pass cost + XAML112 TemplateBindingOpportunity (cat 9) cheaper compiled binding available + XAML113 InlineFreezableDuplication (cat 9) identical inline brush/geometry + +Deferred to a later Phase-1 slice (documented so nothing on the wishlist quietly +falls through — the design note's stated goal): XAML100 ResourceShouldBeHoisted +(needs the cross-sibling scope model) and XAML105 MergedDictionaryKeyShadowing +across *external* dictionaries (needs cross-file resolution). Phase 2 (Roslyn-linked +XAML2xx) and Phase 3 (runtime correlation) live elsewhere per the design note. + +WPF-only rules (XAML102/103/106) are skipped on Avalonia ``.axaml`` because the +``DynamicResource`` / ``x:Shared`` / ``Freezable`` semantics differ or do not exist +(the today/never line from the coverage matrix). + +Usage: + xaml_check.py --target /path/to/legacy/src --out artifacts/own-audit + xaml_check.py --selftest +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +import xml.parsers.expat +from collections.abc import Callable +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +# ItemsControl-family types whose virtualization we care about (XAML107/109 anchors). +ITEMS_CONTROLS = { + "ListBox", "ListView", "DataGrid", "TreeView", "ComboBox", "ItemsControl", + "GridView", "DataGridControl", "Selector", "HeaderedItemsControl", +} +# Panels that DO virtualize — an ItemsPanel of any other panel disables it. +VIRTUALIZING_PANELS = {"VirtualizingStackPanel", "VirtualizingPanel", "ItemsRepeater"} +NON_VIRTUALIZING_PANELS = {"StackPanel", "WrapPanel", "UniformGrid", "DockPanel", "Canvas"} + +# Freezable resource types that benefit from PresentationOptions:Freeze (XAML106). +FREEZABLE_TYPES = { + "SolidColorBrush", "LinearGradientBrush", "RadialGradientBrush", "ImageBrush", + "DrawingBrush", "GeometryDrawing", "ImageDrawing", "DrawingImage", "DrawingGroup", + "PathGeometry", "StreamGeometry", "RectangleGeometry", "EllipseGeometry", + "LineGeometry", "CombinedGeometry", "GeometryGroup", "MatrixTransform", + "RotateTransform", "ScaleTransform", "SkewTransform", "TranslateTransform", + "TransformGroup", "BitmapImage", +} +# Resource keys that are legitimately dynamic (theme/system) — XAML102 must skip. +DYNAMIC_KEY_PREFIXES = ("System", "Theme", "{x:Static") +DYNAMIC_KEY_TYPES = ("SystemColors", "SystemParameters", "SystemFonts") + +# Binding markers for XAML108 (per-keystroke source updates). +_TWOWAY_RE = re.compile(r"Mode\s*=\s*TwoWay", re.IGNORECASE) +_PROPCHANGED_RE = re.compile(r"UpdateSourceTrigger\s*=\s*PropertyChanged", re.IGNORECASE) +_DELAY_RE = re.compile(r"\bDelay\s*=", re.IGNORECASE) +_BINDING_RE = re.compile(r"\{\s*Binding\b", re.IGNORECASE) +# Properties whose source update genuinely floods per keystroke when un-delayed. +EDITABLE_PROPS = {"Text", "Value", "SelectedText", "SearchText", "FilterText", "Password"} + +# Markers that make a Freezable un-freezable (XAML106 exception list). +_DYNAMIC_REF_RE = re.compile(r"\{\s*(DynamicResource|Binding|TemplateBinding|x:Reference)\b", + re.IGNORECASE) + +# XAML112 — a TemplatedParent binding that could be the cheaper {TemplateBinding}. +_TPARENT_RE = re.compile(r"TemplatedParent", re.IGNORECASE) +_CONVERTER_RE = re.compile(r"\bConverter\s*=", re.IGNORECASE) +# XAML110 — an Image whose explicit display size is at or below this is a thumbnail, +# so a full-size decode (string Source, no DecodePixelWidth) is wasteful. +THUMBNAIL_MAX_DIP = 96.0 + +TEMPLATE_TYPES = {"ControlTemplate", "DataTemplate", "HierarchicalDataTemplate", + "ItemsPanelTemplate"} +TRIGGER_TYPES = {"Trigger", "DataTrigger", "MultiTrigger", "MultiDataTrigger", + "EventTrigger"} + + +@dataclass +class Node: + """A line-stamped XML element. ``tag`` keeps the source prefix (e.g. ``x:Key``, + ``ListBox.ItemsPanel``); helpers below strip it when matching.""" + + tag: str + attrib: dict[str, str] + line: int + children: list[Node] = field(default_factory=list) + parent: Node | None = None + + def local(self) -> str: + """The unqualified element name: ``controls:DataGrid`` -> ``DataGrid``; + a property element ``ListBox.ItemsPanel`` -> ``ItemsPanel`` (the last dotted + part), so callers test ``is_property_element`` first when they need either.""" + bare = self.tag.split(":", 1)[-1] + return bare.rsplit(".", 1)[-1] + + def type_name(self) -> str: + """The owning type name, ignoring any property-element suffix: + ``ListBox.ItemsPanel`` -> ``ListBox``; ``controls:DataGrid`` -> ``DataGrid``.""" + bare = self.tag.split(":", 1)[-1] + return bare.split(".", 1)[0] + + def is_property_element(self) -> bool: + return "." in self.tag.split(":", 1)[-1] + + def attr(self, name: str) -> str | None: + """Attribute by local name, prefix-insensitive (``x:Key`` matches ``Key``).""" + for k, v in self.attrib.items(): + if k.split(":", 1)[-1] == name: + return v + return None + + def walk(self): + yield self + for c in self.children: + yield from c.walk() + + +def parse_xaml(text: str | bytes) -> Node | None: + """Build a line-stamped ``Node`` tree from XAML markup via expat, or ``None`` if + the markup is not well-formed (a broken file is skipped, never a crash — the + continue-on-error discipline of the static layer). expat tracks + ``CurrentLineNumber`` so every element carries its real source line; names are + kept *as written* (prefixes intact), which is exactly what XAML tree patterns + match on. + + Pass **bytes** for real files so expat decodes them itself, honoring the BOM and + the XML declaration's ``encoding`` (legacy WPF XAML is often UTF-16): a forced + UTF-8 decode would corrupt those before parsing. In-memory ``str`` fixtures (no + encoding declaration) are accepted too.""" + parser = xml.parsers.expat.ParserCreate() + root: list[Node | None] = [None] + stack: list[Node] = [] + + def start(name: str, attrs: dict[str, str]) -> None: + node = Node(tag=name, attrib=dict(attrs), line=parser.CurrentLineNumber, + parent=stack[-1] if stack else None) + if stack: + stack[-1].children.append(node) + else: + root[0] = node + stack.append(node) + + def end(_name: str) -> None: + if stack: + stack.pop() + + parser.StartElementHandler = start + parser.EndElementHandler = end + try: + parser.Parse(text, True) + except xml.parsers.expat.ExpatError: + return None + return root[0] + + +@dataclass +class XamlFinding: + rule: str + line: int + message: str + + +def _is_avalonia(root: Node) -> bool: + """Avalonia ``.axaml`` declares the avaloniaui default namespace; the WPF-only + rules key off this to stay on the right side of the today/never line.""" + for k, v in root.attrib.items(): + if k == "xmlns" or k.startswith("xmlns"): + if "avaloniaui" in v or "avalonia" in v.lower(): + return True + return False + + +# --------------------------------------------------------------------------- # +# Rules. Each takes the parsed root and yields XamlFindings stamped to the # +# offending element's real line. # +# --------------------------------------------------------------------------- # + +def _rule_virtualization(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML107 — virtualization explicitly disabled on a list-family control.""" + out: list[XamlFinding] = [] + for n in root.walk(): + if n.is_property_element(): + # An whose template panel is non-virtualizing kills it. + if n.local() == "ItemsPanel" and n.type_name() in ITEMS_CONTROLS: + tmpl = next((c for c in n.children if c.local() == "ItemsPanelTemplate"), + None) + panels = (tmpl.children if tmpl else n.children) + for p in panels: + if p.type_name() in NON_VIRTUALIZING_PANELS: + out.append(XamlFinding( + "XAML107", p.line, + f"{n.type_name()} uses a non-virtualizing ItemsPanel " + f"({p.type_name()}); large item counts realize every " + "container [resource: virtualization]")) + continue + if n.type_name() not in ITEMS_CONTROLS: + continue + # Attached/attribute opt-outs that switch virtualization off. + for k, v in n.attrib.items(): + local = k.split(":", 1)[-1].rsplit(".", 1)[-1] + if local == "IsVirtualizing" and v.strip().lower() == "false": + out.append(XamlFinding( + "XAML107", n.line, + f"{n.type_name()} sets {k}=False, disabling UI virtualization " + "[resource: virtualization]")) + elif local == "CanContentScroll" and v.strip().lower() == "false": + out.append(XamlFinding( + "XAML107", n.line, + f"{n.type_name()} sets {k}=False; pixel-scrolling defeats " + "container virtualization [resource: virtualization]")) + return out + + +def _template_score(tmpl: Node) -> tuple[int, dict[str, int]]: + """Weighted complexity of a template subtree: element count + panel-nesting depth + + trigger count + nested ItemsControl depth (design note's XAML109 factors).""" + nodes = 0 + triggers = 0 + items_depth = 0 + + def depth_of(node: Node, panel_depth: int, items: int) -> tuple[int, int]: + nonlocal nodes, triggers, items_depth + max_panel = panel_depth + for c in node.children: + if c.is_property_element(): + pmax, _ = depth_of(c, panel_depth, items) + max_panel = max(max_panel, pmax) + continue + nodes += 1 + tn = c.type_name() + if tn in TRIGGER_TYPES: + triggers += 1 + pd = panel_depth + (1 if tn in NON_VIRTUALIZING_PANELS + or tn in VIRTUALIZING_PANELS or tn == "Grid" else 0) + it = items + (1 if tn in ITEMS_CONTROLS else 0) + items_depth = max(items_depth, it) + cmax, _ = depth_of(c, pd, it) + max_panel = max(max_panel, cmax) + return max_panel, items + + max_panel, _ = depth_of(tmpl, 0, 0) + factors = {"nodes": nodes, "panel_depth": max_panel, + "triggers": triggers, "items_depth": items_depth} + score = nodes + 2 * max_panel + 3 * triggers + 4 * items_depth + return score, factors + + +def _rule_template_complexity(root: Node, avalonia: bool, + threshold: int = 40) -> list[XamlFinding]: + """XAML109 — a template whose weighted complexity exceeds ``threshold``. Each + realized item re-expands the whole subtree, so this is a per-item multiplier.""" + out: list[XamlFinding] = [] + for n in root.walk(): + if n.type_name() not in TEMPLATE_TYPES or n.is_property_element(): + continue + score, f = _template_score(n) + if score > threshold: + out.append(XamlFinding( + "XAML109", n.line, + f"{n.type_name()} complexity {score} > {threshold} " + f"(nodes={f['nodes']}, panel-depth={f['panel_depth']}, " + f"triggers={f['triggers']}, items-depth={f['items_depth']}); " + "every realized item re-expands this subtree " + "[resource: visual tree]")) + return out + + +def _rule_per_keystroke_binding(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML108 — TwoWay + UpdateSourceTrigger=PropertyChanged with no Delay on an + editable property: the source is hit on every keystroke. ``Text`` defaults to + ``LostFocus`` for a reason; ``Delay`` exists to throttle this.""" + out: list[XamlFinding] = [] + for n in root.walk(): + for k, v in n.attrib.items(): + if not _BINDING_RE.search(v) or not _PROPCHANGED_RE.search(v): + continue + prop = k.split(":", 1)[-1].rsplit(".", 1)[-1] + # Text/Value are TwoWay-by-default; otherwise require an explicit TwoWay. + two_way = _TWOWAY_RE.search(v) or prop in EDITABLE_PROPS + if prop in EDITABLE_PROPS and two_way and not _DELAY_RE.search(v): + out.append(XamlFinding( + "XAML108", n.line, + f"{n.type_name()}.{prop} binds TwoWay with " + "UpdateSourceTrigger=PropertyChanged and no Delay; the source " + "updates on every keystroke [resource: binding update]")) + return out + + +def _resource_dictionaries(root: Node): + """Yield every resource scope: an explicit ````, AND the + implicit dictionary of an ```` property element — the common WPF + syntax where keyed resources are direct children with no ```` + wrapper (e.g. ````). + + No double counting when an ```` wraps an explicit + ````: the wrapper's own direct children carry no ``x:Key`` (the + only child is the dictionary), so ``_keyed_resources`` yields nothing for it, while + the inner dictionary is still picked up by the type check.""" + for n in root.walk(): + if n.is_property_element(): + if n.local() == "Resources": + yield n + elif n.type_name() == "ResourceDictionary": + yield n + + +def _rule_duplicate_merged_dict(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML104 — the same dictionary Source merged more than once in one + MergedDictionaries block (wasted load + include-order ambiguity).""" + out: list[XamlFinding] = [] + for n in root.walk(): + if n.local() != "MergedDictionaries" or not n.is_property_element(): + continue + seen: dict[str, int] = {} + for c in n.children: + src = c.attr("Source") + if not src: + continue + key = src.strip().lower().replace("\\", "/") + if key in seen: + out.append(XamlFinding( + "XAML104", c.line, + f"merged dictionary '{src}' is included again (first at line " + f"{seen[key]}); wasted load and include-order ambiguity " + "[resource: merged dictionary]")) + else: + seen[key] = c.line + return out + + +def _keyed_resources(rd: Node): + """Direct keyed children of a ResourceDictionary (its declared resources).""" + for c in rd.children: + if c.is_property_element(): + continue + key = c.attr("Key") + if key: + yield key, c + + +def _rule_duplicate_converter(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML101 — an identical stateless converter declared in several dictionaries. + Stateless = a keyed element with no child content and no configuring attributes + beyond x:Key. Converters are normally one shared instance; duplication is churn. + Started with exact type match (design note: structural equivalence is later).""" + out: list[XamlFinding] = [] + first: dict[str, int] = {} + for rd in _resource_dictionaries(root): + for _key, c in _keyed_resources(rd): + tn = c.type_name() + if "Converter" not in tn: + continue + stateless = not c.children and all( + a.split(":", 1)[-1] in ("Key",) for a in c.attrib) + if not stateless: + continue + if tn in first: + out.append(XamlFinding( + "XAML101", c.line, + f"stateless converter {tn} re-declared (first at line " + f"{first[tn]}); converters are normally a single shared instance " + "[resource: converter]")) + else: + first[tn] = c.line + return out + + +def _rule_shared_false(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML103 (WPF-only) — x:Shared="False" outside the documented exceptions. + Resources are shared by default; x:Shared=False is the deliberate per-lookup- + instance opt-out, so it is worth a second look on converters/styles/brushes.""" + if avalonia: + return [] + out: list[XamlFinding] = [] + for n in root.walk(): + shared = n.attr("Shared") + if shared is not None and shared.strip().lower() == "false": + tn = n.type_name() + # The FrameworkElement/FrameworkContentElement template-insertion case is + # the legitimate reason to opt out — don't flag those. + if tn in ("FrameworkElement", "FrameworkContentElement"): + continue + out.append(XamlFinding( + "XAML103", n.line, + f"{tn} sets x:Shared=False; a fresh instance is built per lookup " + "instead of sharing one [resource: x:Shared]")) + return out + + +def _rule_freezable_freeze(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML106 (WPF-only) — a keyed Freezable with no bindings/dynamic-resource/ + animation and no PresentationOptions:Freeze="True". Freezing drops change- + notification overhead and working set. The exception list is load-bearing: a + Freezable that is animated, data-bound, or references a DynamicResource *cannot* + be frozen, so those must be skipped.""" + if avalonia: + return [] + out: list[XamlFinding] = [] + for rd in _resource_dictionaries(root): + for _key, c in _keyed_resources(rd): + if c.type_name() not in FREEZABLE_TYPES: + continue + # Already frozen? + if any(a.split(":", 1)[-1] == "Freeze" and v.strip().lower() == "true" + for a, v in c.attrib.items()): + continue + # Un-freezable: any descendant binding/dynamic-resource/x:Reference, or an + # animation/trigger child. + blob = json.dumps([ch.tag for ch in c.walk()]) + json.dumps( + [v for nn in c.walk() for v in nn.attrib.values()]) + if _DYNAMIC_REF_RE.search(blob): + continue + if any(ch.local().endswith("Animation") or ch.local() == "Storyboard" + for ch in c.walk()): + continue + out.append(XamlFinding( + "XAML106", c.line, + f"{c.type_name()} resource is not frozen; add " + "PresentationOptions:Freeze=\"True\" to drop change-notification " + "overhead [resource: freezable]")) + return out + + +def _rule_dynamic_resource_static(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML102 (WPF-only) — DynamicResource for a key that is defined locally and is + not a theme/system key. StaticResource is recommended unless the value is + runtime-mutated; DynamicResource carries a deferred-lookup cost per use.""" + if avalonia: + return [] + # Collect locally-declared resource keys (lexically stable, app-local). + local_keys: set[str] = set() + for rd in _resource_dictionaries(root): + for key, _c in _keyed_resources(rd): + local_keys.add(key) + out: list[XamlFinding] = [] + pat = re.compile(r"\{\s*DynamicResource\s+([^}]+)\}", re.IGNORECASE) + for n in root.walk(): + for _k, v in n.attrib.items(): + m = pat.search(v) + if not m: + continue + key = m.group(1).strip() + if key.startswith(DYNAMIC_KEY_PREFIXES) or any( + key.startswith(t) for t in DYNAMIC_KEY_TYPES): + continue + if key in local_keys: + out.append(XamlFinding( + "XAML102", n.line, + f"DynamicResource '{key}' resolves a lexically-stable, app-local " + "resource; StaticResource avoids the deferred-lookup cost " + "[resource: dynamic resource]")) + return out + + +def _rule_image_decode(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML110 (WPF-only) — a thumbnail-sized Image whose Source is a plain URI + string: WPF decodes the bitmap at full native size, then scales down every + layout. A BitmapImage with DecodePixelWidth/Height decodes straight to the + display size (less working set, less GPU upload). The decode hint cannot be set + on a string Source, so the fix is the explicit BitmapImage form.""" + if avalonia: + return [] + out: list[XamlFinding] = [] + for n in root.walk(): + if n.type_name() != "Image" or n.is_property_element(): + continue + src = n.attr("Source") + if not src or src.strip().startswith("{"): # binding / markup ext: can't tell + continue + dims = [] + for d in ("Width", "Height"): + v = n.attr(d) + if v is None: + continue + try: + dims.append(float(v.strip())) + except ValueError: + continue # Auto / * + if dims and min(dims) <= THUMBNAIL_MAX_DIP: + out.append(XamlFinding( + "XAML110", n.line, + f"Image is shown at <={int(min(dims))}px but Source '{src}' is a " + "plain URI; WPF decodes it at full size. Use a BitmapImage with " + "DecodePixelWidth to decode-to-size [resource: image decode]")) + return out + + +def _in_control_template(node: Node) -> bool: + """True if ``node`` is nested inside a ControlTemplate (where TemplatedParent — + and therefore TemplateBinding — is meaningful).""" + p = node.parent + while p is not None: + if p.type_name() == "ControlTemplate" and not p.is_property_element(): + return True + p = p.parent + return False + + +def _rule_template_binding_opportunity(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML112 — inside a ControlTemplate, a {Binding RelativeSource=TemplatedParent} + that carries no Converter and is not TwoWay could be the cheaper {TemplateBinding} + (compiled, no full Binding object). A candidate, not a guarantee: TemplateBinding + cannot do converters / two-way, which is exactly why those are excluded here.""" + out: list[XamlFinding] = [] + for n in root.walk(): + if not _in_control_template(n): + continue + for k, v in n.attrib.items(): + if (_BINDING_RE.search(v) and _TPARENT_RE.search(v) + and not _CONVERTER_RE.search(v) and not _TWOWAY_RE.search(v)): + prop = k.split(":", 1)[-1].rsplit(".", 1)[-1] + out.append(XamlFinding( + "XAML112", n.line, + f"{n.type_name()}.{prop} binds to TemplatedParent with no " + "converter/two-way; {TemplateBinding} is the cheaper compiled " + "form here [resource: template binding]")) + return out + + +def _inline_freezable_sig(node: Node) -> tuple[Any, ...]: + """Structural signature of an inline Freezable: type + non-key attributes + child + element types. Two inline values with the same signature are the same object + re-built per use, so they should be hoisted to one shared keyed resource.""" + attrs = tuple(sorted((k.split(":", 1)[-1], v) for k, v in node.attrib.items() + if k.split(":", 1)[-1] != "Key")) + kids = tuple(c.type_name() for c in node.children if not c.is_property_element()) + return (node.type_name(), attrs, kids) + + +def _rule_inline_freezable_duplication(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML113 — the same inline Freezable (brush/geometry/transform set directly as a + property value, not as a keyed resource) declared identically more than once. + Each occurrence is a separate object; hoisting to one keyed resource shares it. + Extends XAML100's hoisting story to the inline case (framework-agnostic).""" + out: list[XamlFinding] = [] + first: dict[tuple[Any, ...], int] = {} + for n in root.walk(): + if n.type_name() not in FREEZABLE_TYPES or n.is_property_element(): + continue + if n.attr("Key") is not None: + continue # already a shared keyed resource + if not (n.parent and n.parent.is_property_element()): + continue # only inline property values, not free-standing + sig = _inline_freezable_sig(n) + if not sig[1] and not sig[2]: + continue # empty/defaulted element — nothing to share + if sig in first: + out.append(XamlFinding( + "XAML113", n.line, + f"inline {n.type_name()} duplicates an identical one (first at line " + f"{first[sig]}); hoist it to a shared keyed resource instead of " + "rebuilding it per use [resource: inline freezable]")) + else: + first[sig] = n.line + return out + + +def _rule_layout_transform(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML111 (WPF-only) — a LayoutTransform where a RenderTransform would do. + LayoutTransform re-runs measure/arrange on every change; RenderTransform is a + cheap render-time matrix. Legitimate only when layout must react to the transform + (e.g. rotated text that reflows), so this is a candidate to review.""" + if avalonia: + return [] + out: list[XamlFinding] = [] + seen: set[int] = set() + for n in root.walk(): + # Property-element form: + if n.local() == "LayoutTransform" and n.is_property_element(): + if n.line not in seen: + seen.add(n.line) + out.append(XamlFinding( + "XAML111", n.line, + f"{n.type_name()} uses LayoutTransform, which forces a " + "measure/arrange pass on change; prefer RenderTransform unless " + "layout must react [resource: layout transform]")) + continue + # Attribute form (rare): LayoutTransform="..." + for k in n.attrib: + if k.split(":", 1)[-1].rsplit(".", 1)[-1] == "LayoutTransform": + if n.line not in seen: + seen.add(n.line) + out.append(XamlFinding( + "XAML111", n.line, + f"{n.type_name()} sets LayoutTransform, which forces a " + "measure/arrange pass on change; prefer RenderTransform " + "unless layout must react [resource: layout transform]")) + return out + + +RULES: list[Callable[[Node, bool], list[XamlFinding]]] = [ + _rule_virtualization, + _rule_template_complexity, + _rule_per_keystroke_binding, + _rule_duplicate_merged_dict, + _rule_duplicate_converter, + _rule_shared_false, + _rule_freezable_freeze, + _rule_dynamic_resource_static, + _rule_image_decode, + _rule_template_binding_opportunity, + _rule_inline_freezable_duplication, + _rule_layout_transform, +] + + +def analyze_text(text: str | bytes) -> list[XamlFinding]: + """All Phase-1 rules over one markup document (``str`` or raw ``bytes``). + Malformed markup -> no findings.""" + root = parse_xaml(text) + if root is None: + return [] + avalonia = _is_avalonia(root) + out: list[XamlFinding] = [] + for rule in RULES: + out.extend(rule(root, avalonia)) + return out + + +def _to_sarif(results: list[tuple[str, XamlFinding]]) -> dict[str, Any]: + """Canonical SARIF 2.1.0 — the same shape own-check / CodeQL emit, so the + existing parse_sarif reads it with no special-casing. A file-level finding + (line <= 0) omits the region so report/sarif.py keeps it file-level.""" + sarif_results: list[dict[str, Any]] = [] + for path, f in results: + phys: dict[str, Any] = {"artifactLocation": {"uri": path}} + if f.line >= 1: + phys["region"] = {"startLine": f.line} + sarif_results.append({ + "ruleId": f.rule, "level": "warning", + "message": {"text": f.message}, + "locations": [{"physicalLocation": phys}], + }) + return {"version": "2.1.0", + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [{"tool": {"driver": {"name": "xaml-check", + "informationUri": "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/physshell/own.net", + "rules": []}}, + "results": sarif_results}]} + + +def run_xaml_check(target: str, out_dir: Path) -> dict[str, Any]: + """Scan every ``.xaml`` / ``.axaml`` under ``target`` and write SARIF to + ``out_dir/xaml-check.sarif``. Always best-effort: a missing target or zero + markup files yields ``available=False`` with a reason, never a crash.""" + out_dir.mkdir(parents=True, exist_ok=True) + sarif_path = out_dir / "xaml-check.sarif" + status: dict[str, Any] = {"tool": "xaml", "tier": "build-free", + "available": False, "sarif": None, "reason": ""} + + root = Path(target) + if not root.exists(): + status["reason"] = f"target path does not exist: {target}" + return status + files = sorted(p for p in root.rglob("*") + if p.suffix.lower() in (".xaml", ".axaml") and p.is_file()) + if not files: + status["reason"] = "no .xaml/.axaml files under target" + return status + + results: list[tuple[str, XamlFinding]] = [] + scanned = 0 + for fp in files: + try: + # Read bytes, not text: expat then honors the BOM / XML-declaration + # encoding (UTF-16 legacy XAML), instead of a forced UTF-8 decode that + # would corrupt the markup and silently drop the file. + data = fp.read_bytes() + except OSError: + continue + scanned += 1 + rel = fp.relative_to(root).as_posix() + for f in analyze_text(data): + results.append((rel, f)) + + sarif_path.write_text(json.dumps(_to_sarif(results), indent=2), encoding="utf-8") + status.update(available=True, sarif=str(sarif_path), + findings=len(results), files_scanned=scanned) + return status + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description="Run the build-free XAML analyzer -> SARIF.") + ap.add_argument("--target", help="path to the target source tree") + ap.add_argument("--out", default="artifacts/own-audit", help="SARIF output directory") + ap.add_argument("--selftest", action="store_true", help="run built-in checks and exit") + args = ap.parse_args(argv) + + if args.selftest: + return _selftest() + if not args.target: + ap.error("--target is required (or use --selftest)") + + status = run_xaml_check(args.target, Path(args.out)) + print(json.dumps(status, indent=2)) + return 0 if status["available"] else 1 + + +# --------------------------------------------------------------------------- # +# Selftest — embedded markup fixtures exercising every rule + the hard # +# line-preservation requirement, so it gates on Linux CI like the other # +# build-free runners (no .NET, no files on disk needed). # +# --------------------------------------------------------------------------- # + +_WPF_NS = ('xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" ' + 'xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" ' + 'xmlns:PresentationOptions="http://schemas.microsoft.com/winfx/2006/xaml/presentation/options"') + + +def _selftest() -> int: + checks: list[str] = [] + + def check(ok: bool, msg: str) -> None: + checks.append("" if ok else msg) + + def rules(text: str) -> dict[str, XamlFinding]: + return {f.rule: f for f in analyze_text(text)} + + # Line preservation — the hard requirement. The flagged control sits on line 3, + # and the finding MUST carry line 3, not 1. + virt = (f'\n' + ' \n' + ' \n' + ' \n' + '\n') + r = rules(virt) + check("XAML107" in r, "XAML107 must flag IsVirtualizing=False") + check(r.get("XAML107") and r["XAML107"].line == 3, + f"XAML107 line must be preserved (expected 3, got " + f"{r['XAML107'].line if 'XAML107' in r else None})") + + # XAML107 via a non-virtualizing ItemsPanel. + panel = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + check("XAML107" in rules(panel), "XAML107 must flag a non-virtualizing ItemsPanel") + # A VirtualizingStackPanel ItemsPanel must NOT be flagged. + ok_panel = panel.replace("StackPanel", "VirtualizingStackPanel") + check("XAML107" not in rules(ok_panel), + "XAML107 false positive: VirtualizingStackPanel ItemsPanel is fine") + + # XAML108 — per-keystroke binding; the bound property must be editable. + keystroke = (f'\n' + ' \n' + '\n') + check("XAML108" in rules(keystroke), "XAML108 must flag un-delayed PropertyChanged TwoWay") + delayed = keystroke.replace("UpdateSourceTrigger=PropertyChanged", + "UpdateSourceTrigger=PropertyChanged, Delay=300") + check("XAML108" not in rules(delayed), "XAML108 false positive: Delay present") + non_editable = (f'\n' + ' \n' + '\n') + check("XAML108" not in rules(non_editable), + "XAML108 false positive: IsChecked is not a per-keystroke editable property") + + # XAML104 — duplicate merged dictionary include. + dup = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + r = rules(dup) + check("XAML104" in r, "XAML104 must flag a re-included dictionary") + check(r.get("XAML104") and r["XAML104"].line == 5, + "XAML104 must point at the duplicate include (line 5)") + + # XAML101 — duplicate stateless converter across dictionaries. + conv = (f'\n' + ' \n' + ' \n' + '\n') + check("XAML101" in rules(conv), "XAML101 must flag a re-declared stateless converter") + + # XAML103 — x:Shared=False (WPF only). + shared = (f'\n' + ' \n' + '\n') + check("XAML103" in rules(shared), "XAML103 must flag x:Shared=False") + + # XAML106 — unfrozen Freezable, with the exception list honoured. + freez = (f'\n' + ' \n' + '\n') + check("XAML106" in rules(freez), "XAML106 must flag an unfrozen brush") + frozen = freez.replace('Color="#FF112233"', + 'Color="#FF112233" PresentationOptions:Freeze="True"') + check("XAML106" not in rules(frozen), "XAML106 false positive: already frozen") + bound = (f'\n' + ' \n' + '\n') + check("XAML106" not in rules(bound), + "XAML106 false positive: a DynamicResource-referencing brush cannot be frozen") + + # XAML102 — DynamicResource for a locally-defined, non-theme key. + dynr = (f'\n' + ' \n' + ' \n' + '\n') + check("XAML102" in rules(dynr), "XAML102 must flag DynamicResource on a local static key") + sysr = dynr.replace("PanelBrush", "SystemColors.WindowBrushKey") + check("XAML102" not in rules(sysr), + "XAML102 false positive: system/theme keys are legitimately dynamic") + + # XAML109 — a heavy template trips the complexity threshold; a small one does not. + cells = "".join(f'' for i in range(45)) + heavy = (f'\n' + f' {cells}\n' + '\n') + check("XAML109" in rules(heavy), "XAML109 must flag an over-threshold template") + light = (f'\n' + ' \n' + '\n') + check("XAML109" not in rules(light), "XAML109 false positive: a tiny template is fine") + + # XAML110 — a thumbnail Image with a full-size string Source; big image is fine. + img = (f'\n' + ' \n' + '\n') + check("XAML110" in rules(img), "XAML110 must flag a thumbnail with a full-size source") + big = img.replace('Width="32" Height="32"', 'Width="512" Height="512"') + check("XAML110" not in rules(big), "XAML110 false positive: a full-size image is fine") + bound_src = img.replace('Source="Assets/logo.png"', 'Source="{Binding Icon}"') + check("XAML110" not in rules(bound_src), + "XAML110 false positive: a bound source size is unknowable from markup") + + # XAML112 — a TemplatedParent binding inside a ControlTemplate; converters exempt. + tb = (f'\n' + ' \n' + ' \n' + ' \n' + '\n') + check("XAML112" in rules(tb), "XAML112 must flag a TemplatedParent binding") + tb_conv = tb.replace("RelativeSource={RelativeSource TemplatedParent}}", + "RelativeSource={RelativeSource TemplatedParent}, " + "Converter={StaticResource c}}") + check("XAML112" not in rules(tb_conv), + "XAML112 false positive: a converter binding cannot become a TemplateBinding") + tb_outside = tb.replace("", + "").replace( + "", "") + check("XAML112" not in rules(tb_outside), + "XAML112 false positive: TemplatedParent is only meaningful in a ControlTemplate") + + # XAML113 — the same inline brush declared twice; a unique one is fine. + inline = (f'\n' + ' ' + '\n' + ' ' + '\n' + '\n') + r = rules(inline) + check("XAML113" in r, "XAML113 must flag a duplicated inline brush") + uniq = (f'\n' + ' ' + '\n' + ' ' + '\n' + '\n') + check("XAML113" not in rules(uniq), "XAML113 false positive: distinct inline brushes are fine") + + # XAML111 — LayoutTransform (property-element form); WPF-only. + lt = (f'\n' + ' ' + '\n' + '\n') + check("XAML111" in rules(lt), "XAML111 must flag a LayoutTransform") + + # Implicit dictionary (the common WPF syntax, no + # wrapper) must feed the keyed-resource rules — else XAML101/102/106 miss most files. + implicit = (f'\n' + ' \n' + ' \n' + ' \n' + '\n') + r = rules(implicit) + check("XAML106" in r, "keyed-resource rules must see implicit dictionaries") + check(r.get("XAML106") and r["XAML106"].line == 3, + "implicit-dictionary finding must keep the resource's real line (3)") + + # A UTF-16 file (BOM + encoding declaration) must be decoded by expat, not dropped: + # read-as-bytes lets the XML parser honor the declared encoding. + u16_src = (f'\n' + ' \n' + '\n') + u16 = ('\n' + u16_src).encode("utf-16") + check("XAML107" in {f.rule for f in analyze_text(u16)}, + "UTF-16 markup (BOM + declaration) must parse, not be silently dropped") + + # WPF-only rules must stay silent on Avalonia .axaml. + ava = ('\n' + ' \n' + ' \n' + '\n') + ar = rules(ava) + check(not ({"XAML106", "XAML103", "XAML102", "XAML110", "XAML111"} & set(ar)), + "WPF-only rules (102/103/106/110/111) must not fire on Avalonia markup") + + # Malformed markup must be skipped, never crash. + check(analyze_text("") == [], "malformed markup must yield no findings") + + # End-to-end SARIF: the emitted log must be readable by the shared parse_sarif, + # with the real line surviving the round-trip (the contract with the pipeline). + sarif = _to_sarif([ + ("Views/Main.xaml", XamlFinding("XAML107", 3, "x [resource: virtualization]")), + ("Views/Main.xaml", XamlFinding("XAML104", 0, "file-level x"))]) + here = Path(__file__).resolve() + sys.path.insert(0, str(here.parents[3] / "scripts")) + try: + from oracle_compare import parse_sarif + parsed = parse_sarif(json.dumps(sarif), "xaml", []) + by_rule = {f.rule: f for f in parsed} + check(by_rule.get("XAML107") and by_rule["XAML107"].line == 3, + "SARIF round-trip must preserve the element line (3)") + check(by_rule.get("XAML104") and by_rule["XAML104"].line == 0, + "a file-level finding must round-trip as line 0 (region omitted)") + except ImportError: + check(False, "could not import scripts/oracle_compare.parse_sarif for round-trip") + + fails = [c for c in checks if c] + for f in fails: + print(f"XAML_CHECK SELFTEST FAIL: {f}") + print(f"xaml_check selftest: {len(checks) - len(fails)}/{len(checks)} checks passed") + return 1 if fails else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docs/notes/xaml-analyzer-design.md b/docs/notes/xaml-analyzer-design.md new file mode 100644 index 00000000..ed739dcb --- /dev/null +++ b/docs/notes/xaml-analyzer-design.md @@ -0,0 +1,255 @@ +# Own.NET XAML analyzer — design note + +> **Status / home.** This design note was authored in OwnAudit +> (`docs/xaml-analyzer-design.md`) and now lives here, in Own.NET, alongside the +> analyzer it describes. **Phase 1 (markup-only) is implemented** as the build-free +> runner [`audit/static/tools/xaml_check.py`](../../audit/static/tools/xaml_check.py), +> wired into [`audit/static/run_static.py`](../../audit/static/run_static.py) and the +> `desktop-wpf` profile, with its rule→category map in +> [`audit/static/taxonomy/categories.yml`](../../audit/static/taxonomy/categories.yml). +> Implemented rules: XAML101/102/103/104/106/107/108/109/110/111/112/113. Phase 2 +> (Roslyn-linked) and Phase 3 (runtime correlation) remain as described below; the +> Phase-2 binding-path join is sketched in its own section near the end. + +The biggest honest gap in OwnAudit's `docs/wpf-audit-coverage.md` ("**XAML analyzer** — a large slice of the +wishlist lives in `.xaml`, not `.cs` … Biggest gap — and technically cheap: XAML is XML, rules are +tree patterns"). This note turns that gap into a concrete, phased plan with a per-rule catalogue, +each rule tagged **build-free / hybrid / runtime** and with its **Avalonia-mappability**, so nothing +on the wishlist quietly falls through and so the first slice can ship without waiting on the stand. + +The methodology is the project's own — *suspect statically → confirm at runtime → targeted fix → +re-measure* — pointed at markup. The whole point of this note is the **architectural seam**, not the +rule count. + +--- + +## Which repo builds this (read first) + +This is a **design note that lives in Own.NET, alongside the analyzer it describes in +`Own.NET/audit/`.** Per `README.md` / `Plan.md`, the audit is **canonical in +Own.NET** ("*Don't reimplement it here*"): the build-free static runners live in +`Own.NET/audit/static` (next to own-check and CodeQL), and the interprocedural lifetime engine the +hybrid rules feed (CFG lowering, dataflow, OWN001 acquire/release, OWN014 region-escape) lives in +Own.NET too — `OwnAudit/src/OwnAudit.Core` is a thin lift-out skeleton, **not** that engine. + +So the implementation homes are: + +- **Phase 1 (markup-only)** → a build-free XAML runner in **`Own.NET/audit/static`**, alongside the + other build-free static runners. **Done:** `audit/static/tools/xaml_check.py`. It emits the + canonical finding record into the same `audit/` aggregate pipeline. +- **Phase 2 (hybrid, Roslyn-linked)** → **Own.NET's interprocedural core**, because it needs the + Roslyn semantic model and the acquire/release engine that physically live there. +- **Phase 3 (runtime correlation)** → wherever the runtime correlation lands at lift-out time; today + the suspect/confirm split is prototyped in `OwnAudit/runtime/correlate.py`, canonical runtime in + `Own.NET/audit/runtime`. + +OwnAudit's role here is the **design note's origin + (post-lift-out) the consuming/orchestration +side**, not a parallel XAML checker. Everything below describes the analyzer's shape; "the same +pipeline" means **Own.NET's `audit/` pipeline**, not a new one in OwnAudit. + +## The one architectural decision + +**XAML is another fact source feeding the existing engine — not a parallel linter.** + +`audit/` already has one such fact source (the Roslyn/own-check static layer) whose findings flow +through normalize → score → SARIF → baseline → report, and a lifetime engine behind OWN001/OWN014. +XAML becomes a *second* fact source emitting the **same finding record** into that **same `audit/` +pipeline**. No new mechanism: a XAML finding rides the existing fingerprint → SARIF → baseline → +ratchet → drift path for free. + +``` + .cs ──(Roslyn extractor)──┐ + ├─► findings.json ─► fingerprint ─► SARIF / baseline / ratchet / drift + .xaml ──(XAML extractor)───┘ │ + runtime.json ──► correlate.py (confirm) +``` + +Concretely this means the XAML pass emits the canonical record +(`{tool, rule, category_name, resource, path, line, message, suppressed}`, `resource` a *description* +not a CLR type — same contract as the own-checks) and does **not** grow its own report/baseline/gate +code. The hybrid phase then links XAML facts to graph nodes; the runtime phase reuses +`correlate.py`'s suspect/confirm split verbatim. + +## Where this sits relative to existing analyzers (our niche) + +WpfAnalyzers / PropertyChangedAnalyzers are mature but cover **correctness**: dependency-property +declaration, `MarkupExtensionReturnType`, converter boilerplate (e.g. WPF0070 "add default field to +converter"), `INotifyPropertyChanged` plumbing. They do **not** target XAML **performance/lifetime** +pathologies — resource-scope bloat, `DynamicResource` misuse, merged-dictionary shadowing, +virtualization disablement, expensive converter hot paths. That perf/lifetime axis is our lane; +we should not re-implement their correctness rules. + +--- + +## Phase 1 — markup-only static pass (build-free, runs in CI) + +Pure XML: parse `.xaml`/`.axaml`, resolve resource scopes, build a merged-dictionary graph. +**No .NET build, no stand** — a build-free runner in `Own.NET/audit/static` that runs on Linux in CI +like the other build-free runners there. This is the cheapest deliverable of the analyzer and closes +~half the ⚠️ rows in the coverage matrix. + +> **Implementation note (this repo).** Shipped as `audit/static/tools/xaml_check.py` — pure stdlib, +> so it has *no* toolchain prerequisite and always runs on Linux CI (unlike own-check, which needs a +> .NET SDK). The runner's selftest (`xaml_check.py --selftest`) gates the rules, the +> line-preservation requirement, and the SARIF round-trip through the shared `parse_sarif`. Of the +> catalogue below it implements XAML101/102/103/104/106/107/108/109/110, plus three rules added from +> the research-comb feedback — XAML111 (LayoutTransform cost), XAML112 (TemplateBinding opportunity) +> and XAML113 (inline-Freezable duplication). XAML100 (cross-sibling scope model) and XAML105 +> (cross-*file* dictionary shadowing) are the documented deferred tail. + +**Line preservation is a hard requirement, not a detail.** A plain `xml.etree.ElementTree.parse` +discards source positions, but our finding contract requires a real `line` and `report/sarif.py` +maps a missing/0 line to SARIF `startLine=1` — so a naive ElementTree pass would point *every* +XAML alert at the top of the file in code scanning and the dashboard. The parse step must therefore +be **line-preserving** while staying stdlib (still build-free): expat already tracks +`CurrentLineNumber`, so building the tree through an expat `StartElementHandler` that stamps each +element's start line gives us per-element lines with no third-party dependency — no `lxml`. (The +shipped runner does exactly this in `parse_xaml`.) Every rule below resolves its finding to the +offending element's stamped line; a rule that can only locate a file-level issue says so explicitly +(emits line 0, which `report/sarif.py` keeps file-level) rather than silently emitting line 1. + +| Rule | What it flags | Doc rationale | Avalonia | +|---|---|---|---| +| **XAML100** `ResourceShouldBeHoisted` | heavy shared resource (Brush/Style/Geometry/Transform/BitmapImage/template) declared in a control-local dictionary, recurring across siblings | per-instance control resources multiply working set; app/window scope shares (the 52×52 Brush collapse) | ✅ scope model maps | +| **XAML101** `DuplicateStatelessConverterResource` | identical stateless converter declared in many local dictionaries | converters are normally one shared instance; duplication is churn | ✅ | +| **XAML102** `DynamicResourceLikelyStatic` | `DynamicResource` for an app-local, lexically-stable, non-theme/system key | StaticResource recommended unless runtime-mutated; dynamic carries deferred lookup cost | ❌ Avalonia DynamicResource semantics differ | +| **XAML103** `SuspiciousSharedFalse` | `x:Shared="False"` on converters/styles/brushes outside documented exceptions | resources shared by default; `x:Shared=false` is the deliberate opt-out | ❌ WPF-only attribute | +| **XAML104** `DuplicateMergedDictionaryInclude` | same dictionary merged more than once | wasted load + order ambiguity | ~ (Avalonia has merged dicts, diff syntax) | +| **XAML105** `MergedDictionaryKeyShadowing` | key defined in multiple merged dictionaries → effective value depends on include order | "last merged wins, primary beats merged" — silent order dependence | ~ | +| **XAML106** `FreezableResourceShouldFreeze` | `Freezable` resource, no bindings/dynamic-resource/animation, missing `PresentationOptions:Freeze="True"` | freezing drops change-notification overhead + working set | ❌ **Freezable is WPF-only** | +| **XAML107** `VirtualizationExplicitlyDisabled` | `IsVirtualizing="False"`, `CanContentScroll="False"` on lists, non-virtualizing `ItemsPanel`, direct/mixed containers | virtualization critical for large item controls; these accidentally kill it | ✅ `VirtualizingStackPanel`/`ItemsRepeater` | +| **XAML108** `PerKeystrokeBindingWithoutDelay` | `TwoWay` + `UpdateSourceTrigger=PropertyChanged` on an editable property with no `Delay` | `Text` defaults to `LostFocus` for a reason; `Delay` exists to avoid per-keystroke flooding | ✅ | +| **XAML109** `TemplateComplexityHigh` | template-complexity score over threshold (node count, nested panels, Grid/StackPanel depth, trigger count, ItemsControl depth) | template expansion = extra visual-tree objects; layout is a 2-pass cost | ✅ | +| **XAML110** `ImageDecodedAtFullSize` | image shown small (explicit Width/Height ≤ thumbnail) but `Source` is a plain URI string, so no decode-to-size is possible | decode-to-size beats decode-full-then-scale; the hint needs a `BitmapImage`, not a string `Source` | ❌ WPF decode hints differ | +| **XAML111** `LayoutTransformSuspicious` | a `LayoutTransform` (attribute or property element) where a `RenderTransform` would do | `LayoutTransform` re-runs measure/arrange on change; `RenderTransform` is a render-time matrix. Candidate — legit when layout must reflow | ❌ Avalonia uses `LayoutTransformControl` | +| **XAML112** `TemplateBindingOpportunity` | inside a `ControlTemplate`, a `{Binding RelativeSource=TemplatedParent}` with no converter / not two-way | `{TemplateBinding}` is the cheaper compiled form; the converter/two-way exclusions are exactly TemplateBinding's limits | ✅ | +| **XAML113** `InlineFreezableDuplication` | the same inline Freezable (brush/geometry/transform set as a property value, not keyed) declared identically more than once | each inline copy is a separate object; one shared keyed resource collapses them (the inline case of XAML100) | ✅ | + +Exception lists matter (this is where naive greps die): **XAML106** must skip Freezables that are +animated, data-bound, or reference a `DynamicResource` (can't freeze); **XAML103** must allow the +`FrameworkElement`/`FrameworkContentElement` insertion case. Start **XAML101** with exact +type+key match; structural equivalence is a later refinement. (All three exception rules are +implemented and selftested in `xaml_check.py`.) + +## Phase 2 — Roslyn-linked hybrid (where the graph pays rent) + +These are genuinely **not offered by existing WPF analyzers** because they require linking XAML +usage to code symbols — which we already have machinery for. XAML says *which* converter/handler; +the graph says *what it does*. + +| Rule | What it flags | +|---|---| +| **XAML200** `ConverterAllocatesOnHotPath` | `Convert`/`ConvertBack` allocates collections / materializes LINQ / touches FS / reflects / uses Dispatcher | +| **XAML201** `ConverterCallsExpensiveServices` | converter body reaches localization/IO/deep call chains | +| **XAML202** `MarkupExtensionProvideValueExpensive` | custom `ProvideValue` allocates heavily / re-resolves services / does uncached runtime work | +| **XAML203** `XamlEventHandlerCreatesLongLivedSubscription` | `Loaded=`/`Click=`/`EventSetter.Handler` resolves to code that subscribes a longer-lived service with no matching unsubscribe | +| **XAML204** `ItemsSourceBackedByListRebuildPattern` | `ItemsControl` bound to a getter returning `List`/`IEnumerable` (full regen / wrapper overhead) vs `ObservableCollection` | +| **XAML205** `GetterBoundFromXamlAllocatesOrMaterializes` | XAML-bound getter allocates / materializes on each call | + +**XAML203 reuses the existing acquire/release + region-escape engine** (the same one behind own-check +OWN001 `+=`-without-`-=`): a XAML-originated leak becomes a lifetime fact on the same rails, not a new +detector. + +### Phase 2 mechanics — the binding-path join (and where the link-extractor lives) + +The markup pass already separates two kinds of fact, and Phase 2 makes the seam explicit: + +- **`XamlPerfRules`** — resource scope, dictionaries, virtualization, layout, images, Freezables. + These are *self-contained in markup* and are exactly the Phase-1 rules already shipped; they need + no C# at all. +- **`XamlLinkFacts`** — `x:Class`, `DataContext` type, binding paths, event handlers, converter + types, `ItemsSource`. These are **pointers into C#**: on their own they are inert; their value is + the *join* to a symbol. + +The join is the whole point — it is where the interprocedural core earns its keep and where this +stops being "found a `DynamicResource`, nodded gravely": + +``` + binding path in XAML ─┐ + (Text="{Binding Qty}") │ + x:Class + DataContext ─┼─► Roslyn resolves Qty -> the property symbol + │ └─► own-check's interprocedural engine walks: + │ getter (alloc? materialize?), setter, + │ the PropertyChanged cascade it raises, + │ the converter on the binding, + │ the ItemsControl/template it invalidates + └─► report: "this TextBox updates the source on every + keystroke, runs this setter, raises these N + properties, hits this converter, invalidates + this ItemsControl" +``` + +**Where the link-extractor lives — the decision.** It does **not** get a new parallel C# checker in +OwnAudit (`src/OwnAudit.Xaml/`). That would re-create the "two analyzers in two repos" problem this +note opens by ruling out, and it contradicts the canonical-in-Own.NET rule (`README`/`Plan.md`: +"*Don't reimplement it here*"). The XAML link-facts extractor is an **extension of `own-check`** — the +existing error-tolerant `SemanticModel` extractor that already lives in Own.NET and already does the +acquire/release + region-escape walk. own-check learns to read the `.xaml` next to the `.cs` it is +already parsing (resolve `x:Class` → the code-behind type → the `DataContext`/binding symbols), and +emits the binding-join findings as more `OWNxxx`/`XAML2xx` facts on the **same rails**. One extractor, +one semantic model, one lifetime engine — no second toolchain to keep version-matched. + +`OwnAudit.Xaml` as a standalone C# project is the **post-lift-out product form** (Plan.md §7), not the +way to build Phase 2: when `audit/` lifts out, the markup pass + the own-check XAML extension become +that package. Building it standalone *before* lift-out just means maintaining the parallel surface the +markup phase deliberately avoided. + +**Static is a candidate, runtime confirms (ties to Phase 3).** The join produces a *suspicion* — +"this binding *can* flood the setter per keystroke". Whether it actually fires tens of thousands of +times in a real screen is a runtime fact: the converter-call / `PropertyChanged` counters of Phase 3 +promote XAML108+the binding-join candidate from "structurally hot" to "measured hot" through the same +`correlate.py` suspect/confirm split. That is the difference between "you have an un-delayed +`PropertyChanged` binding" and "*this* is why the form freezes when you type one digit". + +The link-fact record stays the canonical shape (so it rides the existing pipeline): a +`{tool: "own-check", rule: "XAML2xx", resource: "", path, line, message}` where `path` +/`line` point at the **XAML** site (where a developer fixes it) and the message names the resolved C# +symbol chain — markup and code stitched into one finding, not two disconnected alerts. + +## Phase 3 — runtime correlation (reuse `correlate.py`, don't add static cleverness) + +Externally validated by the research: *don't sell static as a guarantee — emit candidates, confirm at +runtime.* That is exactly our existing `findings.json` (suspicion) → `runtime.json` → `correlate.py` +(confirmation) split. The XAML candidates that need runtime proof: + +- **binding hot-path reality** — a converter-call counter / binding-error collector says *which* of the + XAML200/204 candidates actually fire tens of thousands of times in a scenario. +- **visual-tree inflation / layout storms** — XAML109's static node count, upgraded by the real + instantiated-tree count (depends on item counts, triggers, virtualization, theme). +- **image/brush cost under animation** — XAML110 confirmed only when a screen animates/zooms. +- **lifetime proof for XAML-originated patterns** — XAML203 promoted from suspicion to a retention + path via the heap walker (phase-5 collector). + +This phase needs the **runtime-trace collector** — the *other* gap from OwnAudit's `wpf-audit-coverage.md` +(binding-error trace + Dispatcher/notification counters). XAML phase 3 and that collector are the +same build. + +--- + +## Avalonia oracle intersection + +Phase-1 markup rules are **mostly oracle-reachable** (`.axaml` is the same dialect): XAML100, 107, +108, 109, 110 run on a leaking Avalonia app today. The **WPF-only tail** validated only on STS: +XAML102/103 (`DynamicResource`/`x:Shared` semantics differ) and **XAML106 (Freezable — WPF-only +concept)**. This is the same today/never line already drawn in the coverage matrix, so the XAML +analyzer and the oracle are complementary: the oracle gives us live `.axaml` to exercise the +framework-agnostic markup rules; the WPF tail waits for STS. (The shipped runner enforces this: +XAML102/103/106 short-circuit when the root declares the Avalonia namespace.) + +## Roadmap summary + +1. **Phase 1** — a build-free XAML runner in **`Own.NET/audit/static`** (line-preserving parse, emits + the canonical finding record, runs in CI). **Done** (`audit/static/tools/xaml_check.py`), starting + with the rules that already had ⚠️ rows in the coverage matrix: **XAML107** (virtualization-off), + **XAML108** (per-keystroke binding), **XAML109** (template complexity), plus the reliably + markup-detectable resource rules XAML101/102/103/104/106. No .NET build, no stand. +2. **Phase 2** — link XAML facts to the Roslyn semantic model in **Own.NET's interprocedural core**; + the hybrid converter/handler/items-source rules. This is where that core earns its keep. +3. **Phase 3** — fold XAML candidates into the runtime correlation (`audit/runtime`; prototyped in + `OwnAudit/runtime/correlate.py`) alongside the runtime-trace collector; one merged finding model, + static suspicion upgraded by scenario evidence. + +The throughline: **XAML is a first-class fact source for the same resource + lifetime core in +Own.NET**, so each phase reuses machinery `audit/` already has (finding contract, +fingerprint/baseline/ratchet, the acquire/release engine, the runtime correlation) instead of growing +a parallel checker — in either repo.