Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong detection and harmful autofix to RuboCop::Cop::Style::HashTransformValues #10927

Closed
Bartuz opened this issue Aug 16, 2022 · 0 comments · Fixed by #10948
Closed

Wrong detection and harmful autofix to RuboCop::Cop::Style::HashTransformValues #10927

Bartuz opened this issue Aug 16, 2022 · 0 comments · Fixed by #10948
Labels

Comments

@Bartuz
Copy link

Bartuz commented Aug 16, 2022

Current docs for HashTransformValues mentions that

# bad
{a: 1, b: 2}.each_with_object({}) { |(k, v), h| h[k] = foo(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) }
{a: 1, b: 2}.transform_values { |v| v * v }

It isn't included, but it would also complain for this code: {a: 1, b: 2}.to_h { |k, _unused_v| [k, k] }
Notice: value of the hash is not used at all. It's just using key

There are two issues with that:

  1. It shouldn't complain when value is not used at all (as it recommends to use transform_values, which in that case is not possible to be used)
  2. Autocorrects to code with NameError: undefined local variable or method 'k' for main:Object

Example code in "Steps to reproduce" section


Expected behavior

Two possible fixes:

  1. Shouldn't detect HashTransformValues rule when .to_h { |k, v| ... } is not using value v.
  2. Shouldn't autofix (-A) to code with NameError (don't correct it for the user, let user fix the issue themselves?)

Actual behavior

Detects offence and autofixes to code which raises NameError.

Steps to reproduce the problem

hash = {
  a: { value: 2 },
  b: { value: 3 }
}

# expected result: { a: :a, b: :b }
# rubocop complains about the line below
hash.to_h { |k, _v| [k, k] }

# rubocop -A output.
# this code causes NameError (undefined k)
hash.transform_values { |_v| k }

# rubocop -A output below
#
# try.rb:7:1: C: [Corrected] Style/HashTransformValues: Prefer transform_values over to_h {...}.
#   hash.to_h { |k, _v| [k, k] }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#  
#
#  1 file inspected, 1 offenses detected, 1 offenses corrected

RuboCop version

rubocop -V
1.35.0 (using Parser 3.1.2.1, rubocop-ast 1.21.0, running on ruby 3.1.0 x86_64-darwin21)
  - rubocop-performance 1.14.2
  - rubocop-rails 2.15.1
@Darhazer Darhazer added the bug label Aug 17, 2022
koic added a commit to koic/rubocop that referenced this issue Aug 22, 2022
…` and `Style/HashTransformValues`

Fixes rubocop#10927.

This PR fixes a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues`
when not using transformed block argument.
bbatsov pushed a commit that referenced this issue Aug 22, 2022
…Style/HashTransformValues`

Fixes #10927.

This PR fixes a false positive for `Style/HashTransformKeys` and `Style/HashTransformValues`
when not using transformed block argument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants