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

EmptyLinesAroundBody & EmptyLinesAroundAccessModifier fight each other #1287

Closed
andreaseger opened this issue Aug 19, 2014 · 1 comment · Fixed by #1318
Closed

EmptyLinesAroundBody & EmptyLinesAroundAccessModifier fight each other #1287

andreaseger opened this issue Aug 19, 2014 · 1 comment · Fixed by #1318

Comments

@andreaseger
Copy link
Contributor

take a simple example like this

class Foo
  def some_method
    "foo"
  end

  private
end

using rubocop 0.25.0 the following command seems to go into an endless loop.

rubocop --auto-correct --only EmptyLinesAroundBody,EmptyLinesAroundAccessModifier 
empty_lines_around_access_modifier_fail.rb

I guess one correction adds a empty line where the other removes it again.

You might say that this AccessModifier is useless anyway and we have a Cop for that named Lint/UselessAccessModifier. The problem is this Cop does not (yet) support auto-correct.

I could not find it on this list on the wiki and to me it seems both possible and save to do auto-correct.

If someone can point me to a good example on how to do this kind of auto-correction or how the auto-correction is applied in general I probably could make a pull request myself.

@jonas054
Copy link
Collaborator

I was about to say that we don't add autocorrect to Lint cops, but a search through the source code revealed that we do sometimes. So I see no reason why we shouldn't in this case.

But I'd also like to see a fix in EmptyLinesAroundAccessModifier because we should not depend on other cops being enabled for proper function.

I am without access to a real computer for a few days, typing this on an iPad, so it's difficult for me to point you to a good example.

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