Add cops to enforce use of transform_keys and transform_values (#…

…7663)

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#7663](https://github.com/rubocop-hq/rubocop/pull/7663): Add new `Style/HashTransformKeys` and `Style/HashTransformValues` cops. ([@djudd][], [@eugeneius][])
 * [#7619](https://github.com/rubocop-hq/rubocop/issues/7619): Support autocorrect of legacy cop names for `Migration/DepartmentName`. ([@koic][])
 
 ### Bug fixes
@@ -4344,3 +4345,4 @@
 [@Tietew]: https://github.com/Tietew
 [@hanachin]: https://github.com/hanachin
 [@masarakki]: https://github.com/masarakki
+[@djudd]: https://github.com/djudd

config/default.yml

@@ -2800,6 +2800,16 @@ Style/HashSyntax:
   # Do not suggest { a?: 1 } over { :a? => 1 } in ruby19 style
   PreferHashRocketsForNonAlnumEndingSymbols: false
 
+Style/HashTransformKeys:
+  Description: 'Prefer `transform_keys` over `each_with_object` and `map`.'
+  Enabled: 'pending'
+  Safe: false
+
+Style/HashTransformValues:
+  Description: 'Prefer `transform_values` over `each_with_object` and `map`.'
+  Enabled: 'pending'
+  Safe: false
+
 Style/IdenticalConditionalBranches:
   Description: >-
                  Checks that conditional statements do not have an identical

lib/rubocop.rb

@@ -120,6 +120,7 @@
 require_relative 'rubocop/cop/mixin/first_element_line_break'
 require_relative 'rubocop/cop/mixin/frozen_string_literal'
 require_relative 'rubocop/cop/mixin/hash_alignment_styles'
+require_relative 'rubocop/cop/mixin/hash_transform_method'
 require_relative 'rubocop/cop/mixin/ignored_pattern'
 require_relative 'rubocop/cop/mixin/ignored_methods'
 require_relative 'rubocop/cop/mixin/integer_node'
@@ -445,6 +446,8 @@
 require_relative 'rubocop/cop/style/global_vars'
 require_relative 'rubocop/cop/style/guard_clause'
 require_relative 'rubocop/cop/style/hash_syntax'
+require_relative 'rubocop/cop/style/hash_transform_keys'
+require_relative 'rubocop/cop/style/hash_transform_values'
 require_relative 'rubocop/cop/style/identical_conditional_branches'
 require_relative 'rubocop/cop/style/if_inside_else'
 require_relative 'rubocop/cop/style/if_unless_modifier'

lib/rubocop/cop/mixin/hash_transform_method.rb

@@ -0,0 +1,172 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Common functionality for Style/HashTransformKeys and
+    # Style/HashTransformValues
+    module HashTransformMethod
+      def on_block(node)
+        on_bad_each_with_object(node) do |*match|
+          handle_possible_offense(node, match, 'each_with_object')
+        end
+      end
+
+      def on_send(node)
+        on_bad_hash_brackets_map(node) do |*match|
+          handle_possible_offense(node, match, 'Hash[_.map {...}]')
+        end
+        on_bad_map_to_h(node) do |*match|
+          handle_possible_offense(node, match, 'map {...}.to_h')
+        end
+      end
+
+      def on_csend(node)
+        on_bad_map_to_h(node) do |*match|
+          handle_possible_offense(node, match, 'map {...}.to_h')
+        end
+      end
+
+      def autocorrect(node)
+        lambda do |corrector|
+          correction = prepare_correction(node)
+          execute_correction(corrector, node, correction)
+        end
+      end
+
+      private
+
+      # @abstract Implemented with `def_node_matcher`
+      def on_bad_each_with_object(_node)
+        raise NotImplementedError
+      end
+
+      # @abstract Implemented with `def_node_matcher`
+      def on_bad_hash_brackets_map(_node)
+        raise NotImplementedError
+      end
+
+      # @abstract Implemented with `def_node_matcher`
+      def on_bad_map_to_h(_node)
+        raise NotImplementedError
+      end
+
+      def handle_possible_offense(node, match, match_desc)
+        puts node.class
+        captures = extract_captures(match)
+
+        # If key didn't actually change either, this is most likely a false
+        # positive (receiver isn't a hash).
+        return if captures.noop_transformation?
+
+        # Can't `transform_keys` if key transformation uses value, or
+        # `transform_values` if value transformation uses key.
+        return if captures.transformation_uses_both_args?
+
+        add_offense(
+          node,
+          message: "Prefer `#{new_method_name}` over `#{match_desc}`."
+        )
+      end
+
+      # @abstract
+      #
+      # @return [Captures]
+      def extract_captures(_match)
+        raise NotImplementedError
+      end
+
+      # @abstract
+      #
+      # @return [String]
+      def new_method_name
+        raise NotImplementedError
+      end
+
+      def prepare_correction(node)
+        if (match = on_bad_each_with_object(node))
+          Autocorrection.from_each_with_object(node, match)
+        elsif (match = on_bad_hash_brackets_map(node))
+          Autocorrection.from_hash_brackets_map(node, match)
+        elsif (match = on_bad_map_to_h(node))
+          Autocorrection.from_map_to_h(node, match)
+        else
+          raise 'unreachable'
+        end
+      end
+
+      def execute_correction(corrector, node, correction)
+        correction.strip_prefix_and_suffix(node, corrector)
+        correction.set_new_method_name(new_method_name, corrector)
+
+        captures = extract_captures(correction.match)
+        correction.set_new_arg_name(captures.transformed_argname, corrector)
+        correction.set_new_body_expression(
+          captures.transforming_body_expr,
+          corrector
+        )
+      end
+
+      # Internal helper class to hold match data
+      Captures = Struct.new(
+        :transformed_argname,
+        :transforming_body_expr,
+        :unchanged_body_expr
+      ) do
+        def noop_transformation?
+          transforming_body_expr.lvar_type? &&
+            transforming_body_expr.children == [transformed_argname]
+        end
+
+        def transformation_uses_both_args?
+          transforming_body_expr.descendants.include?(unchanged_body_expr)
+        end
+      end
+
+      # Internal helper class to hold autocorrect data
+      Autocorrection = Struct.new(:match, :block_node, :leading, :trailing) do # rubocop:disable Metrics/BlockLength
+        def self.from_each_with_object(node, match)
+          new(match, node, 0, 0)
+        end
+
+        def self.from_hash_brackets_map(node, match)
+          new(match, node.children.last, 'Hash['.length, ']'.length)
+        end
+
+        def self.from_map_to_h(node, match)
+          strip_trailing_chars = node.parent&.block_type? ? 0 : '.to_h'.length
+          new(match, node.children.first, 0, strip_trailing_chars)
+        end
+
+        def strip_prefix_and_suffix(node, corrector)
+          expression = node.loc.expression
+          corrector.remove_leading(expression, leading)
+          corrector.remove_trailing(expression, trailing)
+        end
+
+        def set_new_method_name(new_method_name, corrector)
+          range = block_node.send_node.loc.selector
+          if (send_end = block_node.send_node.loc.end)
+            # If there are arguments (only true in the `each_with_object`
+            # case)
+            range = range.begin.join(send_end)
+          end
+          corrector.replace(range, new_method_name)
+        end
+
+        def set_new_arg_name(transformed_argname, corrector)
+          corrector.replace(
+            block_node.arguments.loc.expression,
+            "|#{transformed_argname}|"
+          )
+        end
+
+        def set_new_body_expression(transforming_body_expr, corrector)
+          corrector.replace(
+            block_node.body.loc.expression,
+            transforming_body_expr.loc.expression.source
+          )
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/style/hash_transform_keys.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop looks for uses of `_.each_with_object({}) {...}`,
+      # `_.map {...}.to_h`, and `Hash[_.map {...}]` that are actually just
+      # transforming the keys of a hash, and tries to use a simpler & faster
+      # call to `transform_keys` instead.
+      #
+      # This can produce false positives if we are transforming an enumerable
+      # of key-value-like pairs that isn't actually a hash, e.g.:
+      # `[[k1, v1], [k2, v2], ...]`
+      #
+      # This cop should only be enabled on Ruby version 2.5 or newer
+      # (`transform_keys` was added in Ruby 2.5.)
+      #
+      # @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] }
+      #
+      #   # good
+      #   {a: 1, b: 2}.transform_keys { |k| foo(k) }
+      #   {a: 1, b: 2}.transform_keys { |k| k.to_s }
+      class HashTransformKeys < Cop
+        extend TargetRubyVersion
+        include HashTransformMethod
+
+        minimum_target_ruby_version 2.5
+
+        def_node_matcher :on_bad_each_with_object, <<~PATTERN
+          (block
+            ({send csend} !(send _ :each_with_index) :each_with_object (hash))
+            (args
+              (mlhs
+                (arg $_)
+                (arg _val))
+              (arg _memo))
+            ({send csend} (lvar _memo) :[]= $_ $(lvar _val)))
+        PATTERN
+
+        def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
+          (send
+            (const _ :Hash)
+            :[]
+            (block
+              ({send csend} !(send _ :each_with_index) {:map :collect})
+              (args
+                (arg $_)
+                (arg _val))
+              (array $_ $(lvar _val))))
+        PATTERN
+
+        def_node_matcher :on_bad_map_to_h, <<~PATTERN
+          ({send csend}
+            (block
+              ({send csend} !(send _ :each_with_index) {:map :collect})
+              (args
+                (arg $_)
+                (arg _val))
+              (array $_ $(lvar _val)))
+            :to_h)
+        PATTERN
+
+        private
+
+        def extract_captures(match)
+          key_argname, key_body_expr, val_body_expr = *match
+          Captures.new(key_argname, key_body_expr, val_body_expr)
+        end
+
+        def new_method_name
+          'transform_keys'
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/style/hash_transform_values.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop looks for uses of `_.each_with_object({}) {...}`,
+      # `_.map {...}.to_h`, and `Hash[_.map {...}]` that are actually just
+      # transforming the values of a hash, and tries to use a simpler & faster
+      # call to `transform_values` instead.
+      #
+      # This can produce false positives if we are transforming an enumerable
+      # of key-value-like pairs that isn't actually a hash, e.g.:
+      # `[[k1, v1], [k2, v2], ...]`
+      #
+      # This cop should only be enabled on Ruby version 2.4 or newer
+      # (`transform_values` was added in Ruby 2.4.)
+      #
+      # @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] }
+      #
+      #   # good
+      #   {a: 1, b: 2}.transform_values { |v| foo(v) }
+      #   {a: 1, b: 2}.transform_values { |v| v * v }
+      class HashTransformValues < Cop
+        extend TargetRubyVersion
+        include HashTransformMethod
+
+        minimum_target_ruby_version 2.4
+
+        def_node_matcher :on_bad_each_with_object, <<~PATTERN
+          (block
+            ({send csend} !(send _ :each_with_index) :each_with_object (hash))
+            (args
+              (mlhs
+                (arg _key)
+                (arg $_))
+              (arg _memo))
+            ({send csend} (lvar _memo) :[]= $(lvar _key) $_))
+        PATTERN
+
+        def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
+          (send
+            (const _ :Hash)
+            :[]
+            (block
+              ({send csend} !(send _ :each_with_index) {:map :collect})
+              (args
+                (arg _key)
+                (arg $_))
+              (array $(lvar _key) $_)))
+        PATTERN
+
+        def_node_matcher :on_bad_map_to_h, <<~PATTERN
+          ({send csend}
+            (block
+              ({send csend} !(send _ :each_with_index) {:map :collect})
+              (args
+                (arg _key)
+                (arg $_))
+              (array $(lvar _key) $_))
+            :to_h)
+        PATTERN
+
+        private
+
+        def extract_captures(match)
+          val_argname, key_body_expr, val_body_expr = *match
+          Captures.new(val_argname, val_body_expr, key_body_expr)
+        end
+
+        def new_method_name
+          'transform_values'
+        end
+      end
+    end
+  end
+end

manual/cops.md

@@ -358,6 +358,8 @@ In the following section you find all available cops:
 * [Style/GlobalVars](cops_style.md#styleglobalvars)
 * [Style/GuardClause](cops_style.md#styleguardclause)
 * [Style/HashSyntax](cops_style.md#stylehashsyntax)
+* [Style/HashTransformKeys](cops_style.md#stylehashtransformkeys)
+* [Style/HashTransformValues](cops_style.md#stylehashtransformvalues)
 * [Style/IdenticalConditionalBranches](cops_style.md#styleidenticalconditionalbranches)
 * [Style/IfInsideElse](cops_style.md#styleifinsideelse)
 * [Style/IfUnlessModifier](cops_style.md#styleifunlessmodifier)