From 1464219dd54e4734d70ea3459b8119fa4b511c8b Mon Sep 17 00:00:00 2001 From: Bruno Vezoli Date: Wed, 24 Jul 2019 15:57:14 -0300 Subject: [PATCH] [Fix #78] Create EnumHash Cop --- CHANGELOG.md | 7 +++ config/default.yml | 8 ++++ lib/rubocop/cop/rails/enum_hash.rb | 36 ++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + manual/cops.md | 3 +- manual/cops_rails.md | 33 +++++++++++++ spec/rubocop/cop/rails/enum_hash_spec.rb | 60 ++++++++++++++++++++++++ 7 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/rails/enum_hash.rb create mode 100644 spec/rubocop/cop/rails/enum_hash_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 117e44d192..04048e5e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#78](https://github.com/rubocop-hq/rubocop-rails/issues/78): Add new `Rails/EnumHash` cop. ([@fedeagripa][], [@brunvez][], [@santib][]) + ### Bug fixes * [#53](https://github.com/rubocop-hq/rubocop-rails/issues/53): Fix a false positive for `Rails/SaveBang` when implicitly return using finder method and creation method connected by `||`. ([@koic][]) @@ -47,3 +51,6 @@ [@buehmann]: https://github.com/buehmann [@anthony-robin]: https://github.com/anthony-robin [@rmm5t]: https://github.com/rmm5t +[@fedeagripa]: https://github.com/fedeagripa +[@brunvez]: https://github.com/brunvez +[@santib]: https://github.com/santib diff --git a/config/default.yml b/config/default.yml index 929d6dbfac..e8ea60d302 100644 --- a/config/default.yml +++ b/config/default.yml @@ -145,6 +145,14 @@ Rails/DynamicFindBy: Whitelist: - find_by_sql +Rails/EnumHash: + Description: 'Prefer hash syntax over array syntax when defining enums.' + StyleGuide: '#enums' + Enabled: true + VersionAdded: '2.3' + Include: + - app/models/**/*.rb + Rails/EnumUniqueness: Description: 'Avoid duplicate integers in hash-syntax `enum` declaration.' Enabled: true diff --git a/lib/rubocop/cop/rails/enum_hash.rb b/lib/rubocop/cop/rails/enum_hash.rb new file mode 100644 index 0000000000..cff550baf6 --- /dev/null +++ b/lib/rubocop/cop/rails/enum_hash.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # This cop looks for enums written with array syntax. + # + # When using array syntax, adding an element in a + # position other than the last causes all previous + # definitions to shift. Explicitly specifying the + # value for each key prevents this from happening. + # + # @example + # # bad + # enum status: [:active, :archived] + # + # # good + # enum status: { active: 0, archived: 1 } + # + class EnumHash < Cop + MSG = 'Enum defined as an array found in `%s` enum declaration. '\ + 'Use hash syntax instead.' + + def_node_matcher :enum_with_array?, <<~PATTERN + (send nil? :enum (hash (pair (_ $_) array))) + PATTERN + + def on_send(node) + enum_with_array?(node) do |name| + add_offense(node, message: format(MSG, enum: name)) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index c992525e97..9b6b35a0cb 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -17,6 +17,7 @@ require_relative 'rails/delegate' require_relative 'rails/delegate_allow_blank' require_relative 'rails/dynamic_find_by' +require_relative 'rails/enum_hash' require_relative 'rails/enum_uniqueness' require_relative 'rails/environment_comparison' require_relative 'rails/exit' diff --git a/manual/cops.md b/manual/cops.md index d7e1aaaca6..158f8089b3 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -16,6 +16,7 @@ * [Rails/Delegate](cops_rails.md#railsdelegate) * [Rails/DelegateAllowBlank](cops_rails.md#railsdelegateallowblank) * [Rails/DynamicFindBy](cops_rails.md#railsdynamicfindby) +* [Rails/EnumHash](cops_rails.md#railsenumhash) * [Rails/EnumUniqueness](cops_rails.md#railsenumuniqueness) * [Rails/EnvironmentComparison](cops_rails.md#railsenvironmentcomparison) * [Rails/Exit](cops_rails.md#railsexit) @@ -54,4 +55,4 @@ * [Rails/UnknownEnv](cops_rails.md#railsunknownenv) * [Rails/Validation](cops_rails.md#railsvalidation) - \ No newline at end of file + diff --git a/manual/cops_rails.md b/manual/cops_rails.md index b5b697de92..2b12dae89e 100644 --- a/manual/cops_rails.md +++ b/manual/cops_rails.md @@ -642,6 +642,39 @@ Whitelist | `find_by_sql` | Array * [https://rails.rubystyle.guide#find_by](https://rails.rubystyle.guide#find_by) +## Rails/EnumHash + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | Yes | No | 2.3 | - + +This cop looks for enums written with array syntax. + +When using array syntax, adding an element in a +position other than the last causes all previous +definitions to shift. Explicitly specifying the +value for each key prevents this from happening. + +### Examples + +```ruby +# bad +enum status: [:active, :archived] + +# good +enum status: { active: 0, archived: 1 } +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +Include | `app/models/**/*.rb` | Array + +### References + +* [https://rails.rubystyle.guide#enums](https://rails.rubystyle.guide#enums) + ## Rails/EnumUniqueness Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/rails/enum_hash_spec.rb b/spec/rubocop/cop/rails/enum_hash_spec.rb new file mode 100644 index 0000000000..a803c85a85 --- /dev/null +++ b/spec/rubocop/cop/rails/enum_hash_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::EnumHash do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + context 'when array syntax is used' do + context 'with %i[] syntax' do + it 'registers an offense' do + expect_offense(<<~RUBY) + enum status: %i[active archived] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead. + RUBY + end + end + + context 'with %w[] syntax' do + it 'registers an offense' do + expect_offense(<<~RUBY) + enum status: %w[active archived] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead. + RUBY + end + end + + context 'with %i() syntax' do + it 'registers an offense' do + expect_offense(<<~RUBY) + enum status: %i(active archived) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead. + RUBY + end + end + + context 'with %w() syntax' do + it 'registers an offense' do + expect_offense(<<~RUBY) + enum status: %w(active archived) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead. + RUBY + end + end + + context 'with [] syntax' do + it 'registers an offense' do + expect_offense(<<~RUBY) + enum status: [:active, :archived] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead. + RUBY + end + end + end + + context 'when hash syntax is used' do + it 'does not register an offense' do + expect_no_offenses('enum status: { active: 0, archived: 1 }') + end + end +end