-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix code terminated check with heredoc and backtick #390
Fix code terminated check with heredoc and backtick #390
Conversation
9dbe39f
to
6bf06df
Compare
6bf06df
to
6af6f5d
Compare
6af6f5d
to
c67a079
Compare
c67a079
to
b33dc30
Compare
end | ||
i += 1 | ||
end | ||
start_token.last.nil? ? nil : start_token.last | ||
pending_heredocs.first || start_token.last |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests seem to pass without pending_heredocs.first ||
. Would you mind adding another test to cover this scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these test cases.
['<<A;<<B', "<<A;<<B\n", "%W[\#{<<A;<<B", "%W[\#{<<A;<<B\n"]`
process_string_literal
of these code should all be <<A
.
without pending_heredocs.first ||
, it will be
[nil, '<<A', '%W[', '<<A']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thx
Co-authored-by: Stan Lo <stan001212@gmail.com>
Co-authored-by: Stan Lo <stan001212@gmail.com>
@k0kubun would you mind giving it a look too? thx |
…l_heredoc_backtick
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thank you!
…irb#390) * Fix backtick method def method call handled as backtick open * Fix handling heredoc in check_string_literal * Sort result of lexer.parse by pos in ruby<2.7. It's not sorted when the given code includes heredoc. * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Add check_string_literal test for heredoc code that does not end with newline ruby/irb@44bc712460 Co-authored-by: Stan Lo <stan001212@gmail.com>
* Fix backtick method def method call handled as backtick open * Fix handling heredoc in check_string_literal * Sort result of lexer.parse by pos in ruby<2.7. It's not sorted when the given code includes heredoc. * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Add check_string_literal test for heredoc code that does not end with newline Co-authored-by: Stan Lo <stan001212@gmail.com>
…irb#390) * Fix backtick method def method call handled as backtick open * Fix handling heredoc in check_string_literal * Sort result of lexer.parse by pos in ruby<2.7. It's not sorted when the given code includes heredoc. * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Update lib/irb/ruby-lex.rb Co-authored-by: Stan Lo <stan001212@gmail.com> * Add check_string_literal test for heredoc code that does not end with newline ruby/irb@44bc712460 Co-authored-by: Stan Lo <stan001212@gmail.com>
Fixes #361 and also fixes similar problem with
def `();end
self.`('sleep 1')
(#334)Heredoc
This code shows a sample of finding heredoc_beg corresponding to heredoc_end.
In line 1, there are heredocs(A,B,C,D). These heredocs will start from the next line. (pushed into
pending_heredoc
)Heredoc will be closed in an order
A → B → C → D
, so it should be reversed and pushed to start_token when reached a new line.Backtick
Checks if backtick is
`command`
or not.lexer.parse.sort_by(&:pos)
Test that I added failed in ruby 2.5.0 and 2.6.0 without the third commit.
Result of
Ripper::Lexer.new("<<A\nA\n").parse
(used in < 2.7.0) is not ordered by position.