Merge pull request #94 from santib/add-hashenum-cop

[#78] Create EnumHash Cop

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

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

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 `%<enum>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

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'

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)
 
-<!-- END_COP_LIST -->
+<!-- END_COP_LIST -->

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

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