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

Autocorrect for Rails/Presence corrupts code when there's an elsif block #5479

Closed
fwininger opened this issue Jan 17, 2018 · 2 comments
Closed

Comments

@fwininger
Copy link
Contributor

Expected behavior

I would expect rubocop to leave this code unchanged

Actual behavior

The autocorrect is corrupting the surrounding code, by misplacing the elsif clause

Steps to reproduce the problem

$ cat test.rb
class TestModel < ActiveRecord::Base
  def testcase
    if attr1.present?
      attr1
    elsif attr2.present?
      attr2
    end
  end
end
rubocop test.rb --only "Rails/Presence"
Inspecting 1 file
C

Offenses:

test.rb:5:5: C: Rails/Presence: Use attr2.presence instead of elsif attr2.present?
      attr2.
    elsif attr2.present? ...
    ^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
$ cat test.rb
class TestModel < ActiveRecord::Base
  def testcase
    if attr1.present?
      attr1
    attr2.presence
    end
  end
end

RuboCop version

$ rubocop -V
0.52.1 (using Parser 2.4.0.2, running on ruby 2.5.0 x86_64-linux)
@hezanathos
Copy link

hezanathos commented Jan 18, 2018

This is a real problem. This is happening in tons of projects I guess !

wata727 added a commit to wata727/rubocop that referenced this issue Jan 18, 2018
Fixes rubocop#5479

`elsif` and` unless` are treated as `if` node. However, if using
`present?` with `elsif`, this cop corrupts code by auto-correction.

So change it to ignore the `elsif` node.
bbatsov pushed a commit that referenced this issue Jan 20, 2018
Fixes #5479

`elsif` and` unless` are treated as `if` node. However, if using
`present?` with `elsif`, this cop corrupts code by auto-correction.

So change it to ignore the `elsif` node.
@fwininger
Copy link
Contributor Author

thanks @bbatsov and @wata727 !

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

No branches or pull requests

2 participants