From 80513b92dda31d9aeb0e27ad38dfc38f64829c38 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Sat, 25 Jul 2026 08:46:23 +0200 Subject: [PATCH] fix(rubocop): declare the cop options config/default.yml only documented MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary RuboCop builds each cop's supported-parameter list from the keys actually present in `config/default.yml` (`ConfigValidator#each_invalid_parameter` checks `default_config.key?(param)`). Four working options were invisible to that check, so every run warned they were unsupported: - `Glyphs/IconResolution` `Libraries` — comment only - `Glyphs/LegacyIconHelper` `Mappings` — comment only - `Glyphs/LegacyIconHelper` `LibraryComponents` — absent entirely - `Glyphs/PreferLibraryComponent` `LibraryComponents` — absent entirely The last two are the sharpest case: `LegacyIconHelper`'s own `MSG_UNKNOWN` tells users to "add a `LibraryComponents` mapping", and doing so warned. Each is now declared with an empty default. `DEFAULT_LIBRARIES.merge({})` and `LIBRARY_TO_COMPONENT.merge({})` are no-ops, and `LegacyIconHelper#mappings` already falls back to `DEFAULT_MAPPINGS` when the configured hash is empty, so behaviour is unchanged for everyone. ## Test Coverage - plugin_spec: derives every `cop_config["Key"]` read by each registered Glyphs cop (and its `RuboCop::Cop::Glyphs::*` mixins) from source and asserts `config/default.yml` declares it — a new undeclared option now fails the suite - icon_resolution_spec: `Libraries: {}` still resolves via the built-in defaults - legacy_icon_helper_spec: `Mappings: {}` / `LibraryComponents: {}` still apply the built-in helper map and library components - prefer_library_component_spec: `LibraryComponents: {}` still applies defaults ## Verification - [x] bundle exec rubocop lib spec passes - [x] bundle exec rspec passes (137 examples) - [x] Real `rubocop` run against a project config setting all four options: 4 warnings before, 0 after, byte-identical offences Refs #7 --- CHANGELOG.md | 11 +++++++ README.md | 13 ++++++++ config/default.yml | 14 ++++++--- .../cop/glyphs/icon_resolution_spec.rb | 15 ++++++++++ .../cop/glyphs/legacy_icon_helper_spec.rb | 18 +++++++++++ .../glyphs/prefer_library_component_spec.rb | 15 ++++++++++ spec/rubocop/plugin_spec.rb | 30 +++++++++++++++++++ 7 files changed, 112 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f7b4e9..1d41152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ ### Fixed +- **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 + `LibraryComponents` (`Glyphs/LegacyIconHelper`, `Glyphs/PreferLibraryComponent`) + at all. RuboCop derives its supported-parameter list from the keys actually + present in that file, so it warned `does not support parameter` on + every run — for four working options, one of which + `Glyphs/LegacyIconHelper` itself tells you to add. Each is now declared with an + empty default (a no-op merge), so the options are validated instead of + reported, with behaviour unchanged. Refs #7 + - **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 diff --git a/README.md b/README.md index 2c6f3f0..07dd2a1 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,10 @@ Glyphs/LegacyIconHelper: # Mappings replaces the built-in defaults (_lucide/_phosphor/_hero/_heroicon/_tabler): # Mappings: # _custom: CustomIcon + # LibraryComponents merges over the built-in library => component map, so + # `icon("x", library: "customlib")` corrects to `CustomIcon("x")`: + # LibraryComponents: + # customlib: CustomIcon ``` ### Glyphs/IconResolution @@ -239,6 +243,15 @@ Glyphs/IconResolution: Icon(:house, library: :lucide) # => LucideIcon(:house) ``` +```yaml +Glyphs/PreferLibraryComponent: + Include: + - app/**/*.rb + # LibraryComponents merges over the built-in library => component map: + # LibraryComponents: + # customlib: CustomIcon +``` + ## Migrating an app off icon helpers 1. Add `gem "glyphs"`, `include Glyphs` where the helpers used to be included. diff --git a/config/default.yml b/config/default.yml index efbc98c..ef73c4c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3,10 +3,13 @@ Glyphs/LegacyIconHelper: Enabled: true VersionAdded: '0.1.0' SafeAutoCorrect: true - # Mappings replaces the built-in defaults when set: - # Mappings: + # Replaces the built-in helper => component defaults when set, e.g. # _lucide: LucideIcon # _heroicon: HeroIcon + Mappings: {} + # Merges over the built-in library => component map, e.g. + # customlib: CustomIcon + LibraryComponents: {} DefaultLibraryComponent: HeroIcon Glyphs/IconResolution: @@ -14,12 +17,15 @@ Glyphs/IconResolution: Enabled: true VersionAdded: '0.1.0' IconsPath: app/assets/svg/icons - # Libraries merges over the built-in defaults: - # Libraries: + # Merges over the built-in component => directory/variant defaults, e.g. # PhosphorIcon: { Dir: phosphor, DefaultVariant: light } + Libraries: {} Glyphs/PreferLibraryComponent: Description: 'Prefer library-specific components over generic Icon(..., library: ...).' Enabled: true VersionAdded: '0.1.0' SafeAutoCorrect: true + # Merges over the built-in library => component map, e.g. + # customlib: CustomIcon + LibraryComponents: {} diff --git a/spec/rubocop/cop/glyphs/icon_resolution_spec.rb b/spec/rubocop/cop/glyphs/icon_resolution_spec.rb index ffabb9d..5814a52 100644 --- a/spec/rubocop/cop/glyphs/icon_resolution_spec.rb +++ b/spec/rubocop/cop/glyphs/icon_resolution_spec.rb @@ -135,6 +135,21 @@ end end + context "with the declared empty Libraries default" do + let(:cop_config) { { "IconsPath" => "spec/fixtures/svg/icons", "Libraries" => {} } } + + it "still resolves through the built-in library defaults" do + expect_offense(<<~RUBY) + PhosphorIcon(:locks) + ^^^^^^ Icon `locks` not found in phosphor/regular. Did you mean `:lock`? + RUBY + + expect_correction(<<~RUBY) + PhosphorIcon(:lock) + RUBY + end + end + context "with a Libraries override" do let(:cop_config) do { diff --git a/spec/rubocop/cop/glyphs/legacy_icon_helper_spec.rb b/spec/rubocop/cop/glyphs/legacy_icon_helper_spec.rb index 60cec28..c132666 100644 --- a/spec/rubocop/cop/glyphs/legacy_icon_helper_spec.rb +++ b/spec/rubocop/cop/glyphs/legacy_icon_helper_spec.rb @@ -110,6 +110,24 @@ RUBY end + context "with the declared empty Mappings and LibraryComponents defaults" do + let(:cop_config) { { "Mappings" => {}, "LibraryComponents" => {} } } + + it "still applies the built-in helper mappings and library components" do + expect_offense(<<~RUBY) + _lucide(:house) + ^^^^^^^ Use `LucideIcon(...)` instead of `_lucide(...)`. + icon("check", library: "heroicons") + ^^^^ Use `HeroIcon(...)` instead of `icon(...)`. + RUBY + + expect_correction(<<~RUBY) + LucideIcon(:house) + HeroIcon("check") + RUBY + end + end + context "with custom Mappings" do let(:cop_config) { { "Mappings" => { "_custom" => "CustomIcon" } } } diff --git a/spec/rubocop/cop/glyphs/prefer_library_component_spec.rb b/spec/rubocop/cop/glyphs/prefer_library_component_spec.rb index fc97349..d156815 100644 --- a/spec/rubocop/cop/glyphs/prefer_library_component_spec.rb +++ b/spec/rubocop/cop/glyphs/prefer_library_component_spec.rb @@ -41,4 +41,19 @@ Some::Icon(:house, library: :lucide) RUBY end + + context "with the declared empty LibraryComponents default" do + let(:cop_config) { { "LibraryComponents" => {} } } + + it "still applies the built-in library components" do + expect_offense(<<~RUBY) + Icon(:house, library: :lucide) + ^^^^ Use `LucideIcon(...)` instead of `Icon(..., library: ...)`. + RUBY + + expect_correction(<<~RUBY) + LucideIcon(:house) + RUBY + end + end end diff --git a/spec/rubocop/plugin_spec.rb b/spec/rubocop/plugin_spec.rb index 5c1bbad..e7f11ca 100644 --- a/spec/rubocop/plugin_spec.rb +++ b/spec/rubocop/plugin_spec.rb @@ -26,4 +26,34 @@ ) expect(config.values).to all(include("Enabled" => true)) end + + # RuboCop builds its supported-parameter list from the keys present in + # `config/default.yml` (see `ConfigValidator#each_invalid_parameter`), so an + # option a cop reads but the file only documents in a comment is reported as + # unsupported on every run. + it "declares every cop_config parameter its cops read" do + default_config = YAML.safe_load_file(plugin.rules(nil).value) + + undeclared = glyphs_cops.filter_map do |cop| + missing = cop_config_keys_read_by(cop) - default_config.fetch(cop.cop_name).keys + "#{cop.cop_name} reads undeclared #{missing.join(', ')}" if missing.any? + end + + expect(undeclared).to be_empty + end + + def glyphs_cops + RuboCop::Cop::Registry.global.cops.select { |cop| cop.badge.department == :Glyphs } + end + + # A cop reads its options both directly and through the shared + # `RuboCop::Cop::Glyphs::*` mixins, so both sources have to be scanned. + def cop_config_keys_read_by(cop_class) + mixins = cop_class.included_modules.select { |mod| mod.name.to_s.start_with?("RuboCop::Cop::Glyphs::") } + + [cop_class, *mixins] + .filter_map { |mod| Object.const_source_location(mod.name)&.first } + .flat_map { |path| File.read(path).scan(/cop_config\["([^"]+)"\]/).flatten } + .uniq + end end