diff --git a/CHANGELOG.md b/CHANGELOG.md index c259d9f562e..dff3a0b2a94 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/default.yml b/config/default.yml index f83c62bd6b9..10fbd0edf67 100644 --- a/config/default.yml +++ b/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 diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 00b5eb195e1..0a5e3f2ebf6 100644 --- a/lib/rubocop.rb +++ b/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' diff --git a/lib/rubocop/cop/mixin/hash_transform_method.rb b/lib/rubocop/cop/mixin/hash_transform_method.rb new file mode 100644 index 00000000000..f1bd0b823c1 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/style/hash_transform_keys.rb b/lib/rubocop/cop/style/hash_transform_keys.rb new file mode 100644 index 00000000000..84369a5847e --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/style/hash_transform_values.rb b/lib/rubocop/cop/style/hash_transform_values.rb new file mode 100644 index 00000000000..9c121d7f7b1 --- /dev/null +++ b/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 diff --git a/manual/cops.md b/manual/cops.md index 28b01cec738..00b162db716 100644 --- a/manual/cops.md +++ b/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) diff --git a/manual/cops_style.md b/manual/cops_style.md index 9821432bcf9..a63cbcef9d9 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -2472,6 +2472,66 @@ PreferHashRocketsForNonAlnumEndingSymbols | `false` | Boolean * [https://rubystyle.guide#hash-literals](https://rubystyle.guide#hash-literals) +## Style/HashTransformKeys + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | No | Yes (Unsafe) | - | - + +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.) + +### Examples + +```ruby +# 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 } +``` + +## Style/HashTransformValues + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | No | Yes (Unsafe) | - | - + +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.) + +### Examples + +```ruby +# 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 } +``` + ## Style/IdenticalConditionalBranches Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/rubocop.gemspec b/rubocop.gemspec index a47393a99d0..ccd7b4fb98a 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -41,4 +41,5 @@ Gem::Specification.new do |s| s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 1.7') s.add_development_dependency('bundler', '>= 1.15.0', '< 3.0') + s.add_development_dependency('pry') end diff --git a/spec/rubocop/cli/cli_options_spec.rb b/spec/rubocop/cli/cli_options_spec.rb index 5dff6be37b4..499c52291d9 100644 --- a/spec/rubocop/cli/cli_options_spec.rb +++ b/spec/rubocop/cli/cli_options_spec.rb @@ -244,15 +244,23 @@ class SomeCop < Cop # process. Otherwise, the extra cop will affect other specs. output = `ruby -I . "#{rubocop}" --require redirect.rb --only Style/SomeCop` - expect(output) - .to eq(<<~RESULT) - The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file: - - Style/SomeCop - Inspecting 2 files - .. - 2 files inspected, no offenses detected - RESULT + expected_prefix = <<~PREFIX + The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file: + PREFIX + expected_suffix = <<~SUFFIX + Inspecting 2 files + .. + + 2 files inspected, no offenses detected + SUFFIX + + expect(output).to start_with(expected_prefix) + expect(output).to end_with(expected_suffix) + + remaining_range = expected_prefix.length..-(expected_suffix.length + 1) + pending_cops = output[remaining_range].split("\n") + expect(pending_cops).to include(' - Style/SomeCop') end context 'without using namespace' do diff --git a/spec/rubocop/cop/style/hash_transform_keys_spec.rb b/spec/rubocop/cop/style/hash_transform_keys_spec.rb new file mode 100644 index 00000000000..61e240e25f6 --- /dev/null +++ b/spec/rubocop/cop/style/hash_transform_keys_spec.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::HashTransformKeys, :config do + subject(:cop) { described_class.new(config) } + + context 'when using Ruby 2.5 or newer', :ruby25 do + context 'with inline block' do + it 'flags each_with_object when transform_keys could be used' do + expect_offense(<<~RUBY) + x.each_with_object({}) {|(k, v), h| h[foo(k)] = v} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `each_with_object`. + RUBY + end + end + + context 'with multiline block' do + it 'flags each_with_object when transform_keys could be used' do + expect_offense(<<~RUBY) + some_hash.each_with_object({}) do |(key, val), memo| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `each_with_object`. + memo[key.to_sym] = val + end + RUBY + end + end + + context 'with safe navigation operator' do + it 'flags each_with_object when transform_keyscould be used' do + expect_offense(<<~RUBY) + x&.each_with_object({}) {|(k, v), h| h[foo(k)] = v} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `each_with_object`. + RUBY + end + end + + it 'does not flag each_with_object when both key & value are transformed' do + expect_no_offenses(<<~RUBY) + x.each_with_object({}) {|(k, v), h| h[k.to_sym] = foo(v)} + RUBY + end + + it 'does not flag each_with_object when key transformation uses value' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[foo(v)] = v}') + end + + it 'does not flag each_with_object when no transformation occurs' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[k] = v}') + end + + it 'does not flag each_with_object when its argument is not modified' do + expect_no_offenses(<<~RUBY) + x.each_with_object({}) {|(k, v), h| other_h[k.to_sym] = v} + RUBY + end + + it 'flags _.map{...}.to_h when transform_keys could be used' do + expect_offense(<<~RUBY) + x.map {|k, v| [k.to_sym, v]}.to_h + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `map {...}.to_h`. + RUBY + end + + it 'does not flag _.map{...}.to_h when both key & value are transformed' do + expect_no_offenses('x.map {|k, v| [k.to_sym, foo(v)]}.to_h') + end + + it 'flags Hash[_.map{...}] when transform_keys could be used' do + expect_offense(<<~RUBY) + Hash[x.map {|k, v| [k.to_sym, v]}] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_keys` over `Hash[_.map {...}]`. + RUBY + end + + it 'does not flag Hash[_.map{...}] when both key & value are transformed' do + expect_no_offenses('Hash[x.map {|k, v| [k.to_sym, foo(v)]}]') + end + + it 'does not flag key transformation in the absence of to_h' do + expect_no_offenses('x.map {|k, v| [k.to_sym, v]}') + end + + it 'correctly autocorrects each_with_object' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.each_with_object({}) do |(k, v), h| + h[k.to_s] = v + end + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_keys do |k| + k.to_s + end + RUBY + end + + it 'correctly autocorrects _.map{...}.to_h without block' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.map do |k, v| + [k.to_s, v] + end.to_h + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_keys do |k| + k.to_s + end + RUBY + end + + it 'correctly autocorrects _.map{...}.to_h with block' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.map {|k, v| [k.to_s, v]}.to_h {|k, v| [v, k]} + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_keys {|k| k.to_s}.to_h {|k, v| [v, k]} + RUBY + end + + it 'correctly autocorrects Hash[_.map{...}]' do + corrected = autocorrect_source(<<~RUBY) + Hash[{a: 1, b: 2}.map do |k, v| + [k.to_s, v] + end] + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_keys do |k| + k.to_s + end + RUBY + end + end + + context 'below Ruby 2.5', :ruby24 do + it 'does not flag even if transform_keys could be used' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[foo(k)] = v}') + end + end +end diff --git a/spec/rubocop/cop/style/hash_transform_values_spec.rb b/spec/rubocop/cop/style/hash_transform_values_spec.rb new file mode 100644 index 00000000000..2c7bdd0b416 --- /dev/null +++ b/spec/rubocop/cop/style/hash_transform_values_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::HashTransformValues, :config do + subject(:cop) { described_class.new(config) } + + context 'when using Ruby 2.4 or newer', :ruby24 do + context 'with inline block' do + it 'flags each_with_object when transform_values could be used' do + expect_offense(<<~RUBY) + x.each_with_object({}) {|(k, v), h| h[k] = foo(v)} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `each_with_object`. + RUBY + end + end + + context 'with multiline block' do + it 'flags each_with_object when transform_values could be used' do + expect_offense(<<~RUBY) + some_hash.each_with_object({}) do |(key, val), memo| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `each_with_object`. + memo[key] = val * val + end + RUBY + end + end + + context 'with safe navigation operator' do + it 'flags each_with_object when transform_values could be used' do + expect_offense(<<~RUBY) + x&.each_with_object({}) {|(k, v), h| h[k] = foo(v)} + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `each_with_object`. + RUBY + end + end + + it 'does not flag each_with_object when both key & value are transformed' do + expect_no_offenses(<<~RUBY) + x.each_with_object({}) {|(k, v), h| h[k.to_sym] = foo(v)} + RUBY + end + + it 'does not flag each_with_object when value transformation uses key' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[k] = k.to_s}') + end + + it 'does not flag each_with_object when no transformation occurs' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[k] = v}') + end + + it 'does not flag each_with_object when its argument is not modified' do + expect_no_offenses(<<~RUBY) + x.each_with_object({}) {|(k, v), h| other_h[k] = v * v} + RUBY + end + + it 'flags _.map {...}.to_h when transform_values could be used' do + expect_offense(<<~RUBY) + x.map {|k, v| [k, foo(v)]}.to_h + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `map {...}.to_h`. + RUBY + end + + it 'does not flag _.map{...}.to_h when both key & value are transformed' do + expect_no_offenses('x.map {|k, v| [k.to_sym, foo(v)]}.to_h') + end + + it 'flags Hash[_.map{...}] when transform_values could be used' do + expect_offense(<<~RUBY) + Hash[x.map {|k, v| [k, foo(v)]}] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `Hash[_.map {...}]`. + RUBY + end + + it 'does not flag Hash[_.map{...}] when both key & value are transformed' do + expect_no_offenses('Hash[x.map {|k, v| [k.to_sym, foo(v)]}]') + end + + it 'does not flag value transformation in the absence of to_h' do + expect_no_offenses('x.map {|k, v| [k, foo(v)]}') + end + + it 'correctly autocorrects each_with_object' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.each_with_object({}) {|(k, v), h| h[k] = foo(v)} + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_values {|v| foo(v)} + RUBY + end + + it 'correctly autocorrects _.map{...}.to_h without block' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.map {|k, v| [k, foo(v)]}.to_h + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_values {|v| foo(v)} + RUBY + end + + it 'correctly autocorrects _.map{...}.to_h with block' do + corrected = autocorrect_source(<<~RUBY) + {a: 1, b: 2}.map {|k, v| [k, foo(v)]}.to_h {|k, v| [v, k]} + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_values {|v| foo(v)}.to_h {|k, v| [v, k]} + RUBY + end + + it 'correctly autocorrects Hash[_.map{...}]' do + corrected = autocorrect_source(<<~RUBY) + Hash[{a: 1, b: 2}.map {|k, v| [k, foo(v)]}] + RUBY + + expect(corrected).to eq(<<~RUBY) + {a: 1, b: 2}.transform_values {|v| foo(v)} + RUBY + end + end + + context 'below Ruby 2.4', :ruby23 do + it 'does not flag even if transform_values could be used' do + expect_no_offenses('x.each_with_object({}) {|(k, v), h| h[k] = foo(v)}') + end + end +end