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

Potential invalid offence with Lint/EndAlignment on case statements. #2317

Closed
savef opened this issue Oct 13, 2015 · 4 comments
Closed

Potential invalid offence with Lint/EndAlignment on case statements. #2317

savef opened this issue Oct 13, 2015 · 4 comments

Comments

@savef
Copy link
Contributor

savef commented Oct 13, 2015

I'm getting a Lint/EndAlignment offence with the following code:

process case value
when 1 then 'one'
when 2 then 'two'
end

I would expect that using AlignWith: variable with this cop it would treat method calls as just as valid as variable assignments. Supposing you don't agree with that there should be another switch for this cop to make it not handle case statements.

I'm using v0.34.2. I think this offence will only have been present since PR #2157. Note that I also use Style/CaseIndentation: IndentWhenRelativeTo: end.

Thanks.

@lumeet
Copy link
Contributor

lumeet commented Oct 20, 2015

I agree, the cop shouldn't complain about this. PR submitted.

@savef
Copy link
Contributor Author

savef commented Oct 20, 2015

Nice, thanks!

@jonas054
Copy link
Collaborator

What about

return case value
when 1 then 'one'
when 2 then 'two'
end

Should that also be included in AlignWith: variable? Or would it be better to introduce AlignWith: start_of_line that enforces alignment with the beginning of the line where the keyword is?

@lumeet
Copy link
Contributor

lumeet commented Oct 21, 2015

I guess this is trickier than I first thought. I don't really know what to do with even harder scenarios...

return process case value
when 1 then 'one'
end,
case other
when 2 then 'two'
end

A new style with such a subtle difference sounds a bit of an overkill to me, too. One possible solution is to ignore these cases altogether by Lint/EndAlignment... and perhaps introduce a new cop with an option to discourage the use of case this way. :)

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