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

False auto-correction in Lint/BlockAlignment #1573

Closed
lumeet opened this issue Jan 16, 2015 · 4 comments
Closed

False auto-correction in Lint/BlockAlignment #1573

lumeet opened this issue Jan 16, 2015 · 4 comments

Comments

@lumeet
Copy link
Contributor

lumeet commented Jan 16, 2015

This kind of code correcly triggers an offense but no auto-correction is applied, even though "[Corrected]" is reported.

a = test do
      do_something
    end
@lumeet
Copy link
Contributor Author

lumeet commented Jan 16, 2015

I was about to fix this but then stopped to wonder what exactly is the correct behavior. The style guide doesn't seem to cover this specific case. On the other hand, the specs imply that this actually should be valid, too. So, either there should be no offense in the first place or auto-correction should actually remove the superfluous whitespace.

@jonas054
Copy link
Collaborator

The BlockAlignment is not clearly described in its comment header or in the specs, and it contains at least one bug, as you have discovered. Here's how it's supposed to work (I think):

  • For assignments, end should be aligned with the left-hand-side.
  • For other expressions, end should be aligned with the expression taking the block.
  • It's always OK to align end with the start of the line where do is.

If you want to tackle this, feel free to do so @lumeet. Otherwise I'll do it.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 7, 2015

@lumeet why did you close this?

@lumeet
Copy link
Contributor Author

lumeet commented Mar 7, 2015

@bbatsov Isn't it completely solved by #1576? I thought it was so obvious that I didn't bother commenting. I've now learned to use the correct PR header. :)

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

3 participants