Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <param> 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
Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 10 additions & 4 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,29 @@ 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:
Description: 'Validate that statically-known icon names exist in the synced SVG directories.'
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: {}
15 changes: 15 additions & 0 deletions spec/rubocop/cop/glyphs/icon_resolution_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cop/glyphs/legacy_icon_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" } } }

Expand Down
15 changes: 15 additions & 0 deletions spec/rubocop/cop/glyphs/prefer_library_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 30 additions & 0 deletions spec/rubocop/plugin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading