From 819871cc4edb5132039cbc9b86abf6ad914bce06 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 26 Feb 2022 02:45:24 +0900 Subject: [PATCH] Enable `Style/MapToHash` cop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby 2.6 added block argument processing to `Enumerable#to_h`. https://bugs.ruby-lang.org/issues/15143 Rails 7 requires Ruby 2.7.0 or higher, so the new feature can use it. `Style/MapToHash` cop will detect it. And this cop in the `Style` department, but this seems to improve performance as follows: ```ruby # map_to_hash.rb require 'benchmark/ips' ARRAY = (1..100).to_a HASH = {foo: 1, bar: 2} Benchmark.ips do |x| x.report('array.map.to_h') { ARRAY.map { |v| [v, v * 2] }.to_h } x.report('array.to_h') { ARRAY.to_h { |v| [v, v * 2] } } x.compare! end Benchmark.ips do |x| x.report('hash.map.to_h') { HASH.map { |k, v| [k.to_s, v * 2] }.to_h } x.report('hash.to_h') { HASH.to_h { |k, v| [k.to_s, v * 2] } } x.compare! end ``` ```console % ruby map_to_hash.rb Warming up -------------------------------------- array.map.to_h 9.063k i/100ms array.to_h 9.609k i/100ms Calculating ------------------------------------- array.map.to_h 89.063k (± 3.9%) i/s - 453.150k in 5.096572s array.to_h 96.449k (± 1.7%) i/s - 490.059k in 5.082529s Comparison: array.to_h: 96448.7 i/s array.map.to_h: 89063.4 i/s - 1.08x (± 0.00) slower Warming up -------------------------------------- hash.map.to_h 106.284k i/100ms hash.to_h 149.354k i/100ms Calculating ------------------------------------- hash.map.to_h 1.102M (± 2.2%) i/s - 5.527M in 5.019657s hash.to_h 1.490M (± 0.9%) i/s - 7.468M in 5.013264s Comparison: hash.to_h: 1489707.0 i/s hash.map.to_h: 1101561.5 i/s - 1.35x (± 0.00) slower ``` `Style/MapToHash` cop ... https://docs.rubocop.org/rubocop/1.25/cops_style.html#stylemaptohash --- .rubocop.yml | 3 +++ actiontext/lib/action_text/attachment.rb | 2 +- actiontext/lib/action_text/trix_attachment.rb | 4 ++-- actionview/lib/action_view/ripper_ast_parser.rb | 8 ++++---- .../connection_adapters/mysql/schema_statements.rb | 4 ++-- .../lib/active_record/encryption/encryptable_record.rb | 4 ++-- activestorage/test/service/mirror_service_test.rb | 4 ++-- 7 files changed, 16 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 17b6b40af2..5bbdd11447 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -159,6 +159,9 @@ Style/FrozenStringLiteralComment: - 'actionmailbox/db/migrate/**/*.rb' - 'actiontext/db/migrate/**/*.rb' +Style/MapToHash: + Enabled: true + Style/RedundantFreeze: Enabled: true diff --git a/actiontext/lib/action_text/attachment.rb b/actiontext/lib/action_text/attachment.rb index 2e07a42171..f77e1a0965 100644 --- a/actiontext/lib/action_text/attachment.rb +++ b/actiontext/lib/action_text/attachment.rb @@ -91,7 +91,7 @@ module ActionText private def node_attributes - @node_attributes ||= ATTRIBUTES.map { |name| [ name.underscore, node[name] ] }.to_h.compact + @node_attributes ||= ATTRIBUTES.to_h { |name| [ name.underscore, node[name] ] }.compact end def attachable_attributes diff --git a/actiontext/lib/action_text/trix_attachment.rb b/actiontext/lib/action_text/trix_attachment.rb index 8b9642e95d..bff7a44665 100644 --- a/actiontext/lib/action_text/trix_attachment.rb +++ b/actiontext/lib/action_text/trix_attachment.rb @@ -39,10 +39,10 @@ module ActionText end def typecast_attribute_values(attributes) - attributes.map do |key, value| + attributes.to_h do |key, value| typecast = ATTRIBUTE_TYPES[key] || ATTRIBUTE_TYPES[:default] [key, typecast.call(value)] - end.to_h + end end end diff --git a/actionview/lib/action_view/ripper_ast_parser.rb b/actionview/lib/action_view/ripper_ast_parser.rb index 09af3b670f..eef5b56cb5 100644 --- a/actionview/lib/action_view/ripper_ast_parser.rb +++ b/actionview/lib/action_view/ripper_ast_parser.rb @@ -86,11 +86,11 @@ module ActionView end def hash_from_body(body) - body.map do |hash_node| + body.to_h do |hash_node| return nil if hash_node.type != :assoc_new [hash_node[0], hash_node[1]] - end.to_h + end end def symbol? @@ -189,9 +189,9 @@ module ActionView parser = RenderCallExtractor.new(code) parser.parse - parser.render_calls.group_by(&:first).collect do |method, nodes| + parser.render_calls.group_by(&:first).to_h do |method, nodes| [ method.to_sym, nodes.collect { |v| v[1] } ] - end.to_h + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 6a7c50c963..9f7276e75d 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -57,9 +57,9 @@ module ActiveRecord orders = options.delete(:orders) lengths = options.delete(:lengths) - columns = index[-1].map { |name| + columns = index[-1].to_h { |name| [ name.to_sym, expressions[name] || +quote_column_name(name) ] - }.to_h + } index[-1] = add_options_for_index_columns( columns, order: orders, length: lengths diff --git a/activerecord/lib/active_record/encryption/encryptable_record.rb b/activerecord/lib/active_record/encryption/encryptable_record.rb index b26e6efead..cf7cede8ee 100644 --- a/activerecord/lib/active_record/encryption/encryptable_record.rb +++ b/activerecord/lib/active_record/encryption/encryptable_record.rb @@ -188,12 +188,12 @@ module ActiveRecord end def build_decrypt_attribute_assignments - Array(self.class.encrypted_attributes).collect do |attribute_name| + Array(self.class.encrypted_attributes).to_h do |attribute_name| type = type_for_attribute(attribute_name) encrypted_value = ciphertext_for(attribute_name) new_value = type.deserialize(encrypted_value) [attribute_name, new_value] - end.to_h + end end def cant_modify_encrypted_attributes_when_frozen diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index 32b3954bce..4e86aaae91 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -3,11 +3,11 @@ require "service/shared_service_tests" class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase - mirror_config = (1..3).map do |i| + mirror_config = (1..3).to_h do |i| [ "mirror_#{i}", service: "Disk", root: Dir.mktmpdir("active_storage_tests_mirror_#{i}") ] - end.to_h + end config = mirror_config.merge \ mirror: { service: "Mirror", primary: "primary", mirrors: mirror_config.keys },