From dbdee93776ae3df24d35ce5440c803b1efb52bda Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 3 Sep 2024 12:30:50 +0200 Subject: [PATCH] [Fix #1343] False negatives for `Rails/EnumSyntax` I don't believe there is any issue with just looking at all value types. It just translates `foo: bar` into `:foo, bar`. Additionally, add `instance_methods` as a rails option. Not doing this would result autocorrect treating it as a enum column --- changelog/fix_false_negatives_enum_syntax.md | 1 + lib/rubocop/cop/rails/enum_syntax.rb | 26 +++++++---------- spec/rubocop/cop/rails/enum_syntax_spec.rb | 30 ++++++++++++++++++-- 3 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 changelog/fix_false_negatives_enum_syntax.md diff --git a/changelog/fix_false_negatives_enum_syntax.md b/changelog/fix_false_negatives_enum_syntax.md new file mode 100644 index 0000000000..b1d030fdf2 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/rails/enum_syntax.rb b/lib/rubocop/cop/rails/enum_syntax.rb index 39501c4331..9a36dadfef 100644 --- a/lib/rubocop/cop/rails/enum_syntax.rb +++ b/lib/rubocop/cop/rails/enum_syntax.rb @@ -24,7 +24,10 @@ class EnumSyntax < Base 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 + + # 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,10 +47,9 @@ 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 @@ -63,9 +57,7 @@ def check_and_correct_keyword_args(node) 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] == '_' diff --git a/spec/rubocop/cop/rails/enum_syntax_spec.rb b/spec/rubocop/cop/rails/enum_syntax_spec.rb index 2abf4e47fa..80c698cc05 100644 --- a/spec/rubocop/cop/rails/enum_syntax_spec.rb +++ b/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