From 4ec7f5a4679e720721175d82c981b85bd77296de Mon Sep 17 00:00:00 2001 From: Santiago Bartesaghi Date: Sun, 28 Jul 2019 14:34:50 -0300 Subject: [PATCH] Fix false negatives for Rails/EnumUniqueness cop --- CHANGELOG.md | 1 + lib/rubocop/cop/rails/enum_uniqueness.rb | 36 ++++++++++++++----- .../rubocop/cop/rails/enum_uniqueness_spec.rb | 29 +++++++++++++++ 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aec27826f..5e4bb804bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/cop/rails/enum_uniqueness.rb b/lib/rubocop/cop/rails/enum_uniqueness.rb index 58ef1d39d8..7c5f13b300 100644 --- a/lib/rubocop/cop/rails/enum_uniqueness.rb +++ b/lib/rubocop/cop/rails/enum_uniqueness.rb @@ -23,22 +23,42 @@ class EnumUniqueness < Cop MSG = 'Duplicate value `%s` found in `%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 diff --git a/spec/rubocop/cop/rails/enum_uniqueness_spec.rb b/spec/rubocop/cop/rails/enum_uniqueness_spec.rb index 0b3a8525f5..756f42ea4e 100644 --- a/spec/rubocop/cop/rails/enum_uniqueness_spec.rb +++ b/spec/rubocop/cop/rails/enum_uniqueness_spec.rb @@ -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