Style/Next autocorrection breaks indentation in some cases

[deivid-rodriguez]

The following file

def a
  res = nil

  [1, 2, 3].each do |number|
    if number > 2
      if number == 1
        res = "it's one"
      else
        res = "it's not one"
      end
    end
  end

  res
end

gets autocorrected to

def a
  res = nil

  [1, 2, 3].each do |number|
    next unless number > 2
    if number == 1
      res = "it's one"
    else
      res = "it's not one"
          end
  end

  res
end

introducing a Lint/EndAlignment offense.

[lumeet]

I think this has been fixed on master with #2429. #2453 wouldn't break it either.

[deivid-rodriguez]

With both #2429 and #2453 I still get an incorrect autocorrection

def a
  res = nil

  [1, 2, 3].each do |number|
    next unless number > 2
    res = if number == 1
            "it's one"
          else
            "it's not one"
    end
  end

  res
end

[lumeet]

Unless I'm completely wrong, Style/Next is not to be blamed for this. Using the default configuration with Next disabled, I'm getting similar alignment for the end.

[jonas054]

See this comment for an explanation of why the ends don't get corrected. In this case, not correcting end alignment by default is not a good feature, since other cops have misaligned them during auto-correction.

[deivid-rodriguez]

@jonas054 So what would be the best solution here? Maybe allowing autocorrections which leave offenses in the corrected code (with the expectation that they will be corrected by other cops) is not a good idea?

[jonas054]

That's what we usually do, but ends are special -- they might not be auto-corrected, depending on configuration. So if they were correctly aligned to begin with, it would be much better if Style/Next could make sure that they continued to be correctly aligned.

[deivid-rodriguez]

So then we should consider this a bug. Right?

[jonas054]

Yes, I think so.

[alexdowad]

I'm about to push a fix for this one to my open PR.