Elixir: Code Formatter inconsistently changes indentation inside heredoc

[rrrene]

Environment

• Elixir & Erlang versions (elixir --version):

Erlang/OTP 20 [erts-9.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false]

Elixir 1.6.0-rc.0 (182c730) (compiled with OTP 19)

• Operating system:

MacOS 10.12.6

Current behavior

When I create a file with a weirdly indented heredoc, like this

# test.ex
defmodule SampleModule do
  @my_module_attribute ~S'''
def fun() do
  42
end
  '''
end

and I use the formatter

mix format --check-equivalent test.ex

it does change the heredoc's content

# test.ex
defmodule SampleModule do
  @my_module_attribute ~S'''
  def fun() do
  42
  end
  '''
end

(note how the line containing 42 is not indented any more to the right)

Expected behavior

It should either indent the heredoc all at once

# test.ex
defmodule SampleModule do
  @my_module_attribute ~S'''
  def fun() do
    42
  end
  '''
end

... or leave the indentation as is.

Thx for taking the time to review this (and for the formatter in the first place!) 👏

[josevalim]

The formatter is actually correct. :) We use the position of the trailing """ to get the heredoc indentation. The final string in your case is:

def fun() do
42
end

You can verify this by printing the attribute right after. We should probably warn if this is the case.

[davydog187]

@josevalim I don't think @rrrene is arguing that the left-most position is incorrect. The problem is that it uniformly indents all lines to the same indentation level. The 42 should be indented once more, respecting the original offset.

[josevalim]

The formatter is correct and it is keeping the code semantics. If the formatter kept the proposed indentation, the formatter would output the wrong string. Use IO.puts on the heredoc value and you will see what I mean.

[rrrene]

Actually, @davydog187 is correct: I assumed that the content of the heredoc should be kept.

In the first case shown above, it is:

def fun() do
  42
end

and after applying the formatter it becomes:

def fun() do
42
end

So it actually changes the content of the heredoc, which is unfortunate for white-space-sensitive documents.

@josevalim I understand your argument that the closing ''' is the technical reference point for the heredoc's indentation. Still, the formatter unfortunately changes working code here. The code behaves differently after formatting and --check-equivalent does not catch this difference 😢

I am not saying this is a major bug. It just took me a while to realize that I did not break my test but the formatter did, so I wanted to report it. :+1:

[rrrene]

@josevalim Okay, never mind. I tested it the way you proposed and you are right. Not sure where I went wrong before.

Sorry for the inconvienince and thx for doing a great job!

[josevalim]

Folks, that’s what I am saying: the code does not behave differently after
formatting. Your expectation of what the string looks like does not match
the reality. The string you see after formatting is the string that
actually exists in practice. There is definitely a bug or at least area for
improvement, but it is not a formatter one.

EDIT: oops, I saw your later response just now @rrrene. :)

[OvermindDL1]

_/me is thinking that heredocs with lines without sufficient indentation should be a compile error..._

[josevalim]

@OvermindDL1 that's what I am thinking but we need to start at least with a warning since it is valid behaviour today.