Skip to content

Commit ab122c4

Browse files
committed
Refactor multi-prism version logic
The reason this logic for different methods branches in the class instead of internally was to be eagerly aggressive about runtime performance. This code is currently only used once for the document where it's invoked ~N times (where N is number of lines): ```ruby module SyntaxSuggest class CleanDocument # ... def join_trailing_slash! trailing_groups = @document.select(&:trailing_slash?).map do |code_line| take_while_including(code_line.index..) { |x| x.trailing_slash? } end join_groups(trailing_groups) self end ``` Since this is not currently a hot-spot I think merging the branches and using a case statement is a reasonable tradeoff and avoids the need to do specific version testing. An alternative idea was presented in #241 of behavior-based testing for branch logic (which I would prefer), however, calling the code triggered requiring a `DelegateClass` when the `syntax_suggest/api` is being required.
1 parent 42a3b8f commit ab122c4

File tree

2 files changed

+10
-14
lines changed

2 files changed

+10
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
## HEAD (unreleased)
22

3-
- Fix: Correctly identify trailing slashes when using Prism > 1.8.0. (https://github.com/ruby/syntax_suggest/pull/241)
3+
- Fix: Correctly identify trailing slashes when using Prism > 1.8.0. (https://github.com/ruby/syntax_suggest/pull/243)
44
- Internal: Add tests to multiple versions of prism
55

66
## 2.0.2

lib/syntax_suggest/code_line.rb

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -180,21 +180,17 @@ def ignore_newline_not_beg?
180180
# EOM
181181
# expect(lines.first.trailing_slash?).to eq(true)
182182
#
183-
if SyntaxSuggest.use_prism_parser? && Prism::VERSION <= "1.8.0"
184-
# Older versions of prism didn't correctly emit on_sp
185-
def trailing_slash?
186-
last = @lex.last
187-
return false unless last
188-
189-
last.type == :on_tstring_end || (last.type == :on_sp && last.token == TRAILING_SLASH)
190-
end
191-
else
192-
def trailing_slash?
193-
last = @lex.last
194-
return false unless last
195-
return false unless last.type == :on_sp
183+
def trailing_slash?
184+
last = @lex.last
196185

186+
# Older versions of prism diverged slightly from Ripper in compatibility mode
187+
case last&.type
188+
when :on_sp
197189
last.token == TRAILING_SLASH
190+
when :on_tstring_end
191+
true
192+
else
193+
false
198194
end
199195
end
200196

0 commit comments

Comments
 (0)