Handle to_h with block in hash transformation cops

Since Ruby 2.6, `Hash#to_h` accepts a block which is called with each
pair and must return a key and value to include in the new hash.

If either the key or the value are always returned unchanged, using
`Hash#transform_keys` or `Hash#transform_values` is faster and clearer.

This is similar to the existing logic that detects `map { ... }.to_h`.

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 * [#8362](https://github.com/rubocop-hq/rubocop/issues/8362): Add numbers of correctable offenses to summary. ([@nguyenquangminh0711][])
 * [#8513](https://github.com/rubocop-hq/rubocop/pull/8513): Clarify the ruby warning mentioned in the `Lint/ShadowingOuterLocalVariable` documentation. ([@chocolateboy][])
+* [#8517](https://github.com/rubocop-hq/rubocop/pull/8517): Make `Style/HashTransformKeys` and `Style/HashTransformValues` aware of `to_h` with block. ([@eugeneius][])
 
 ## 0.89.1 (2020-08-10)
 

config/default.yml

@@ -3101,15 +3101,17 @@ Style/HashSyntax:
   PreferHashRocketsForNonAlnumEndingSymbols: false
 
 Style/HashTransformKeys:
-  Description: 'Prefer `transform_keys` over `each_with_object` and `map`.'
+  Description: 'Prefer `transform_keys` over `each_with_object`, `map`, or `to_h`.'
   Enabled: 'pending'
   VersionAdded: '0.80'
+  VersionChanged: '0.90'
   Safe: false
 
 Style/HashTransformValues:
-  Description: 'Prefer `transform_values` over `each_with_object` and `map`.'
+  Description: 'Prefer `transform_values` over `each_with_object`, `map`, or `to_h`.'
   Enabled: 'pending'
   VersionAdded: '0.80'
+  VersionChanged: '0.90'
   Safe: false
 
 Style/IdenticalConditionalBranches:

docs/modules/ROOT/pages/cops_style.adoc

@@ -3794,7 +3794,7 @@ NOTE: Required Ruby version: 2.5
 | No
 | Yes (Unsafe)
 | 0.80
-| -
+| 0.90
 |===
 
 This cop looks for uses of `_.each_with_object({}) {...}`,
@@ -3815,7 +3815,9 @@ This cop should only be enabled on Ruby version 2.5 or newer
 ----
 # bad
 {a: 1, b: 2}.each_with_object({}) { |(k, v), h| h[foo(k)] = v }
-{a: 1, b: 2}.map { |k, v| [k.to_s, v] }
+Hash[{a: 1, b: 2}.collect { |k, v| [foo(k), v] }]
+{a: 1, b: 2}.map { |k, v| [k.to_s, v] }.to_h
+{a: 1, b: 2}.to_h { |k, v| [k.to_s, v] }
 
 # good
 {a: 1, b: 2}.transform_keys { |k| foo(k) }
@@ -3831,7 +3833,7 @@ This cop should only be enabled on Ruby version 2.5 or newer
 | No
 | Yes (Unsafe)
 | 0.80
-| -
+| 0.90
 |===
 
 This cop looks for uses of `_.each_with_object({}) {...}`,
@@ -3852,7 +3854,9 @@ This cop should only be enabled on Ruby version 2.4 or newer
 ----
 # bad
 {a: 1, b: 2}.each_with_object({}) { |(k, v), h| h[k] = foo(v) }
-{a: 1, b: 2}.map { |k, v| [k, v * v] }
+Hash[{a: 1, b: 2}.collect { |k, v| [k, foo(v)] }]
+{a: 1, b: 2}.map { |k, v| [k, v * v] }.to_h
+{a: 1, b: 2}.to_h { |k, v| [k, v * v] }
 
 # good
 {a: 1, b: 2}.transform_values { |v| foo(v) }

lib/rubocop/cop/mixin/hash_transform_method.rb

@@ -9,6 +9,12 @@ def on_block(node)
         on_bad_each_with_object(node) do |*match|
           handle_possible_offense(node, match, 'each_with_object')
         end
+
+        return if target_ruby_version < 2.6
+
+        on_bad_to_h(node) do |*match|
+          handle_possible_offense(node, match, 'to_h {...}')
+        end
       end
 
       def on_send(node)
@@ -43,6 +49,11 @@ def on_bad_map_to_h(_node)
         raise NotImplementedError
       end
 
+      # @abstract Implemented with `def_node_matcher`
+      def on_bad_to_h(_node)
+        raise NotImplementedError
+      end
+
       def handle_possible_offense(node, match, match_desc)
         captures = extract_captures(match)
 
@@ -82,6 +93,8 @@ def prepare_correction(node)
           Autocorrection.from_hash_brackets_map(node, match)
         elsif (match = on_bad_map_to_h(node))
           Autocorrection.from_map_to_h(node, match)
+        elsif (match = on_bad_to_h(node))
+          Autocorrection.from_to_h(node, match)
         else
           raise 'unreachable'
         end
@@ -137,6 +150,10 @@ def self.from_map_to_h(node, match)
           new(match, node.children.first, 0, strip_trailing_chars)
         end
 
+        def self.from_to_h(node, match)
+          new(match, node, 0, 0)
+        end
+
         def strip_prefix_and_suffix(node, corrector)
           expression = node.loc.expression
           corrector.remove_leading(expression, leading)

lib/rubocop/cop/style/hash_transform_keys.rb

@@ -18,7 +18,9 @@ module Style
       # @example
       #   # bad
       #   {a: 1, b: 2}.each_with_object({}) { |(k, v), h| h[foo(k)] = v }
-      #   {a: 1, b: 2}.map { |k, v| [k.to_s, v] }
+      #   Hash[{a: 1, b: 2}.collect { |k, v| [foo(k), v] }]
+      #   {a: 1, b: 2}.map { |k, v| [k.to_s, v] }.to_h
+      #   {a: 1, b: 2}.to_h { |k, v| [k.to_s, v] }
       #
       #   # good
       #   {a: 1, b: 2}.transform_keys { |k| foo(k) }
@@ -68,6 +70,17 @@ class HashTransformKeys < Base
             :to_h)
         PATTERN
 
+        def_node_matcher :on_bad_to_h, <<~PATTERN
+          (block
+            ({send csend}
+              !{(send _ :each_with_index) (array ...)}
+              :to_h)
+            (args
+              (arg $_)
+              (arg _val))
+            (array $_ $(lvar _val)))
+        PATTERN
+
         private
 
         def extract_captures(match)

lib/rubocop/cop/style/hash_transform_values.rb

@@ -18,7 +18,9 @@ module Style
       # @example
       #   # bad
       #   {a: 1, b: 2}.each_with_object({}) { |(k, v), h| h[k] = foo(v) }
-      #   {a: 1, b: 2}.map { |k, v| [k, v * v] }
+      #   Hash[{a: 1, b: 2}.collect { |k, v| [k, foo(v)] }]
+      #   {a: 1, b: 2}.map { |k, v| [k, v * v] }.to_h
+      #   {a: 1, b: 2}.to_h { |k, v| [k, v * v] }
       #
       #   # good
       #   {a: 1, b: 2}.transform_values { |v| foo(v) }
@@ -65,6 +67,17 @@ class HashTransformValues < Base
             :to_h)
         PATTERN
 
+        def_node_matcher :on_bad_to_h, <<~PATTERN
+          (block
+            ({send csend}
+              !{(send _ :each_with_index) (array ...)}
+              :to_h)
+            (args
+              (arg _key)
+              (arg $_))
+            (array $(lvar _key) $_))
+        PATTERN
+
         private
 
         def extract_captures(match)

spec/rubocop/cop/style/hash_transform_keys_spec.rb

@@ -171,4 +171,25 @@
       expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[foo(k)] = v}')
     end
   end
+
+  context 'when using Ruby 2.6 or newer', :ruby26 do
+    it 'flags _.to_h{...} when transform_keys could be used' do
+      expect_offense(<<~RUBY)
+        x.to_h {|k, v| [k.to_sym, v]}
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `to_h {...}`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.transform_keys {|k| k.to_sym}
+      RUBY
+    end
+  end
+
+  context 'below Ruby 2.6', :ruby25 do
+    it 'does not flag _.to_h{...}' do
+      expect_no_offenses(<<~RUBY)
+        x.to_h {|k, v| [k.to_sym, v]}
+      RUBY
+    end
+  end
 end

spec/rubocop/cop/style/hash_transform_values_spec.rb

@@ -133,4 +133,25 @@
       {a: 1, b: 2}.transform_values {|v| foo(v)}.to_h {|k, v| [v, k]}
     RUBY
   end
+
+  context 'when using Ruby 2.6 or newer', :ruby26 do
+    it 'flags _.to_h{...} when transform_values could be used' do
+      expect_offense(<<~RUBY)
+        x.to_h {|k, v| [k, foo(v)]}
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `to_h {...}`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.transform_values {|v| foo(v)}
+      RUBY
+    end
+  end
+
+  context 'below Ruby 2.6', :ruby25 do
+    it 'does not flag _.to_h{...}' do
+      expect_no_offenses(<<~RUBY)
+        x.to_h {|k, v| [k, foo(v)]}
+      RUBY
+    end
+  end
 end