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

Incorrect autocorrection by Style/CollectionCompact #9023

Closed
wendelscardua opened this issue Nov 10, 2020 · 1 comment · Fixed by #9024
Closed

Incorrect autocorrection by Style/CollectionCompact #9023

wendelscardua opened this issue Nov 10, 2020 · 1 comment · Fixed by #9024

Comments

@wendelscardua
Copy link

The new Style/CollectionCompact cop may change some code behavior. It assumes that reject can be replaced with compact even when it shouldn't. Probably it is applying to an Array#reject a correction that would only be valid for a Hash#reject. See below for an example.


Expected behavior

When a reject method is called for an array with a block with two arguments, and this block only checks if the second argument is nil, this reject shouldn't be replaced with a compact.

Actual behavior

reject gets replaced with compact in that scenario.

Steps to reproduce the problem

  • Create this ruby file (lets call it test.rb):
x = [[1, nil], [2, 3], [4, nil]]
x = x.reject { |_a, b| b.nil? }
puts x.size
  • Run it
$ ruby test.rb
1
  • Autocorrect this file with rubocop, selecting only the Style/CollectionCompact cop:
$ bundle exec rubocop --only Style/CollectionCompact test.rb -a
(some lines about new cops not enabled omited)
Inspecting 1 file
Scanning /home/wendel/projects/voltera-api/test.rb
C

Offenses:

test.rb:2:7: C: [Corrected] Style/CollectionCompact: Use compact instead of reject { |_x, y| y.nil? }.
x = x.reject { |_x, y| y.nil? }
      ^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected
  • File content is now:
x = [[1, 2], [3, nil], [4, nil]]
x = x.compact
puts x.size
  • Run the modified script:
$ ruby test.rb
3
  • The output is now 3 instead of 1

RuboCop version

$ bundle exec rubocop -V
1.2.0 (using Parser 2.7.2.0, rubocop-ast 1.1.1, running on ruby 2.7.1 x86_64-linux)
  - rubocop-performance 1.8.1
  - rubocop-rails 2.8.1
  - rubocop-rake 0.5.1
koic added a commit to koic/rubocop that referenced this issue Nov 11, 2020
Fixes rubocop#9023.

This PR marks unsafe for `Style/CollectionCompact` by default because
false positives may occur in the nil check of block arguments to the
receiver object.
For example, `[[1, 2], [3, nil]].reject { |first, second| second.nil? }`
and `[[1, 2], [3, nil]].compact` are not compatible. This will work fine
when the receiver is a hash object.
@koic
Copy link
Member

koic commented Nov 11, 2020

Thank you for detailed feedback. I think this is difficult to detect without false positives and I opened a PR #9024.

koic added a commit that referenced this issue Nov 11, 2020
[Fix #9023] Mark unsafe for `Style/CollectionCompact`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants