From b0441e3a50718acf573e694004c7f3d1a2e4e89c Mon Sep 17 00:00:00 2001 From: fatkodima Date: Mon, 19 Aug 2024 00:28:04 +0300 Subject: [PATCH] Add new `Rails/HashLiteralKeysConversion` cop --- .../new_hash_literal_keys_conversion_cop.md | 1 + config/default.yml | 5 + .../cop/rails/hash_literal_keys_conversion.rb | 146 +++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../hash_literal_keys_conversion_spec.rb | 276 ++++++++++++++++++ 5 files changed, 429 insertions(+) create mode 100644 changelog/new_hash_literal_keys_conversion_cop.md create mode 100644 lib/rubocop/cop/rails/hash_literal_keys_conversion.rb create mode 100644 spec/rubocop/cop/rails/hash_literal_keys_conversion_spec.rb diff --git a/changelog/new_hash_literal_keys_conversion_cop.md b/changelog/new_hash_literal_keys_conversion_cop.md new file mode 100644 index 0000000000..0793e05948 --- /dev/null +++ b/changelog/new_hash_literal_keys_conversion_cop.md @@ -0,0 +1 @@ +* [#1332](https://github.com/rubocop/rubocop-rails/issues/1332): Add new `Rails/HashLiteralKeysConversion` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index 1f3b5376dc..50edaef1b2 100644 --- a/config/default.yml +++ b/config/default.yml @@ -540,6 +540,11 @@ Rails/HasManyOrHasOneDependent: Include: - app/models/**/*.rb +Rails/HashLiteralKeysConversion: + Description: 'Convert hash literal keys manually instead of using keys conversion methods.' + Enabled: pending + VersionAdded: '<>' + Rails/HelperInstanceVariable: Description: 'Do not use instance variables in helpers.' Enabled: true diff --git a/lib/rubocop/cop/rails/hash_literal_keys_conversion.rb b/lib/rubocop/cop/rails/hash_literal_keys_conversion.rb new file mode 100644 index 0000000000..51aa8c9b14 --- /dev/null +++ b/lib/rubocop/cop/rails/hash_literal_keys_conversion.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Detects when keys conversion methods are called on literal hashes, where it is redundant + # or keys can be manually converted to the required type. + # + # @example + # # bad + # { a: 1, b: 2 }.symbolize_keys + # + # # bad + # { a: 1, b: 2 }.stringify_keys + # + # # good + # { 'a' => 1, 'b' => 2 } + # + # # good + # { a: 1, var => 3 }.symbolize_keys + # + # # good + # { a:, b: 2 }.stringify_keys + # { a: 1, b: foo }.deep_stringify_keys + # + class HashLiteralKeysConversion < Base + extend AutoCorrector + + REDUNDANT_CONVERSION_MSG = 'Redundant hash keys conversion, all the keys have the required type.' + MSG = 'Convert hash keys explicitly to the required type.' + + CONVERSION_METHODS = { + symbolize_keys: :sym, + symbolize_keys!: :sym, + stringify_keys: :str, + stringify_keys!: :str, + deep_symbolize_keys: :sym, + deep_symbolize_keys!: :sym, + deep_stringify_keys: :str, + deep_stringify_keys!: :str + }.freeze + + RESTRICT_ON_SEND = CONVERSION_METHODS.keys + + def on_send(node) + return unless (receiver = node.receiver)&.hash_type? + + type = CONVERSION_METHODS[node.method_name] + deep = node.method_name.start_with?('deep_') + return unless convertible_hash?(receiver, deep: deep) + + check(node, receiver, type: type, deep: deep) + end + + # rubocop:disable Metrics/AbcSize + def check(node, hash_node, type: :sym, deep: false) + pair_nodes = pair_nodes(hash_node, deep: deep) + + type_pairs, other_pairs = pair_nodes.partition { |pair_node| pair_node.key.type == type } + + if type_pairs == pair_nodes + add_offense(node.loc.selector, message: REDUNDANT_CONVERSION_MSG) do |corrector| + corrector.remove(node.loc.dot) + corrector.remove(node.loc.selector) + end + else + add_offense(node.loc.selector) do |corrector| + corrector.remove(node.loc.dot) + corrector.remove(node.loc.selector) + autocorrect_hash_keys(other_pairs, type, corrector) + end + end + end + # rubocop:enable Metrics/AbcSize + + private + + def convertible_hash?(node, deep: false) + node.pairs.each do |pair| + return false unless convertible_key?(pair) + return false if deep && !convertible_node?(pair.value, deep: deep) + end + + true + end + + def convertible_key?(pair) + key, _value = *pair + + (key.str_type? || key.sym_type?) && !pair.value_omission? && !key.value.match?(/\W/) + end + + def convertible_array?(node, deep:) + node.values.all? do |value| + convertible_node?(value, deep: deep) + end + end + + def convertible_node?(node, deep:) + if node.hash_type? + convertible_hash?(node, deep: deep) + elsif node.array_type? + convertible_array?(node, deep: deep) + else + node.literal? + end + end + + def pair_nodes(hash_node, deep: false) + if deep + pair_nodes = [] + do_pair_nodes(hash_node, pair_nodes) + pair_nodes + else + hash_node.pairs + end + end + + def do_pair_nodes(node, pair_nodes) + if node.hash_type? + node.pairs.each do |pair_node| + pair_nodes << pair_node + do_pair_nodes(pair_node.value, pair_nodes) + end + elsif node.array_type? + node.each_value do |value| + do_pair_nodes(value, pair_nodes) + end + end + end + + def autocorrect_hash_keys(pair_nodes, type, corrector) + pair_nodes.each do |pair_node| + if type == :sym + corrector.replace(pair_node.key, ":#{pair_node.key.value}") + else + corrector.replace(pair_node.key, "'#{pair_node.key.source}'") + end + + corrector.replace(pair_node.loc.operator, '=>') + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index e5173baeb0..da2d9cce18 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -59,6 +59,7 @@ require_relative 'rails/freeze_time' require_relative 'rails/has_and_belongs_to_many' require_relative 'rails/has_many_or_has_one_dependent' +require_relative 'rails/hash_literal_keys_conversion' require_relative 'rails/helper_instance_variable' require_relative 'rails/http_positional_arguments' require_relative 'rails/http_status' diff --git a/spec/rubocop/cop/rails/hash_literal_keys_conversion_spec.rb b/spec/rubocop/cop/rails/hash_literal_keys_conversion_spec.rb new file mode 100644 index 0000000000..7510efbfa9 --- /dev/null +++ b/spec/rubocop/cop/rails/hash_literal_keys_conversion_spec.rb @@ -0,0 +1,276 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::HashLiteralKeysConversion, :config do + it 'registers an offense and corrects when using `symbolize_keys` with only symbol keys' do + expect_offense(<<~RUBY) + { a: 1, b: 2 }.symbolize_keys + ^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type. + RUBY + + expect_correction(<<~RUBY) + { a: 1, b: 2 } + RUBY + end + + it 'registers an offense and corrects when using `symbolize_keys` with only symbol and string keys' do + expect_offense(<<~RUBY) + { a: 1, 'b' => 2 }.symbolize_keys + ^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { a: 1, :b => 2 } + RUBY + end + + it 'does not register an offense when using `symbolize_keys` with integer keys' do + expect_no_offenses(<<~RUBY) + { a: 1, 2 => 3 }.symbolize_keys + RUBY + end + + it 'does not register an offense when using `symbolize_keys` with non hash literal receiver' do + expect_no_offenses(<<~RUBY) + options.symbolize_keys + RUBY + end + + it 'registers an offense and corrects when using `stringify_keys` with only string keys' do + expect_offense(<<~RUBY) + { 'a' => 1, 'b' => 2 }.stringify_keys + ^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type. + RUBY + + expect_correction(<<~RUBY) + { 'a' => 1, 'b' => 2 } + RUBY + end + + it 'registers an offense and corrects when using `stringify_keys` with only symbol and string keys' do + expect_offense(<<~RUBY) + { a: 1, 'b' => 2 }.stringify_keys + ^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { 'a'=> 1, 'b' => 2 } + RUBY + end + + it 'does not register an offense when using `stringify_keys` with integer keys' do + expect_no_offenses(<<~RUBY) + { 'a' => 1, 2 => 3 }.stringify_keys + RUBY + end + + it 'does not register an offense when using `stringify_keys` with non hash literal receiver' do + expect_no_offenses(<<~RUBY) + options.stringify_keys + RUBY + end + + it 'registers an offense and corrects when using `deep_symbolize_keys` with symbol keys' do + expect_offense(<<~RUBY) + { + a: 1, + b: { + c: 1 + } + }.deep_symbolize_keys + ^^^^^^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type. + RUBY + + expect_correction(<<~RUBY) + { + a: 1, + b: { + c: 1 + } + } + RUBY + end + + it 'registers an offense and corrects when using `deep_symbolize_keys` with symbol and string keys' do + expect_offense(<<~RUBY) + { + 'a' => 1, + b: { + c: 1 + } + }.deep_symbolize_keys + ^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { + :a => 1, + b: { + c: 1 + } + } + RUBY + end + + it 'registers an offense and corrects when using `deep_symbolize_keys` with flat and only symbol and string keys' do + expect_offense(<<~RUBY) + { + 'a' => 1, + b: 2 + }.deep_symbolize_keys + ^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { + :a => 1, + b: 2 + } + RUBY + end + + it 'registers an offense and corrects when using `deep_symbolize_keys` with nested array' do + expect_offense(<<~RUBY) + { + 'a' => 1, + b: [ + 'c' => 2 + ] + }.deep_symbolize_keys + ^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { + :a => 1, + b: [ + :c => 2 + ] + } + RUBY + end + + it 'does not register an offense when using `deep_symbolize_keys` with nested array and non convertible keys' do + expect_no_offenses(<<~RUBY) + { + 'a' => 1, + b: [ + { foo => 2 } + ] + }.deep_symbolize_keys + RUBY + end + + it 'does not register an offense when using `deep_symbolize_keys` with integer keys' do + expect_no_offenses(<<~RUBY) + { + 'a' => 1, + b: { + 2 => 3 + } + }.deep_symbolize_keys + RUBY + end + + it 'does not register an offense when using `deep_symbolize_keys` with nested array having' \ + 'a hash with non literal value' do + expect_no_offenses(<<~RUBY) + { a: [{ foo: foo }] }.deep_symbolize_keys + RUBY + end + + it 'does not register an offense when using `deep_symbolize_keys` with non hash literal receiver' do + expect_no_offenses(<<~RUBY) + options.deep_symbolize_keys + RUBY + end + + it 'does not register an offense when using `deep_symbolize_keys` with non literal values' do + expect_no_offenses(<<~RUBY) + { 'a' => 1, b: foo }.deep_symbolize_keys + RUBY + end + + it 'registers an offense and corrects when using `deep_stringify_keys` with only string keys' do + expect_offense(<<~RUBY) + { + 'a' => 1, + 'b' => { + 'c' => 1 + } + }.deep_stringify_keys + ^^^^^^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type. + RUBY + + expect_correction(<<~RUBY) + { + 'a' => 1, + 'b' => { + 'c' => 1 + } + } + RUBY + end + + it 'registers an offense and corrects when using `deep_stringify_keys` with only symbol and string keys' do + expect_offense(<<~RUBY) + { + 'a' => 1, + b: { + c: 1 + } + }.deep_stringify_keys + ^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type. + RUBY + + expect_correction(<<~RUBY) + { + 'a' => 1, + 'b'=> { + 'c'=> 1 + } + } + RUBY + end + + it 'does not register an offense when using `deep_stringify_keys` with integer keys' do + expect_no_offenses(<<~RUBY) + { + 'a' => 1, + b: { + 2 => 3 + } + }.deep_stringify_keys + RUBY + end + + it 'does not register an offense when using `deep_stringify_keys` with non hash literal receiver' do + expect_no_offenses(<<~RUBY) + options.deep_stringify_keys + RUBY + end + + it 'registers an offense and autocorrects when using `symbolize_keys` with empty hash literal' do + expect_offense(<<~RUBY) + {}.symbolize_keys + ^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type. + RUBY + + expect_correction(<<~RUBY) + {} + RUBY + end + + it 'does not register an offense when using `symbolize_keys` with non alphanumeric keys' do + expect_no_offenses(<<~RUBY) + { 'hello world' => 1 }.symbolize_keys + RUBY + end + + context 'Ruby >= 3.1', :ruby31 do + it 'does not register an offense when using hash value omission' do + expect_no_offenses(<<~RUBY) + { a:, b: 2 }.stringify_keys + RUBY + end + end +end