Swap squiggly heredoc tokens when ripper lexer emits out of sequence

[mjago]

Fix for #6 where ripper emits tokens out of sequence.

i.e.: ruby -rripper -rpp -e 'pp Ripper.lex("<<~EOF\n \#{1}\#{2}\nEOF")'
gives:

 [[1, 6], :on_nl, "\n"],
 [[2, 1], :on_embexpr_beg, "\#{"],
 [[2, 1], :on_tstring_content, ""],
 [[2, 3], :on_int, "1"],
 [[2, 4], :on_embexpr_end, "}"],
 [[2, 5], :on_embexpr_beg, "\#{"],
 [[2, 7], :on_int, "2"],
 [[2, 8], :on_embexpr_end, "}"],
 [[2, 9], :on_tstring_content, "\n"],
 [[3, 0], :on_heredoc_end, "EOF"]]```

[bessey]

Could you elaborate on what is out of sequence in that example? I'm afraid I don't follow 😓

[bessey]

Still confused about why the order of tokens is wrong, but at least I understand the nature of the squiggly bug!

[bessey]

Shouldn't expected here be

<<~EOF
#{1} #{2}
EOF

?

[bessey]

Ah, I think I'm beginning to understand the Ripper bug in 2.3. Please correct me if I'm wrong but it seems from experimentation that when Ripper encounters two interpolation expressions with whitespace between them, it removes the [leading whitespace count] whitespace characters from the space between them, as though the 1st expression is immediately followed by a newline.

It's pretty clear in this example:

ruby -rripper -rpp -e 'pp Ripper.lex("<<~EOF\n   \#{1}      \#{2}\nEOF")'
[[[1, 0], :on_heredoc_beg, "<<~EOF"],
 [[1, 6], :on_nl, "\n"],
 [[2, 3], :on_embexpr_beg, "\#{"],
 [[2, 3], :on_tstring_content, ""],
 [[2, 5], :on_int, "1"],
 [[2, 6], :on_embexpr_end, "}"],
 [[2, 10], :on_tstring_content, "   "],
 [[2, 13], :on_embexpr_beg, "\#{"],
 [[2, 15], :on_int, "2"],
 [[2, 16], :on_embexpr_end, "}"],
 [[2, 17], :on_tstring_content, "\n"],
 [[3, 0], :on_heredoc_end, "EOF"]]

Note that the heredoc is indented by 3 spaces, and there are 6 spaces between expression 1 and 2, but in Rippers output, there are only 3 spaces between expression 1 and 2.

[mjago]

Yes - the sexp doesn't help either:

 [[:string_literal,
   [:string_content,
    [:@tstring_content, "", [2, 3]],
    [:string_embexpr, [[:@int, "1", [2, 5]]]],
    [:@tstring_content, "   ", [2, 10]],
    [:string_embexpr, [[:@int, "2", [2, 15]]]],
    [:@tstring_content, "\n", [2, 17]]]]]]``` 

[mjago]

It turns out fixes have been backported to v3.5, v4. 2 and v5-dev and I now have a fix over those versions (early versions are unfixable). I'll tidy up later.

[bessey]

(For others benefit, these refer to Ruby 2.X versions, as in, Ruby 2.3.5, 2.4.2 and 2.5.0-dev. Took me a moment!)

Glad to hear they're being backported to things we use in production. So are you saying on 2.3.5 this test would pass?:

#~# ORIGINAL heredoc_squiggly_extra_spaces

<<~EOF
#{1} #{2}
EOF

#~# EXPECTED

<<~EOF
#{1} #{2}
EOF

[mjago]

@bessey thanks for the version clarification 😊 . Yes all latest versions pass.

[mjago]

1. :on_embexpr_beg => :on_tstring_content is out of sequence
2. Yes it should be #{1} #{2} . I didn't write the original spec which has a typo.
   It isn't the first time Rufo has found a bug in Ripper https://bugs.ruby-lang.org/issues/13701

[gingermusketeer]

@mjago Given that this bug causes rufo to change the behaviour of the code and users may not pick it up if they do not check the changes carefully. Do you think we need to alert users to this bug if they are running a version of ruby with this issue?

Perhaps we should raise an exception, warn or skip the file if there is a squiggly heredoc present and they are running an unpatched version of ruby?

[gingermusketeer]

Should we bump the ruby versions in

rufo/.travis.yml

Lines 4 to 8 in 274aeab

	rvm:
	- 2.4.1
	- 2.3.4
	- 2.2.7
	- ruby-head

So that we can move this to the 2.3 test file?

[mjago]

Yes good point @gingermusketeer

[mjago]

@mjago Given that this bug causes rufo to change the behaviour of the code and users may not pick it up if they do not check the changes carefully. Do you think we need to alert users to this bug if they are running a version of ruby with this issue?

Perhaps we should raise an exception, warn or skip the file if there is a squiggly heredoc present and they are running an unpatched version of ruby?

I've opened an issue to discuss - thanks for the heads-up @gingermusketeer #36

[bessey]

Looks great, thanks for finding that bug!