From 8808dab9e57f1081956505c9006ebe2c959a0dae Mon Sep 17 00:00:00 2001 From: Bruno Vezoli Date: Wed, 24 Jul 2019 15:57:14 -0300 Subject: [PATCH] [Fix #78] Create HashEnum Cop --- CHANGELOG.md | 7 +++ config/default.yml | 7 +++ lib/rubocop/cop/rails/hash_enum.rb | 31 ++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + manual/cops.md | 1 + manual/cops_rails.md | 24 ++++++++++ spec/rubocop/cop/rails/hash_enum_spec.rb | 60 ++++++++++++++++++++++++ 7 files changed, 131 insertions(+) create mode 100644 lib/rubocop/cop/rails/hash_enum.rb create mode 100644 spec/rubocop/cop/rails/hash_enum_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 117e44d192..7f2e410b4d 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/HashEnum` 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..63d786626f 100644 --- a/config/default.yml +++ b/config/default.yml @@ -213,6 +213,13 @@ Rails/HasManyOrHasOneDependent: Include: - app/models/**/*.rb +Rails/HashEnum: + Description: 'Prefer hash syntax over array syntax when defining enums' + Enabled: true + VersionAdded: '0.74' + Include: + - app/models/**/*.rb + Rails/HelperInstanceVariable: Description: 'Do not use instance variables in helpers' Enabled: true diff --git a/lib/rubocop/cop/rails/hash_enum.rb b/lib/rubocop/cop/rails/hash_enum.rb new file mode 100644 index 0000000000..b208851d5e --- /dev/null +++ b/lib/rubocop/cop/rails/hash_enum.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # This cop looks for enums written with array syntax + # + # @example + # # bad + # enum status: [:active, :archived] + # + # # good + # enum status: { active: 0, archived: 1 } + # + class HashEnum < 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, _args| + 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..a704738b44 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -25,6 +25,7 @@ require_relative 'rails/find_each' require_relative 'rails/has_and_belongs_to_many' require_relative 'rails/has_many_or_has_one_dependent' +require_relative 'rails/hash_enum' require_relative 'rails/helper_instance_variable' require_relative 'rails/http_positional_arguments' require_relative 'rails/http_status' diff --git a/manual/cops.md b/manual/cops.md index d7e1aaaca6..3a598f12a7 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -24,6 +24,7 @@ * [Rails/FindEach](cops_rails.md#railsfindeach) * [Rails/HasAndBelongsToMany](cops_rails.md#railshasandbelongstomany) * [Rails/HasManyOrHasOneDependent](cops_rails.md#railshasmanyorhasonedependent) +* [Rails/HashEnum](cops_rails.md#railshashenum) * [Rails/HelperInstanceVariable](cops_rails.md#railshelperinstancevariable) * [Rails/HttpPositionalArguments](cops_rails.md#railshttppositionalarguments) * [Rails/HttpStatus](cops_rails.md#railshttpstatus) diff --git a/manual/cops_rails.md b/manual/cops_rails.md index b5b697de92..69253324c8 100644 --- a/manual/cops_rails.md +++ b/manual/cops_rails.md @@ -896,6 +896,30 @@ Include | `app/models/**/*.rb` | Array * [https://rails.rubystyle.guide#has_many-has_one-dependent-option](https://rails.rubystyle.guide#has_many-has_one-dependent-option) +## Rails/HashEnum + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | Yes | No | 0.74 | - + +This cop looks for enums written with array syntax + +### 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 + ## Rails/HelperInstanceVariable Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/rails/hash_enum_spec.rb b/spec/rubocop/cop/rails/hash_enum_spec.rb new file mode 100644 index 0000000000..9398d764d5 --- /dev/null +++ b/spec/rubocop/cop/rails/hash_enum_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::HashEnum 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