From 11fc14f196a563054332e386a308b8d76435ee71 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Fri, 17 Jul 2026 13:28:30 +0200 Subject: [PATCH] fix(scanner): harvest ICON* constants assigned with .freeze MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prism models `ICONS = { … }.freeze` as a CallNode, so declaration harvest skipped hash/array values and cross-file dynamics (`PhosphorIcon(@icon)` + frozen ICONS map) lost names at prune. Unwrap zero-arg `.freeze` (and parentheses) before walking the declaration value. Regression fixtures cover frozen hash + array maps rendered from another file. --- CHANGELOG.md | 12 ++++++- README.md | 6 ++-- lib/glyphs/source_scanner.rb | 35 +++++++++++++++++++ .../source/app/components/frozen_icons_map.rb | 16 +++++++++ .../source/app/components/selectable_row.rb | 16 +++++++++ spec/glyphs/source_scanner_spec.rb | 13 +++++++ 6 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/source/app/components/frozen_icons_map.rb create mode 100644 spec/fixtures/source/app/components/selectable_row.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5fa32..6f7b4e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +### Fixed + +- **Declaration harvest unwraps trailing `.freeze`.** `ICONS = { "x" => :car }.freeze` + (and `%i[a b].freeze`) used to yield a Prism `CallNode`, so hash/array values + were never collected. Cross-file dynamics (`PhosphorIcon(@icon)` in a shared + component + names only in a frozen `ICONS` map) then lost those icons at + `glyphs:prune_icons`. `collect_declaration` now unwraps zero-arg `.freeze` + (and parentheses) before walking the value. + ### Added - **Dynamic icon calls are now resolved from source.** `SourceScanner` no longer @@ -13,7 +22,8 @@ - _declaration-based_: literals in icon-declaration positions anywhere (a hash pair keyed `/icon/i`, or a constant named `/ICON/`) are kept for every dynamically-rendered library, closing the cross-file gap (e.g. a notifier - `ICON = :bell` constant rendered from a view). + `ICON = :bell` constant rendered from a view). Frozen maps + (`ICONS = { … }.freeze`) are included — see Fixed above. This makes `keep_icons` a last-resort escape hatch (DB/ENV/gem-chrome names) rather than the primary mechanism. Only literals are harvested, so the scanner diff --git a/README.md b/README.md index 64c6feb..2c6f3f0 100644 --- a/README.md +++ b/README.md @@ -133,9 +133,9 @@ The scanner resolves them from source, so you rarely need `keep_icons` at all: ternaries (`@open ? "caret-up" : "caret-down"`), `case/when`, and locals. - **Declaration-based** — literals in icon-declaration positions *anywhere* — a hash pair keyed like an icon (`icon: :gear`, `menu_icon: "house"`) or a - constant named like one (`ICON = :bell`, `STATUS_ICONS = { .. => :warning }`) — - are kept for every dynamically-rendered library, so a name declared in one - file and rendered from another survives. + constant named like one (`ICON = :bell`, `STATUS_ICONS = { .. => :warning }`, + including trailing `.freeze`) — are kept for every dynamically-rendered + library, so a name declared in one file and rendered from another survives. Only literals are harvested, so the scanner never invents a reference; the worst case is keeping a coincidentally icon-named string, which the post-prune diff --git a/lib/glyphs/source_scanner.rb b/lib/glyphs/source_scanner.rb index f5bd36e..d83858c 100644 --- a/lib/glyphs/source_scanner.rb +++ b/lib/glyphs/source_scanner.rb @@ -346,7 +346,11 @@ def collect_literal(set, node) # Harvests icon names from a declaration value: a bare literal, an array of # literals (`ICON = %i[a b]`), or a hash's values (`{ "sms" => :device }`). + # Trailing `.freeze` is unwrapped so `ICONS = { … }.freeze` still harvests — + # without this, cross-file dynamics (`PhosphorIcon(@icon)` + names only in a + # frozen `ICONS` hash) lose those names at prune. def collect_declaration(value_node) + value_node = unwrap_declaration_wrappers(value_node) case value_node when Prism::SymbolNode, Prism::StringNode collect_literal(@declaration_literals, value_node) @@ -356,6 +360,37 @@ def collect_declaration(value_node) value_node.elements.grep(Prism::AssocNode).each { |assoc| collect_declaration(assoc.value) } end end + + # Peel zero-arg `.freeze` and parentheses so the underlying Hash/Array/literal + # is what declaration harvest walks. + def unwrap_declaration_wrappers(value_node) + loop do + case value_node + when Prism::CallNode + break unless freeze_call?(value_node) + + value_node = value_node.receiver + when Prism::ParenthesesNode + value_node = value_node.body + when Prism::StatementsNode + # `(expr)` → StatementsNode with one child; multi-statement bodies are + # not valid constant values, so take the sole statement when present. + break unless value_node.body.size == 1 + + value_node = value_node.body.first + else + break + end + end + value_node + end + + # `receiver.freeze` with no args — the common immutable-constant pattern. + def freeze_call?(node) + node.name == :freeze && + node.receiver && + (node.arguments.nil? || node.arguments.arguments.empty?) + end end end end diff --git a/spec/fixtures/source/app/components/frozen_icons_map.rb b/spec/fixtures/source/app/components/frozen_icons_map.rb new file mode 100644 index 0000000..8b0ce3e --- /dev/null +++ b/spec/fixtures/source/app/components/frozen_icons_map.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Cross-file declaration harvest: icon names live only here, rendered +# dynamically from `selectable_row.rb` via `PhosphorIcon(@icon)`. +# The trailing `.freeze` used to hide HashNode values from declaration +# harvest (Prism sees a CallNode). Not loaded at runtime — only parsed +# by SourceScanner specs. +class FrozenIconsMap + ICONS = { + "driving_license" => :car, + "residence_permit" => :identification_badge, + }.freeze + + # Array form with freeze should harvest too. + STATUS_ICONS = %i[warning check_circle].freeze +end diff --git a/spec/fixtures/source/app/components/selectable_row.rb b/spec/fixtures/source/app/components/selectable_row.rb new file mode 100644 index 0000000..e53a87b --- /dev/null +++ b/spec/fixtures/source/app/components/selectable_row.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Shared row that renders an icon passed in from another file. Forces a +# dynamic phosphor keep-set so FrozenIconsMap declarations are exercised +# as global declaration harvests. Not loaded at runtime — only parsed. +class SelectableRow < Phlex::HTML + include Glyphs + + def initialize(icon:) + @icon = icon + end + + def view_template + PhosphorIcon(@icon) + end +end diff --git a/spec/glyphs/source_scanner_spec.rb b/spec/glyphs/source_scanner_spec.rb index 54999b8..59e08d2 100644 --- a/spec/glyphs/source_scanner_spec.rb +++ b/spec/glyphs/source_scanner_spec.rb @@ -81,6 +81,19 @@ def ref(library, variant, name) expect(keeps[:phosphor]).to include("bell-ringing") end + # `ICONS = { "driving_license" => :car, … }.freeze` in frozen_icons_map.rb, + # rendered via PhosphorIcon(@icon) in selectable_row.rb. Without unwrapping + # the trailing `.freeze` CallNode, declaration harvest sees nothing and + # cross-file dynamics prune those SVGs (production Icons::IconNotFound). + it "keeps ICON* hash and array values even when the constant is .freeze'd" do + expect(keeps[:phosphor]).to include( + "car", + "identification-badge", + "warning", + "check-circle" + ) + end + # A library with no dynamic call gets no dynamic keeps — heroicons here is # only ever called with literal names, so declaration literals don't leak # into it.