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/EnumUniqueness cop #97

Merged
merged 1 commit into from Aug 9, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
### 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][])
* [#97](https://github.com/rubocop-hq/rubocop-rails/pull/97): Fix two false negatives for `Rails/EnumUniqueness`. 1. When `enum` name is not a literal. 2. When `enum` has multiple definitions. ([@santib][])

### Changes

Expand Down
36 changes: 28 additions & 8 deletions lib/rubocop/cop/rails/enum_uniqueness.rb
Expand Up @@ -23,22 +23,42 @@ class EnumUniqueness < Cop
MSG = 'Duplicate value `%<value>s` found in `%<enum>s` ' \
'enum declaration.'

def_node_matcher :enum_declaration, <<-PATTERN
(send nil? :enum (hash (pair (_ $_) ${array hash})))
def_node_matcher :enum?, <<~PATTERN
(send nil? :enum (hash $...))
PATTERN

def_node_matcher :enum_values, <<~PATTERN
(pair $_ ${array hash})
PATTERN

def on_send(node)
enum_declaration(node) do |name, args|
items = args.values
enum?(node) do |pairs|
pairs.each do |pair|
enum_values(pair) do |key, args|
items = args.values

return unless duplicates?(items)
next unless duplicates?(items)

consecutive_duplicates(items).each do |item|
add_offense(item, message: format(MSG, value: item.source,
enum: name))
consecutive_duplicates(items).each do |item|
add_offense(item, message: format(
MSG, value: item.source, enum: enum_name(key)
))
end
end
end
end
end

private

def enum_name(key)
case key.type
when :sym, :str
key.value
else
key.source
end
end
end
end
end
Expand Down
29 changes: 29 additions & 0 deletions spec/rubocop/cop/rails/enum_uniqueness_spec.rb
Expand Up @@ -85,4 +85,33 @@
end
end
end

context 'when the enum name is a string' do
it 'registers an offense' do
expect_offense(<<~RUBY)
enum "status" => [:active, :archived, :active]
^^^^^^^ Duplicate value `:active` found in `status` enum declaration.
RUBY
end
end

context 'when the enum name is not a literal' do
it 'registers an offense' do
expect_offense(<<~RUBY)
enum KEY => [:active, :archived, :active]
^^^^^^^ Duplicate value `:active` found in `KEY` enum declaration.
RUBY
end
end

context 'with multiple enum definition for a `enum` method call' do
it 'registers an offense' do
expect_offense(<<~RUBY)
enum status: [:active, :archived, :active],
^^^^^^^ Duplicate value `:active` found in `status` enum declaration.
role: [:owner, :member, :guest, :member]
^^^^^^^ Duplicate value `:member` found in `role` enum declaration.
RUBY
end
end
end