From f333c28f78344aefba06078c5f4f532ec9f3882a Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 22 Aug 2022 15:42:56 +0900 Subject: [PATCH] [Fix #10927] Fix a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues` Fixes #10927. This PR fixes a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues` when not using transformed block argument. --- ...lse_positive_for_style_hash_transform_values.md | 1 + lib/rubocop/cop/mixin/hash_transform_method.rb | 14 +++++++++----- spec/rubocop/cop/style/hash_transform_keys_spec.rb | 12 ++++++++++++ .../cop/style/hash_transform_values_spec.rb | 12 ++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 changelog/fix_a_false_positive_for_style_hash_transform_values.md diff --git a/changelog/fix_a_false_positive_for_style_hash_transform_values.md b/changelog/fix_a_false_positive_for_style_hash_transform_values.md new file mode 100644 index 00000000000..fea41eb5032 --- /dev/null +++ b/changelog/fix_a_false_positive_for_style_hash_transform_values.md @@ -0,0 +1 @@ +* [#10927](https://github.com/rubocop/rubocop/issues/10927): Fix a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues` when not using transformed block argument. ([@koic][]) diff --git a/lib/rubocop/cop/mixin/hash_transform_method.rb b/lib/rubocop/cop/mixin/hash_transform_method.rb index 52e9fecf513..2e6a36c3a12 100644 --- a/lib/rubocop/cop/mixin/hash_transform_method.rb +++ b/lib/rubocop/cop/mixin/hash_transform_method.rb @@ -68,6 +68,8 @@ def handle_possible_offense(node, match, match_desc) # `transform_values` if value transformation uses key. return if captures.transformation_uses_both_args? + return unless captures.use_transformed_argname? + message = "Prefer `#{new_method_name}` over `#{match_desc}`." add_offense(node, message: message) do |corrector| correction = prepare_correction(node) @@ -113,11 +115,7 @@ def execute_correction(corrector, node, correction) end # Internal helper class to hold match data - Captures = Struct.new( - :transformed_argname, - :transforming_body_expr, - :unchanged_body_expr - ) do + 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] @@ -126,6 +124,12 @@ def noop_transformation? def transformation_uses_both_args? transforming_body_expr.descendants.include?(unchanged_body_expr) end + + def use_transformed_argname? + transforming_body_expr.each_descendant(:lvar).any? do |node| + node.source == transformed_argname.to_s + end + end end # Internal helper class to hold autocorrect data diff --git a/spec/rubocop/cop/style/hash_transform_keys_spec.rb b/spec/rubocop/cop/style/hash_transform_keys_spec.rb index 31424f1d6c4..a8a77f2c7c0 100644 --- a/spec/rubocop/cop/style/hash_transform_keys_spec.rb +++ b/spec/rubocop/cop/style/hash_transform_keys_spec.rb @@ -156,6 +156,12 @@ expect_no_offenses('Hash[x.map {|k, v| [k.to_sym, foo(v)]}]') end + it 'does not flag _.map {...}.to_h when key block argument is unused' do + expect_no_offenses(<<~RUBY) + x.map {|_k, v| [v, v]}.to_h + RUBY + 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 @@ -274,6 +280,12 @@ RUBY end + it 'does not flag _.to_h {...} when key block argument is unused' do + expect_no_offenses(<<~RUBY) + x.to_h {|_k, v| [v, v]} + RUBY + end + it 'does not flag `_.to_h{...}` when its receiver is an array literal' do expect_no_offenses(<<~RUBY) [1, 2, 3].to_h { |k, v| [k.to_sym, v] } diff --git a/spec/rubocop/cop/style/hash_transform_values_spec.rb b/spec/rubocop/cop/style/hash_transform_values_spec.rb index 48c5ed0e05b..f43970ad15d 100644 --- a/spec/rubocop/cop/style/hash_transform_values_spec.rb +++ b/spec/rubocop/cop/style/hash_transform_values_spec.rb @@ -156,6 +156,12 @@ expect_no_offenses('Hash[x.map {|k, v| [k.to_sym, foo(v)]}]') end + it 'does not flag _.map {...}.to_h when value block argument is unused' do + expect_no_offenses(<<~RUBY) + x.map {|k, _v| [k, k]}.to_h + RUBY + 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 @@ -260,6 +266,12 @@ RUBY end + it 'does not flag _.to_h {...} when value block argument is unused' do + expect_no_offenses(<<~RUBY) + x.to_h {|k, _v| [k, k]} + RUBY + end + it 'does not flag `_.to_h{...}` when its receiver is an array literal' do expect_no_offenses(<<~RUBY) [1, 2, 3].to_h { |k, v| [k, foo(v)] }