Merge pull request #1348 from Earlopain/enum-syntax-non-literal

[Fix #1343] False negatives for `Rails/EnumSyntax`

changelog/fix_false_negatives_enum_syntax.md

@@ -0,0 +1 @@
+* [#1343](https://github.com/rubocop/rubocop-rails/issues/1343): Fix false negatives for `Rails/EnumSyntax` for non-literal mappings. ([@earlopain][])

lib/rubocop/cop/rails/enum_syntax.rb

@@ -24,7 +24,10 @@ class EnumSyntax < Base
         MSG = 'Enum defined with keyword arguments in `%<enum>s` enum declaration. Use positional arguments instead.'
         MSG_OPTIONS = 'Enum defined with deprecated options in `%<enum>s` enum declaration. Remove the `_` prefix.'
         RESTRICT_ON_SEND = %i[enum].freeze
-        OPTION_NAMES = %w[prefix suffix scopes default].freeze
+
+        # From https://github.com/rails/rails/blob/v7.2.1/activerecord/lib/active_record/enum.rb#L231
+        OPTION_NAMES = %w[prefix suffix scopes default instance_methods].freeze
+        UNDERSCORED_OPTION_NAMES = OPTION_NAMES.map { |option| "_#{option}" }.freeze
 
         def_node_matcher :enum?, <<~PATTERN
           (send nil? :enum (hash $...))
@@ -34,14 +37,6 @@ class EnumSyntax < Base
           (send nil? :enum $_ ${array hash} $_)
         PATTERN
 
-        def_node_matcher :enum_values, <<~PATTERN
-          (pair $_ ${array hash})
-        PATTERN
-
-        def_node_matcher :enum_options, <<~PATTERN
-          (pair $_ $_)
-        PATTERN
-
         def on_send(node)
           check_and_correct_keyword_args(node)
           check_enum_options(node)
@@ -52,20 +47,17 @@ def on_send(node)
         def check_and_correct_keyword_args(node)
           enum?(node) do |pairs|
             pairs.each do |pair|
-              key, values = enum_values(pair)
-              next unless key
+              next if option_key?(pair)
 
-              correct_keyword_args(node, key, values, pairs[1..])
+              correct_keyword_args(node, pair.key, pair.value, pairs[1..])
             end
           end
         end
 
         def check_enum_options(node)
           enum_with_options?(node) do |key, _, options|
             options.children.each do |option|
-              name, = enum_options(option)
-
-              add_offense(name, message: format(MSG_OPTIONS, enum: enum_name_value(key))) if name.source[0] == '_'
+              add_offense(option.key, message: format(MSG_OPTIONS, enum: enum_name_value(key))) if option_key?(option)
             end
           end
         end
@@ -107,6 +99,10 @@ def enum_name(elem)
           end
         end
 
+        def option_key?(pair)
+          UNDERSCORED_OPTION_NAMES.include?(pair.key.source)
+        end
+
         def correct_options(options)
           corrected_options = options.map do |pair|
             name = if pair.key.source[0] == '_'

spec/rubocop/cop/rails/enum_syntax_spec.rb

@@ -82,6 +82,32 @@
         end
       end
 
+      context 'when the enum name is underscored' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum :_key => { active: 0, archived: 1 }, _prefix: true
+                          ^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `_key` enum declaration. Use positional arguments instead.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            enum :_key, { active: 0, archived: 1 }, prefix: true
+          RUBY
+        end
+      end
+
+      context 'when the enum value is not a literal' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum key: %i[foo bar].map.with_index { |v, i| [v, i] }.to_h
+                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `key` enum declaration. Use positional arguments instead.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            enum :key, %i[foo bar].map.with_index { |v, i| [v, i] }.to_h
+          RUBY
+        end
+      end
+
       it 'autocorrects' do
         expect_offense(<<~RUBY)
           enum status: { active: 0, archived: 1 }
@@ -95,12 +121,12 @@
 
       it 'autocorrects options too' do
         expect_offense(<<~RUBY)
-          enum status: { active: 0, archived: 1 }, _prefix: true, _suffix: true, _default: :active, _scopes: true
+          enum status: { active: 0, archived: 1 }, _prefix: true, _suffix: true, _default: :active, _scopes: true, _instance_methods: true
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `status` enum declaration. Use positional arguments instead.
         RUBY
 
         expect_correction(<<~RUBY)
-          enum :status, { active: 0, archived: 1 }, prefix: true, suffix: true, default: :active, scopes: true
+          enum :status, { active: 0, archived: 1 }, prefix: true, suffix: true, default: :active, scopes: true, instance_methods: true
         RUBY
       end
     end