Merge pull request #1336 from koic/add_new_rails_enum_syntax_cop

[Fix #1238] Add new `Rails/EnumSyntax` cop

changelog/new_add_new_rails_enum_syntax_cop.md

@@ -0,0 +1 @@
+* [#1238](https://github.com/rubocop/rubocop-rails/issues/1238): Add new `Rails/EnumSyntax` cop. ([@maxprokopiev][], [@koic][])

config/default.yml

@@ -424,6 +424,14 @@ Rails/EnumHash:
   Include:
     - app/models/**/*.rb
 
+Rails/EnumSyntax:
+  Description: 'Use positional arguments over keyword arguments when defining enums.'
+  Enabled: pending
+  Severity: warning
+  VersionAdded: '<<next>>'
+  Include:
+    - app/models/**/*.rb
+
 Rails/EnumUniqueness:
   Description: 'Avoid duplicate integers in hash-syntax `enum` declaration.'
   Enabled: true

lib/rubocop/cop/rails/enum_syntax.rb

@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Looks for enums written with keyword arguments syntax.
+      #
+      # Defining enums with keyword arguments syntax is deprecated and will be removed in Rails 8.0.
+      # Positional arguments should be used instead:
+      #
+      # @example
+      #   # bad
+      #   enum status: { active: 0, archived: 1 }, _prefix: true
+      #
+      #   # good
+      #   enum :status, { active: 0, archived: 1 }, prefix: true
+      #
+      class EnumSyntax < Base
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        minimum_target_rails_version 7.0
+
+        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
+
+        def_node_matcher :enum?, <<~PATTERN
+          (send nil? :enum (hash $...))
+        PATTERN
+
+        def_node_matcher :enum_with_options?, <<~PATTERN
+          (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)
+        end
+
+        private
+
+        def check_and_correct_keyword_args(node)
+          enum?(node) do |pairs|
+            pairs.each do |pair|
+              key, values = enum_values(pair)
+              next unless key
+
+              correct_keyword_args(node, key, values, 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] == '_'
+            end
+          end
+        end
+
+        def correct_keyword_args(node, key, values, options)
+          add_offense(values, message: format(MSG, enum: enum_name_value(key))) do |corrector|
+            # TODO: Multi-line autocorrect could be implemented in the future.
+            next if multiple_enum_definitions?(node)
+
+            preferred_syntax = "enum #{enum_name(key)}, #{values.source}#{correct_options(options)}"
+
+            corrector.replace(node, preferred_syntax)
+          end
+        end
+
+        def multiple_enum_definitions?(node)
+          keys = node.first_argument.keys.map { |key| key.source.delete_prefix('_') }
+          filterred_keys = keys.filter { |key| !OPTION_NAMES.include?(key) }
+          filterred_keys.size >= 2
+        end
+
+        def enum_name_value(key)
+          case key.type
+          when :sym, :str
+            key.value
+          else
+            key.source
+          end
+        end
+
+        def enum_name(elem)
+          case elem.type
+          when :str
+            elem.value.dump
+          when :sym
+            elem.value.inspect
+          else
+            elem.source
+          end
+        end
+
+        def correct_options(options)
+          corrected_options = options.map do |pair|
+            name = if pair.key.source[0] == '_'
+                     pair.key.source[1..]
+                   else
+                     pair.key.source
+                   end
+
+            "#{name}: #{pair.value.source}"
+          end.join(', ')
+
+          ", #{corrected_options}" unless corrected_options.empty?
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -46,6 +46,7 @@
 require_relative 'rails/dynamic_find_by'
 require_relative 'rails/eager_evaluation_log_message'
 require_relative 'rails/enum_hash'
+require_relative 'rails/enum_syntax'
 require_relative 'rails/enum_uniqueness'
 require_relative 'rails/env_local'
 require_relative 'rails/environment_comparison'

spec/rubocop/cop/rails/enum_syntax_spec.rb

@@ -0,0 +1,168 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::EnumSyntax, :config do
+  context 'Rails >= 7.0', :rails70 do
+    context 'when keyword arguments are used' do
+      context 'with %i[] syntax' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum status: %i[active archived], _prefix: true
+                         ^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `status` enum declaration. Use positional arguments instead.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            enum :status, %i[active archived], prefix: true
+          RUBY
+        end
+      end
+
+      context 'with %i[] syntax with multiple enum definitions' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum x: %i[foo bar], y: %i[baz qux]
+                    ^^^^^^^^^^^ Enum defined with keyword arguments in `x` enum declaration. Use positional arguments instead.
+                                    ^^^^^^^^^^^ Enum defined with keyword arguments in `y` enum declaration. Use positional arguments instead.
+          RUBY
+
+          # TODO: It could be autocorrected, but it hasn't been implemented yet because it's a rare case.
+          #
+          #   enum :x, %i[foo bar]
+          #   enum :y, %i[baz qux]
+          #
+          expect_no_corrections
+        end
+      end
+
+      context 'with hash syntax' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum status: { active: 0, archived: 1 }, _prefix: 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
+          RUBY
+        end
+      end
+
+      context 'with options prefixed with `_`' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum :status, { active: 0, archived: 1 }, _prefix: true, _suffix: true
+                                                                     ^^^^^^^ Enum defined with deprecated options in `status` enum declaration. Remove the `_` prefix.
+                                                      ^^^^^^^ Enum defined with deprecated options in `status` enum declaration. Remove the `_` prefix.
+          RUBY
+        end
+      end
+
+      context 'when the enum name is a string' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum "status" => { active: 0, archived: 1 }
+                             ^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `status` enum declaration. Use positional arguments instead.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            enum "status", { active: 0, archived: 1 }
+          RUBY
+        end
+      end
+
+      context 'when the enum name is not a literal' do
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            enum KEY => { active: 0, archived: 1 }
+                        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `KEY` enum declaration. Use positional arguments instead.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            enum KEY, { active: 0, archived: 1 }
+          RUBY
+        end
+      end
+
+      it 'autocorrects' do
+        expect_offense(<<~RUBY)
+          enum status: { active: 0, archived: 1 }
+                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined with keyword arguments in `status` enum declaration. Use positional arguments instead.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          enum :status, { active: 0, archived: 1 }
+        RUBY
+      end
+
+      it 'autocorrects options too' do
+        expect_offense(<<~RUBY)
+          enum status: { active: 0, archived: 1 }, _prefix: true, _suffix: true, _default: :active, _scopes: 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
+        RUBY
+      end
+    end
+
+    context 'when positional arguments are used' do
+      it 'does not register an offense' do
+        expect_no_offenses('enum :status, { active: 0, archived: 1 }, prefix: true')
+      end
+    end
+
+    context 'when enum with no arguments' do
+      it 'does not register an offense' do
+        expect_no_offenses('enum')
+      end
+    end
+  end
+
+  context 'Rails <= 6.1', :rails61 do
+    context 'when keyword arguments are used' do
+      context 'with %i[] syntax' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            enum status: { active: 0, archived: 1 }, _prefix: true
+          RUBY
+        end
+      end
+
+      context 'with options prefixed with `_`' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            enum :status, { active: 0, archived: 1 }, _prefix: true, _suffix: true
+          RUBY
+        end
+      end
+
+      context 'when the enum name is a string' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            enum "status" => { active: 0, archived: 1 }
+          RUBY
+        end
+      end
+
+      context 'when the enum name is not a literal' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            enum KEY => { active: 0, archived: 1 }
+          RUBY
+        end
+      end
+
+      it 'autocorrects' do
+        expect_no_offenses(<<~RUBY)
+          enum status: { active: 0, archived: 1 }
+        RUBY
+      end
+
+      it 'autocorrects options too' do
+        expect_no_offenses(<<~RUBY)
+          enum status: { active: 0, archived: 1 }, _prefix: true, _suffix: true
+        RUBY
+      end
+    end
+  end
+end