Formatter fails on string interpolation

[newtonapple]

A simple running Crystal program (bug.cr):

echo 'puts "#{"1"}"' > bug.cr

cat bug.cr
# => puts "#{"1"}"

crystal run bug.cr
# => 1

However, the formatter will fail when it tries to format bug.cr:

crystal tool format bug.cr

Error: couldn't format 'bug.cr', please report a bug including the contents of it: https://github.com/crystal-lang/crystal/issues

expecting }, not `NUMBER, 1`, at :1:10 (Exception)
  from Crystal::Formatter#check<Symbol>:Nil
  from Crystal::Formatter#visit<Crystal::StringLiteral>:Bool
  from Crystal::ASTNode+@Crystal::ASTNode#accept<Crystal::Formatter>:(Bool | Nil)
  from Crystal::Formatter#format_args_simple<Array(Crystal::ASTNode+), Int32, Bool>:Tuple(Bool, Bool, Int32)
  from Crystal::Formatter#format_args<Array(Crystal::ASTNode+), Bool, (Array(Crystal::NamedArgument) | Nil), (Crystal::ASTNode+ | Nil), Int32, Bool>:Tuple(Bool | Nil, Bool, Int32)
  from Crystal::Formatter#visit<Crystal::Call>:Bool
  from Crystal::ASTNode+@Crystal::ASTNode#accept<Crystal::Formatter>:(Bool | Nil)
  from Crystal::format<String, String>:String
  from Crystal::Command#tool:(Bool | Nil)
  from Crystal::Command#run:(Bool | Crystal::Compiler::Result | Nil)
  from main

Crystal version:

Crystal 0.26.0 (2018-08-17)

LLVM: 6.0.1
Default target: x86_64-apple-macosx

[makenowjust]

"#{"1"}" is reduced to simple string literal "1" internally by the compiler. Unfortunately, the formatter does not support this.
I'll try to fix it.

[asterite]

@makenowjust It's ok if you try and succeed. However, I don't think it's really important to fix this, there's no point in interpolating a string literal. This is just an edge-case.

[makenowjust]

@asterite I understand what you said as that you must not add a complex change to fix an edge-case. I'd keep in mind. Thank you.

[newtonapple]

Sure, this is definitely a corner case. It's one of those things that could really trip people up and give them a bad experience for the language. I ran into the problem because I was writing some test code that would be refactored later. The interpolated string was just a placeholder that would be substituted out later. And, having the formatter blowing up or not formatting code on a perfectly valid syntax is just not a very good experience. It took me a good 15-20 minutes to track down the problem. I agree that this is probably not the highest priority bug, but it probably should be fixed at some point.

[asterite]

We should probably fix this by keeping the interpolation instead of just generating a string literal. I'll fix it later today.

[makenowjust]

I implemented two solutions for this:

1. makenowjust@dacbdbd: modify parser to keep interpolation and fix formatter and literal expander.
2. makenowjust@adbf845: fix formatter

The first diff has 101 additions and 74 deletions, but the second diff has 6 additions and 4 deletions.
The second fix is very simpler than the first.

Modifying parser to keep interpolation affects in many places unfortunately, for example macro expansion.
And I'm not sure this implementation is perfect.

@asterite I guess you would fix like the first, however I can't think it is better than the simple solution.

Additional comment by implementer: this is just formatter bug, not related to parser. I'm sure it cannot be fixed without formatter fix, even if the parser keeps interpolations.

We can fix parser to keep interpolation after fixed this. I prefer to apply simple and perfect solution first, then consider to merge complex solution after.

[asterite]

@makenowjust thank you! I vote to keep the simpler fix. Does it work with a string interpolation that has many string literals interpolated, with string content too? For example "foo#{"bar"} Baz #{"qux"} "

[straight-shoota]

I don't think it's really important to fix this, there's no point in interpolating a string literal.

I strongly disagree. It is valid code and the formatter should support all valid code.

[makenowjust]

@asterite Of course, yes. I'll add this to spec then open pull request.