From b351ea40664dae5a8410943a0af54ac9f8d3f75d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 21:00:54 +0000 Subject: [PATCH 1/3] audit(xaml): add XAML105 MergedDictionaryKeyShadowing (in-file) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the merged-dictionary story (XAML104 = duplicate include) with the key shadowing case: the same x:Key defined in >=2 inline merged dictionaries, or in the primary dictionary AND an inline merged one. The effective value then depends on merge order (WPF: last merged wins, a primary key beats merged ones) — a silent order-dependence that breaks when includes are reordered. In-file only: dictionaries pulled in by Source="..." reference external files this single-file pass can't resolve, so their keys aren't compared (cross-file variant documented as a later slice). FP-safe — only literal same-key collisions flag; a lone key (the normal case) never does. - xaml_check.py: _rule_merged_dict_shadowing, registered in RULES; docstring updated. - taxonomy: XAML105 -> category 9 (merged-dictionary-shadowing). - selftest grows to 47: two inline merged dicts colliding flags; primary+merged collision flags; distinct keys don't; external Source= dicts are not compared. - README + design-note catalogue updated. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb --- audit/README.md | 8 +-- audit/static/taxonomy/categories.yml | 1 + audit/static/tools/xaml_check.py | 82 ++++++++++++++++++++++++++-- docs/notes/xaml-analyzer-design.md | 2 +- 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/audit/README.md b/audit/README.md index 8adbb5a8..5d295498 100644 --- a/audit/README.md +++ b/audit/README.md @@ -131,10 +131,10 @@ python audit/static/run_static.py --selftest # full pipeline end-to-end on fix - **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 XAML100/101/102/103/104/ - 106/107/108/109/110/111/112/113 (resource hoisting, 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 + 105/106/107/108/109/110/111/112/113 (resource hoisting, merged-dictionary key shadowing, + 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). - **XAML Phase-2 seam — done:** `static/tools/xaml_facts.py` emits `xaml-facts.json` (resource graph diff --git a/audit/static/taxonomy/categories.yml b/audit/static/taxonomy/categories.yml index 20a2452f..158c2a1a 100644 --- a/audit/static/taxonomy/categories.yml +++ b/audit/static/taxonomy/categories.yml @@ -65,6 +65,7 @@ rules: "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 + "XAML105": {category: 9, name: merged-dictionary-shadowing} # order-dependent key collision "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 diff --git a/audit/static/tools/xaml_check.py b/audit/static/tools/xaml_check.py index a76c6938..bf324167 100644 --- a/audit/static/tools/xaml_check.py +++ b/audit/static/tools/xaml_check.py @@ -32,6 +32,7 @@ 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 + XAML105 MergedDictionaryKeyShadowing (cat 9) in-file key collision, order-dependent XAML106 FreezableResourceShouldFreeze (cat 9) WPF-only, change-notify overhead XAML107 VirtualizationExplicitlyDisabled (cat 8) virtualization accidentally killed XAML108 PerKeystrokeBindingWithoutDelay (cat 6) per-keystroke source flooding @@ -43,10 +44,11 @@ XAML100 covers the recurring heavy-resource case across control-local scopes — both Freezables and Styles/templates (the latter on a full-subtree signature). XAML105 -MergedDictionaryKeyShadowing across *external* dictionaries (cross-file resolution) -remains a later slice — documented so nothing on the wishlist quietly falls through. -Phase 2 (Roslyn-linked XAML2xx) and Phase 3 (runtime correlation) live elsewhere per -the design note. +covers the *in-file* merged-dictionary key collision (inline dictionaries + the +primary); the same shadowing across **external** ``Source=`` dictionaries needs +cross-file resolution and remains a later slice — documented so nothing on the +wishlist quietly falls through. 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 @@ -401,6 +403,44 @@ def _keyed_resources(rd: Node): yield key, c +def _rule_merged_dict_shadowing(root: Node, avalonia: bool) -> list[XamlFinding]: + """XAML105 — the same ``x:Key`` defined in more than one **inline** merged + dictionary (or in the primary dictionary AND an inline merged one): the effective + value then depends on merge order (WPF: last merged wins, and a primary key beats + merged ones), a silent order-dependence that breaks when includes are reordered. + + In-file only: dictionaries pulled in by ``Source="..."`` reference external files + this single-file pass can't resolve, so their keys aren't compared (the cross-file + variant is a documented later slice). FP-safe — only literal same-key collisions + flag, and a lone key (the normal case) never does.""" + out: list[XamlFinding] = [] + for md in root.walk(): + if md.local() != "MergedDictionaries" or not md.is_property_element(): + continue + host = md.parent # the owning ResourceDictionary / + # key -> [(where, line)] across the primary dict and each inline merged dict + keymap: dict[str, list[tuple[str, int]]] = {} + if host is not None: + for key, c in _keyed_resources(host): + keymap.setdefault(key, []).append(("primary", c.line)) + inline_dicts = [c for c in md.children + if c.type_name() == "ResourceDictionary" and c.attr("Source") is None] + for i, mdict in enumerate(inline_dicts, start=1): + for key, c in _keyed_resources(mdict): + keymap.setdefault(key, []).append((f"merged #{i}", c.line)) + + for key, occ in keymap.items(): + if len(occ) < 2: + continue # the normal case: a key defined once + where = ", ".join(f"{w} (line {ln})" for w, ln in occ) + out.append(XamlFinding( + "XAML105", occ[0][1], + f"resource key '{key}' is defined in {len(occ)} merged/primary scopes " + f"[{where}]; the effective value depends on merge order (last merged " + "wins, primary beats merged) [resource: merged dictionary]")) + return out + + def _scope_owner(rd: Node) -> str: """The element type that owns a resource dictionary — the nearest ```` ancestor (``Grid``, ``Border``, ``DataTemplate`` …), or ``root`` for the document @@ -725,6 +765,7 @@ def _rule_layout_transform(root: Node, avalonia: bool) -> list[XamlFinding]: RULES: list[Callable[[Node, bool], list[XamlFinding]]] = [ _rule_resource_hoist, + _rule_merged_dict_shadowing, _rule_virtualization, _rule_template_complexity, _rule_per_keystroke_binding, @@ -941,6 +982,39 @@ def rules(text: str) -> dict[str, XamlFinding]: check(r.get("XAML104") and r["XAML104"].line == 5, "XAML104 must point at the duplicate include (line 5)") + # XAML105 — the same key in two inline merged dictionaries (order-dependent). + shadow = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + r = rules(shadow) + check("XAML105" in r, "XAML105 must flag a key defined in two inline merged dictionaries") + check(r.get("XAML105") and "accent" in r["XAML105"].message, "XAML105 must name the key") + # primary key + a merged-dictionary key collision is also shadowing. + primary = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + check("XAML105" in rules(primary), + "XAML105 must flag a primary key shadowed by a merged dictionary") + # distinct keys across merged dicts -> no collision, no finding. + nodup = shadow.replace('x:Key="accent" Color="Blue"', 'x:Key="other" Color="Blue"') + check("XAML105" not in rules(nodup), + "XAML105 false positive: distinct keys across merged dictionaries do not shadow") + # external Source dictionaries are not resolved -> not compared (deferred). + check("XAML105" not in rules(dup), + "XAML105 must not compare keys of external Source= dictionaries (cross-file deferred)") + # XAML101 — duplicate stateless converter across dictionaries. conv = (f'\n' ' \n' diff --git a/docs/notes/xaml-analyzer-design.md b/docs/notes/xaml-analyzer-design.md index 679c7446..a510a21b 100644 --- a/docs/notes/xaml-analyzer-design.md +++ b/docs/notes/xaml-analyzer-design.md @@ -115,7 +115,7 @@ offending element's stamped line; a rule that can only locate a file-level issue | **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 | ~ | +| **XAML105** `MergedDictionaryKeyShadowing` ✅ *(in-file)* | key defined in ≥2 **inline** merged dictionaries (or primary + merged) → effective value depends on include order; external `Source=` dictionaries deferred (cross-file) | "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 | ✅ | From 044dc0f882052eb9676a55b61184d7a025693d1e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 21:06:54 +0000 Subject: [PATCH 2/3] xaml: XAML105 requires 2+ distinct scopes (no same-scope dup-key FP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ResourceDictionary that declares the same x:Key twice directly (or twice inside one inline merged dictionary) is a duplicate-key error within a single scope — runtime-invalid, but a different problem from XAML105's order-dependent merge shadowing. The old check fired on any key with 2+ occurrences, so two primary entries sharing a key were mislabeled as [primary, primary] shadowing. Collisions now require the key to span 2+ DISTINCT scopes (primary vs merged, or two different merged dicts). Added a selftest: two same-key primary entries plus a non-colliding merged dict must not emit XAML105. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb --- audit/static/tools/xaml_check.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/audit/static/tools/xaml_check.py b/audit/static/tools/xaml_check.py index bf324167..ce1d7883 100644 --- a/audit/static/tools/xaml_check.py +++ b/audit/static/tools/xaml_check.py @@ -430,8 +430,11 @@ def _rule_merged_dict_shadowing(root: Node, avalonia: bool) -> list[XamlFinding] keymap.setdefault(key, []).append((f"merged #{i}", c.line)) for key, occ in keymap.items(): - if len(occ) < 2: - continue # the normal case: a key defined once + # Shadowing requires the key in 2+ DISTINCT scopes (primary vs merged, or + # two different merged dicts). Two entries in the SAME scope are a + # duplicate-key error (runtime-invalid) — a different problem, not XAML105. + if len({w for w, _ln in occ}) < 2: + continue where = ", ".join(f"{w} (line {ln})" for w, ln in occ) out.append(XamlFinding( "XAML105", occ[0][1], @@ -1014,6 +1017,19 @@ def rules(text: str) -> dict[str, XamlFinding]: # external Source dictionaries are not resolved -> not compared (deferred). check("XAML105" not in rules(dup), "XAML105 must not compare keys of external Source= dictionaries (cross-file deferred)") + # two primary entries with the SAME key are a duplicate-key error in ONE scope, not + # order-dependent merge shadowing -> XAML105 must not fire (needs 2+ distinct scopes). + samescope = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + check("XAML105" not in rules(samescope), + "XAML105 false positive: same-scope duplicate keys are not merge-order shadowing") # XAML101 — duplicate stateless converter across dictionaries. conv = (f'\n' From c02b1eb4f6b9e9c9da3489b5e364c175f72d0f9a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 21:12:56 +0000 Subject: [PATCH 3/3] xaml: collapse same-scope duplicates in the XAML105 message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The distinct-scope gate was correct, but the emitted finding still formatted raw occurrences: a key appearing twice in the primary dict AND once in a merged dict reported "3 merged/primary scopes" and listed "primary" twice. Collapse occurrences to the first line per scope before building the message, so the count and the "where" list reflect distinct scopes only. Added a selftest (dup primary + merged → "2 scopes", "primary" listed once). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb --- audit/static/tools/xaml_check.py | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/audit/static/tools/xaml_check.py b/audit/static/tools/xaml_check.py index ce1d7883..60acae90 100644 --- a/audit/static/tools/xaml_check.py +++ b/audit/static/tools/xaml_check.py @@ -433,12 +433,17 @@ def _rule_merged_dict_shadowing(root: Node, avalonia: bool) -> list[XamlFinding] # Shadowing requires the key in 2+ DISTINCT scopes (primary vs merged, or # two different merged dicts). Two entries in the SAME scope are a # duplicate-key error (runtime-invalid) — a different problem, not XAML105. - if len({w for w, _ln in occ}) < 2: + # Collapse to one line per scope (first occurrence) so the count and the + # "where" list reflect distinct scopes, not repeated same-scope entries. + by_scope: dict[str, int] = {} + for w, ln in occ: + by_scope.setdefault(w, ln) + if len(by_scope) < 2: continue - where = ", ".join(f"{w} (line {ln})" for w, ln in occ) + where = ", ".join(f"{w} (line {ln})" for w, ln in by_scope.items()) out.append(XamlFinding( - "XAML105", occ[0][1], - f"resource key '{key}' is defined in {len(occ)} merged/primary scopes " + "XAML105", next(iter(by_scope.values())), + f"resource key '{key}' is defined in {len(by_scope)} merged/primary scopes " f"[{where}]; the effective value depends on merge order (last merged " "wins, primary beats merged) [resource: merged dictionary]")) return out @@ -1030,6 +1035,24 @@ def rules(text: str) -> dict[str, XamlFinding]: '\n') check("XAML105" not in rules(samescope), "XAML105 false positive: same-scope duplicate keys are not merge-order shadowing") + # a key duplicated in primary AND present in a merged dict still shadows across the + # two distinct scopes, but the same-scope dup must collapse: report 2 scopes (not 3), + # with 'primary' listed once. + dupprimary = (f'\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n') + dp = rules(dupprimary) + check("XAML105" in dp, "XAML105 must flag a primary key also defined in a merged dict") + check(dp.get("XAML105") and "2 merged/primary scopes" in dp["XAML105"].message, + "XAML105 must collapse same-scope duplicates: 2 distinct scopes, not 3") + check(dp.get("XAML105") and dp["XAML105"].message.count("primary (line") == 1, + "XAML105 must list each distinct scope once (no repeated 'primary')") # XAML101 — duplicate stateless converter across dictionaries. conv = (f'\n'