Invalid offense with Lint/EndAlignment on if statements

[lovehandle]

When AlignWith is set to variable, if statements treated as arguments are flagged as offenses.

Example:

arr << if foo
  1
else
  2
end

This appears to have been reported with case statements here: #2317. Can we apply the same treatment to if statements? I'm also wondering if start_of_line wouldn't be a more appropriate style name. Happy to follow up with a commit. Just looking for a 👍 that this is expected behavior.

Miscellaneous:

rubocop -v
0.34.2

[rrosenblum]

I came across this recently as well. Feel free to work on it. If not, I was going to try and knock it out soon.

[rrosenblum]

I take that back, I was thinking of auto-correct not aligning the end in your scenario. I am not too familiar with the AlignWidth styles, but under normal cases, this is an offense.

The correct style that I am thinking of is:

arr << if foo 
         1   
       else
         2   
       end

[rrosenblum]

Having looked at this a bit last night, with AlignWith set to variable this would be a false positive.

[jonas054]

The comment that describes Lint/EndAlignment: AlignWith in config/default.yml says

  # The value `variable` means that in assignments, `end` should be aligned
  # with the start of the variable on the left hand side of `=`. In all other
  # situations, `end` should still be aligned with the keyword.

so this behavior is not a bug. In #2317 we made the variable style apply to method calls in addition to assignments, but only if the argument to the method is a case.

If we keep widening the scope of the variable style (around the corner are return, next, etc.), the very least we should do is to update the comment describing it. The other option is to add a new style, start_of_line like we have for Lint/DefEndAlignment.

[lovehandle]

I find start_of_line to more accurately reflect how I've seen this syntax used in Ruby code. I'd love the option to have RuboCop enforce that style of syntax, and I'd be happy to take a swing at this but I'll wait for a 👍 .

[alexdowad]

So we want to have a start_of_line style for EndAlignment?

[alexdowad]

Sorry, maintainers: can you please confirm whether you agree that start_of_line should be added?

[jonas054]

👍 from me on start_of_line.

[bbatsov]

Yeah, let's go for start_of_line.

[alexdowad]

You want start_of_line, you get start_of_line.

[ianfixes]

For those of you who are here via a google search and are looking to fill in the blank:

# SupportedStylesAlignWith: keyword, variable, start_of_line
Layout/EndAlignment:
  Enabled: true
  EnforcedStyleAlignWith: ________________________

EnforcedStyleAlignWith: keyword

arr << if foo
         1
       else
         2
       end

var1 = if bar
         1
       else
         2
       end

EnforcedStyleAlignWith: variable

arr << if foo
         1
       else
         2
       end

var1 = if bar
  1
else
  2
end

EnforcedStyleAlignWith: start_of_line

arr << if foo
  1
else
  2
end

var1 = if bar
  1
else
  2
end