Formatter counts lines from lexer tokens

[straight-shoota]

When formatting some specific source code, the formatter produces an invalid result that doesn't parse anymore.

crystal tool format with Crystal 1.19.1 produces this invalid diff:

@@ -102,8 +101,8 @@ abstract class Admiral::Command
       type = command.is_a?(TypeDeclaration) ? command.type : type

       SubCommands::SPECS[name] = {
-        type:        type,
-        description: {
+        type: type,
+        descri       ption: {
           name + (short ? ", #{short.id}" : ""),
           description,
         },

I noticed the failure while formatting jwaldrip/admiral.cr@211937b in crystal-lang/test-ecosystem#89

The reproduction is not very concise because it requires certain line number arrangement to trigger the error. It can probably be minimized further.

Reproduction

abstract class Admiral::Command
  private macro inherited
    private struct SubCommands
      SPECS = {} of String => NamedTuple(
      )

      def self.locate(name)
        new(name.to_s).locate
      end

      def self.invoke(name, *args, **params)
        new(name).invoke(*args, **params)
      end

      def initialize(name : ::Admiral::StringValue)
        initialize name.value
      end

      def initialize(@name : String)
      end

      def invoke(*args, **params)
        if sub_command_class = locate
          sub_command_class.run(*args, **params, program_name: @name)
        else
          raise ::Admiral::Error.new("Invalid subcommand: #{@name}.")
        end
      end
    end

    def sub(command, *args, **params)
      SubCommands.invoke(command, *args, **params, parent: self)
    end
  end

    {%
      SubCommands::SPECS[name] = {
        type:        type,
        description: 1,
        short: short,
      }
    %}
end

It looks like some AST nodes have incorrect locations which then throws off the formatter when it aligns the values in the named tuple literal. The alignment processing runs after the actual code is generated and is informed by the reported line numbers.

I noticed a similar issue while implementing #16748 where it was related to skip_space_or_newline.

---

Add a 👍 reaction to issues you find important.

[straight-shoota]

I found a reduction:

macro foo

end

{
  a:        1,
  description: 2,
}

Produces:

macro foo
end

{
  a: 1,
  des          cription: 2,
}

The reason for this bug is in Formatter#format_macro_literal_only which writes the result of a subformat run to the output, but @line advances according to the original lexer locations. If the subformat produces a different number of lines than the original because it removes unnecessary newlines (or adds additional ones), the counting is off.

crystal/src/compiler/crystal/tools/formatter.cr

Lines 1762 to 1768 in f4c2f0a

	write value
	next_macro_token
	until @token.type.macro_end?
	next_macro_token
	end

A fix for this problem already exists in Formatter#format_macro_contents which performs a similar subformat but it makes sure to advance @lines by the number of lines in the subformat output:

crystal/src/compiler/crystal/tools/formatter.cr

Lines 2210 to 2229 in f4c2f0a

	write value
	# No need to append a newline because the formatter value
	# will already have it.
	write_indent
	increment_lines(macro_node_line + value.lines.size + 1 - @line)
	line = @line
	# We have to potentially skip multiple macro literal tokens
	while @token.type.macro_literal?
	next_macro_token
	end
	# Skipping the macro literal tokens might have altered `@line`:
	# restore it to what it was before the macro tokens (we are
	# already accounting for the lines in a different way).
	if @line != line
	increment_lines(line - @line)
	end

So we just need to do the same in #format_macro_literal_only. This should fix the immediate issue.
Potentially, we could merge the implementations.

However, I believe there is actually a more fundamental issue: The formatter should never increment lines based on token locations from the lexer. We can't assuming that the output will have similar line breaks as the input. The formatter's job involves adding and removing line breaks to produce a well-formatted result.