diff --git a/changelog/new_add_new_rails_enum_syntax_cop.md b/changelog/new_add_new_rails_enum_syntax_cop.md new file mode 100644 index 0000000000..19909d3b66 --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 1f3b5376dc..f2b87a6367 100644 --- a/config/default.yml +++ b/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: '<>' + Include: + - app/models/**/*.rb + Rails/EnumUniqueness: Description: 'Avoid duplicate integers in hash-syntax `enum` declaration.' Enabled: true diff --git a/lib/rubocop/cop/rails/enum_syntax.rb b/lib/rubocop/cop/rails/enum_syntax.rb new file mode 100644 index 0000000000..39501c4331 --- /dev/null +++ b/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 `%s` enum declaration. Use positional arguments instead.' + MSG_OPTIONS = 'Enum defined with deprecated options in `%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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index e5173baeb0..b7eddc4fe4 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/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' diff --git a/spec/rubocop/cop/rails/enum_syntax_spec.rb b/spec/rubocop/cop/rails/enum_syntax_spec.rb new file mode 100644 index 0000000000..2abf4e47fa --- /dev/null +++ b/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