Skip to content
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

Merged
merged 7 commits into from
Oct 18, 2022

Conversation

tompng
Copy link
Member

@tompng tompng commented Aug 7, 2022

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.

<<A+<<B+%[#{<<C+<<D
#{<<E+<<F
#{<<G+<<H
G
H
}
E
F
}
A
B
C
D
}]

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

irb(main):001:0` def `();end
irb(main):002:0` 
irb(main):003:0` self.`('sleep 1')
irb(main):004:0` 
# expect
irb(main):001:0> def `(); end
=> :`
irb(main):002:0> self.`('sleep 1')
=> ""

Checks if backtick is `command` or not.

`sleep 1`         #<Ripper::Lexer::Elem: on_backtick@1:0 BEG token: "`">
self.`('sleep 1') #<Ripper::Lexer::Elem: on_backtick@1:5 ARG token: "`">
def `(arg); end   #<Ripper::Lexer::Elem: on_backtick@1:4 ENDFN token: "`">

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.

[#<struct Ripper::Lexer::Elem pos=[1, 0], event=:on_heredoc_beg, tok="<<A", state=EXPR_BEG>,
 #<struct Ripper::Lexer::Elem pos=[2, 0], event=:on_heredoc_end, tok="A\n", state=EXPR_BEG>,
 #<struct Ripper::Lexer::Elem pos=[1, 3], event=:on_nl, tok="\n", state=EXPR_BEG>]

@tompng tompng marked this pull request as draft August 12, 2022 09:58
@tompng tompng force-pushed the check_string_literal_heredoc_backtick branch 2 times, most recently from 9dbe39f to 6bf06df Compare August 12, 2022 19:47
@tompng tompng marked this pull request as ready for review August 12, 2022 20:00
@tompng tompng marked this pull request as draft August 12, 2022 22:47
@tompng tompng force-pushed the check_string_literal_heredoc_backtick branch from 6bf06df to 6af6f5d Compare August 12, 2022 23:04
@tompng tompng marked this pull request as ready for review August 12, 2022 23:14
@tompng tompng force-pushed the check_string_literal_heredoc_backtick branch from 6af6f5d to c67a079 Compare September 27, 2022 18:10
@tompng tompng force-pushed the check_string_literal_heredoc_backtick branch from c67a079 to b33dc30 Compare October 5, 2022 18:11
lib/irb/ruby-lex.rb Outdated Show resolved Hide resolved
lib/irb/ruby-lex.rb Outdated Show resolved Hide resolved
end
i += 1
end
start_token.last.nil? ? nil : start_token.last
pending_heredocs.first || start_token.last
Copy link
Member

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?

Copy link
Member Author

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']

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thx

tompng and others added 3 commits October 10, 2022 23:31
Co-authored-by: Stan Lo <stan001212@gmail.com>
Co-authored-by: Stan Lo <stan001212@gmail.com>
@st0012
Copy link
Member

st0012 commented Oct 17, 2022

@k0kubun would you mind giving it a look too? thx

Copy link
Member

@k0kubun k0kubun left a 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!

@k0kubun k0kubun merged commit 44bc712 into ruby:master Oct 18, 2022
matzbot pushed a commit to ruby/ruby that referenced this pull request Oct 18, 2022
…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>
@tompng tompng deleted the check_string_literal_heredoc_backtick branch October 20, 2022 09:23
st0012 added a commit to st0012/irb that referenced this pull request Oct 22, 2022
* 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>
tenderlove pushed a commit to Shopify/ruby that referenced this pull request Oct 27, 2022
…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>
@st0012 st0012 added the bug Something isn't working label Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

IRB can't recognize heredoc after words
3 participants