Skip to content

Commit

Permalink
[Fix #10927] Fix a false positive for Style/HashTransformKeys and `…
Browse files Browse the repository at this point in the history
…Style/HashTransformValues`

Fixes #10927.

This PR fixes a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues`
when not using transformed block argument.
  • Loading branch information
koic authored and bbatsov committed Aug 22, 2022
1 parent bfe8170 commit f333c28
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
@@ -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][])
14 changes: 9 additions & 5 deletions lib/rubocop/cop/mixin/hash_transform_method.rb
Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/style/hash_transform_keys_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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] }
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/style/hash_transform_values_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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)] }
Expand Down

0 comments on commit f333c28

Please sign in to comment.