fix(rubocop): report a missing icon directory instead of failing open

CHANGELOG.md

@@ -4,6 +4,16 @@
 
 ### Fixed
 
+- **`Glyphs/IconResolution` no longer fails open on a missing icon directory.**
+  A directory that did not exist produced the same empty list as one that
+  existed with no SVGs, and both were read as "nothing to check" — so a library
+  that was never synced, or a `DefaultVariant` naming a variant the project does
+  not have, silently disabled the cop for every call site of that library. Green
+  cop, green CI, no validation at all, indefinitely. `load_icons` now returns
+  `nil` for an absent directory and `[]` for an empty one; the absent case warns
+  once per library/variant, naming the path. The synced-but-empty case still
+  passes silently. Refs #8
+
 - **Cop options are declared, so RuboCop stops calling them unsupported.**
   `config/default.yml` documented `Libraries` (`Glyphs/IconResolution`) and
   `Mappings` (`Glyphs/LegacyIconHelper`) only in comments, and never mentioned
@@ -24,6 +34,11 @@
 
 ### Added
 
+- **`Glyphs/IconResolution` gained a `Strict` option** (default `false`). It
+  escalates the missing-icon-directory warning above to an offence, so projects
+  that depend on this cop can fail closed in CI instead of trusting a warning
+  not to scroll past.
+
 - **Dynamic icon calls are now resolved from source.** `SourceScanner` no longer
   silently skips `LucideIcon(some_var)` / `PhosphorIcon(tile[:icon])` — it harvests
   the literal name from two places so the pruner keeps it:

README.md

@@ -235,8 +235,23 @@ Glyphs/IconResolution:
   # Libraries merges over the built-in defaults (component => Dir/DefaultVariant):
   Libraries:
     PhosphorIcon: { Dir: phosphor, DefaultVariant: regular }
+  # Escalate a missing icon directory from a warning to an offence:
+  Strict: false
 ```
 
+If the directory a library resolves to does not exist — the library was never synced, or
+`DefaultVariant` names a variant you do not have — the cop cannot validate any of that
+library's call sites. Rather than pass silently, it warns once per library/variant:
+
+```
+[Glyphs/IconResolution] Icon directory `app/assets/svg/icons/phosphor/regular` not found,
+so `PhosphorIcon` names are not validated. Sync the library or fix `IconsPath`/`Libraries`.
+```
+
+Set `Strict: true` to make that an offence instead, so CI fails closed rather than reporting
+green while checking nothing. A directory that exists but ships no SVGs is not a
+misconfiguration and stays silent either way.
+
 ### Glyphs/PreferLibraryComponent
 
 ```ruby

config/default.yml

@@ -20,6 +20,9 @@ Glyphs/IconResolution:
   # Merges over the built-in component => directory/variant defaults, e.g.
   #   PhosphorIcon: { Dir: phosphor, DefaultVariant: light }
   Libraries: {}
+  # An icon directory that does not exist warns once per library/variant.
+  # Strict turns that warning into an offence so CI fails closed.
+  Strict: false
 
 Glyphs/PreferLibraryComponent:
   Description: 'Prefer library-specific components over generic Icon(..., library: ...).'

lib/rubocop/cop/glyphs/icon_resolution.rb

@@ -11,6 +11,11 @@ module Glyphs
       # calls (`_lucide(:house)`) are validated. A literal `variant:` keyword is
       # honored; calls with dynamic names or variants are skipped.
       #
+      # A directory that does not exist is a configuration error, not "nothing
+      # to check": it warns once per library/variant, or — with `Strict` — adds
+      # an offence, so the cop can never quietly validate nothing. A directory
+      # that exists but ships no SVGs stays silent.
+      #
       # @example
       #   # bad
       #   LucideIcon(:non_existent_icon)
@@ -51,10 +56,14 @@ class IconResolution < Base
 
         ICONIFY_PATTERN = /\biconify\s+(lucide|phosphor|heroicons)--([a-z0-9-]+)/
 
+        DEFAULT_ICONS_PATH = "app/assets/svg/icons"
+
         RAW_MSG = "Use `%{component}(:%{symbol}, class: \"%{rest}\")` instead of raw `iconify %{library}--…` class."
         RAW_DSTR_MSG = "Use `LucideIcon(name, class: ...)` etc. instead of building a raw " \
                        "`iconify <library>--…` class string."
         MISSING_MSG = "Icon `%{name}` not found in %{library}/%{variant}. %{suggestion}"
+        MISSING_DIR_MSG = "Icon directory `%{path}` not found, so `%{component}` names are not validated. " \
+                          "Sync the library or fix `IconsPath`/`Libraries`."
 
         def on_str(node)
           return if node.parent&.dstr_type?
@@ -97,14 +106,15 @@ def libraries
           @libraries ||= DEFAULT_LIBRARIES.merge(cop_config["Libraries"] || {})
         end
 
-        def check_call(node, _component, library)
+        def check_call(node, component, library)
           name = literal_name(node.first_argument)
           return unless name
 
           variant = variant_for(node, library)
           return if variant == :dynamic
 
           available = available_icons(library["Dir"], variant)
+          return report_missing_directory(node, component, library["Dir"], variant) if available.nil?
           return if available.empty?
           return if available.include?(name)
 
@@ -124,6 +134,21 @@ def check_call(node, _component, library)
           end
         end
 
+        # The directory the cop was told to read is missing, so every call for
+        # this library goes unchecked. Report it rather than pass silently.
+        def report_missing_directory(node, component, library_dir, variant)
+          message = format(MISSING_DIR_MSG, path: configured_directory(library_dir, variant), component:)
+          return add_offense(node.first_argument, message:) if cop_config["Strict"]
+
+          self.class.warn_once("[Glyphs/IconResolution] #{message}")
+        end
+
+        # Built from the unexpanded `IconsPath` so the message shows the path as
+        # the project wrote it, not an absolute machine-specific one.
+        def configured_directory(library_dir, variant)
+          self.class.directory_for(cop_config["IconsPath"] || DEFAULT_ICONS_PATH, library_dir, variant)
+        end
+
         def variant_for(node, library)
           pair = variant_pair(node)
           return library["DefaultVariant"] if pair.nil?
@@ -258,24 +283,47 @@ def damerau_levenshtein(left, right)
         end
 
         def icons_base_path
-          @icons_base_path ||= File.expand_path(cop_config["IconsPath"] || "app/assets/svg/icons", Dir.pwd)
+          @icons_base_path ||= File.expand_path(cop_config["IconsPath"] || DEFAULT_ICONS_PATH, Dir.pwd)
         end
 
         def available_icons(library_dir, variant)
           self.class.available_icons_for(icons_base_path, library_dir, variant)
         end
 
         class << self
+          # nil when the directory is absent, [] when it exists but ships no
+          # SVGs — the caller has to tell those apart. Memoized with `key?` (not
+          # `||=`) so a nil result is not re-probed on every call site.
           def available_icons_for(base_path, library_dir, variant)
             @available_icons_cache ||= {}
-            @available_icons_cache[[base_path, library_dir, variant]] ||= load_icons(base_path, library_dir, variant)
+            key = [base_path, library_dir, variant]
+            return @available_icons_cache[key] if @available_icons_cache.key?(key)
+
+            @available_icons_cache[key] = load_icons(directory_for(base_path, library_dir, variant))
+          end
+
+          def directory_for(base_path, library_dir, variant)
+            File.join(*[base_path, library_dir, variant].compact.reject { |part| part.to_s.empty? })
+          end
+
+          # RuboCop builds a fresh cop instance per file, so deduplicating the
+          # missing-directory warning has to outlive the instance.
+          def warn_once(message)
+            @warned_messages ||= {}
+            return if @warned_messages.key?(message)
+
+            @warned_messages[message] = true
+            warn message
+          end
+
+          def reset_warnings!
+            @warned_messages = nil
           end
 
           private
 
-          def load_icons(base_path, library_dir, variant)
-            path = File.join(*[base_path, library_dir, variant].compact.reject { |part| part.to_s.empty? })
-            return [] unless Dir.exist?(path)
+          def load_icons(path)
+            return nil unless Dir.exist?(path)
 
             Dir.children(path).filter_map { |file| File.basename(file, ".svg") if file.end_with?(".svg") }.sort
           end

spec/rubocop/cop/glyphs/icon_resolution_spec.rb

@@ -4,6 +4,10 @@
   let(:config) { RuboCop::Config.new("Glyphs/IconResolution" => cop_config) }
   let(:cop_config) { { "IconsPath" => "spec/fixtures/svg/icons" } }
 
+  # The missing-directory warning is deduplicated for the whole process (RuboCop
+  # builds a fresh cop instance per file), so examples must not inherit it.
+  before { described_class.reset_warnings! }
+
   context "with component calls" do
     it "accepts icons that exist" do
       expect_no_offenses(<<~RUBY)
@@ -127,11 +131,48 @@
     end
   end
 
-  context "when the icons path does not exist" do
+  context "when the icons directory does not exist" do
     let(:cop_config) { { "IconsPath" => "spec/fixtures/nope" } }
 
-    it "reports nothing" do
-      expect_no_offenses("LucideIcon(:anything_at_all)")
+    it "warns instead of silently validating nothing" do
+      expect { expect_no_offenses("LucideIcon(:anything_at_all)") }
+        .to output(%r{\[Glyphs/IconResolution\].*spec/fixtures/nope/lucide/outline}).to_stderr
+    end
+
+    it "warns once per library and variant, not once per call site" do
+      expect { expect_no_offenses("LucideIcon(:one)\nLucideIcon(:two)") }.to output.to_stderr
+      expect { expect_no_offenses("LucideIcon(:three)") }.not_to output.to_stderr
+      expect { expect_no_offenses("HeroIcon(:four)") }.to output(%r{nope/heroicons/outline}).to_stderr
+    end
+  end
+
+  context "when the icons directory does not exist and Strict is enabled" do
+    let(:cop_config) { { "IconsPath" => "spec/fixtures/nope", "Strict" => true } }
+
+    it "reports an offence at the call site instead of warning" do
+      expect do
+        expect_offense(<<~RUBY)
+          LucideIcon(:anything_at_all)
+                     ^^^^^^^^^^^^^^^^ Icon directory `spec/fixtures/nope/lucide/outline` not found, so `LucideIcon` names are not validated. Sync the library or fix `IconsPath`/`Libraries`.
+        RUBY
+      end.not_to output.to_stderr
+
+      expect_no_corrections
+    end
+  end
+
+  # Load-bearing: a library that is synced but genuinely ships no SVGs is not a
+  # misconfiguration, and must not produce an offence on every call site.
+  context "when the icons directory exists but holds no SVGs" do
+    let(:cop_config) do
+      {
+        "IconsPath" => "spec/fixtures/svg/icons",
+        "Libraries" => { "EmptyIcon" => { "Dir" => "emptylib", "DefaultVariant" => "regular" } }
+      }
+    end
+
+    it "stays silent" do
+      expect { expect_no_offenses("EmptyIcon(:whatever_at_all)") }.not_to output.to_stderr
     end
   end