Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false negatives for Rails/EnumHash cop #96

Merged
merged 1 commit into from Jul 26, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 24 additions & 4 deletions lib/rubocop/cop/rails/enum_hash.rb
Expand Up @@ -21,13 +21,33 @@ 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)))
def_node_matcher :enum?, <<~PATTERN
(send nil? :enum (hash $...))
PATTERN

def_node_matcher :array_pair?, <<~PATTERN
(pair $_ $array)
PATTERN

def on_send(node)
enum_with_array?(node) do |name|
add_offense(node, message: format(MSG, enum: name))
enum?(node) do |pairs|
pairs.each do |pair|
key, array = array_pair?(pair)
next unless key

add_offense(array, message: format(MSG, enum: enum_name(key)))
end
end
end

private

def enum_name(key)
case key.type
when :sym, :str
key.value
else
key.source
end
end
end
Expand Down
39 changes: 34 additions & 5 deletions spec/rubocop/cop/rails/enum_hash_spec.rb
Expand Up @@ -10,7 +10,7 @@
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.
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum status: is not a bad part, so I think the cop should add an offense only to the array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement ❤️

RUBY
end
end
Expand All @@ -19,7 +19,7 @@
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.
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
RUBY
end
end
Expand All @@ -28,7 +28,7 @@
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.
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
RUBY
end
end
Expand All @@ -37,7 +37,7 @@
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.
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
RUBY
end
end
Expand All @@ -46,7 +46,36 @@
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.
^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
RUBY
end
end

context 'when the enum name is a string' 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 'when the enum name is not a literal' do
it 'registers an offense' do
expect_offense(<<~RUBY)
enum KEY => %i[active archived]
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `KEY` enum declaration. Use hash syntax instead.
RUBY
end
end

context 'with multiple enum definition for a `enum` method call' 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.
role: %i[owner member guest]
^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `role` enum declaration. Use hash syntax instead.
RUBY
end
end
Expand Down