From 20d1c51f55d1869a318a51aa64f06eca5c99728f Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 20 Jan 2026 13:34:55 -0600 Subject: [PATCH 1/3] Test multiple prism versions in CI --- .github/workflows/ci.yml | 9 +++++++++ CHANGELOG.md | 2 ++ Gemfile | 10 +++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf371c1..61689a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,12 @@ jobs: fail-fast: false matrix: ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }} + prism_version: + - 1.2.0 # Shipped with Ruby 3.4 as default parser https://www.ruby-lang.org/en/news/2024/12/25/ruby-3-4-0-released/ + - 1.8.0 + - head + env: + PRISM_VERSION: ${{ matrix.prism_version }} steps: - name: Checkout code uses: actions/checkout@v6 @@ -40,6 +46,9 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true + - name: Use latest prism version (head only) + if: matrix.prism_version == 'head' + run: bundle update prism - name: test run: bin/rake test continue-on-error: ${{ matrix.ruby == 'head' }} diff --git a/CHANGELOG.md b/CHANGELOG.md index ee4c4da..e9f765d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## HEAD (unreleased) +- Internal: Add tests to multiple versions of prism + ## 2.0.2 - Fix: Separate multiple parser errors by newline. (https://github.com/ruby/syntax_suggest/pull/232) diff --git a/Gemfile b/Gemfile index 6595e0b..c193614 100644 --- a/Gemfile +++ b/Gemfile @@ -12,4 +12,12 @@ gem "standard" gem "ruby-prof" gem "benchmark-ips" -gem "prism" + +case ENV["PRISM_VERSION"]&.strip&.downcase +when "head" + gem "prism", github: "ruby/prism" +when nil, "" + gem "prism" +else + gem "prism", ENV["PRISM_VERSION"] +end From 42a3b8f6cb7ea5ce8bb97fab9ebddd9bc0ee5362 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 20 Jan 2026 13:38:50 +0100 Subject: [PATCH 2/3] Handle `on_sp` when using prism It used to not emit this token type, but now it does. So when a newer version of prism is present, we can fall back to the same code that ripper uses. Ref: * https://github.com/ruby/ruby/pull/15914 * https://github.com/ruby/prism/pull/3859 --- CHANGELOG.md | 1 + lib/syntax_suggest/code_line.rb | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9f765d..2119304 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## HEAD (unreleased) +- Fix: Correctly identify trailing slashes when using Prism > 1.8.0. (https://github.com/ruby/syntax_suggest/pull/241) - Internal: Add tests to multiple versions of prism ## 2.0.2 diff --git a/lib/syntax_suggest/code_line.rb b/lib/syntax_suggest/code_line.rb index 58197e9..892c273 100644 --- a/lib/syntax_suggest/code_line.rb +++ b/lib/syntax_suggest/code_line.rb @@ -180,10 +180,13 @@ def ignore_newline_not_beg? # EOM # expect(lines.first.trailing_slash?).to eq(true) # - if SyntaxSuggest.use_prism_parser? + if SyntaxSuggest.use_prism_parser? && Prism::VERSION <= "1.8.0" + # Older versions of prism didn't correctly emit on_sp def trailing_slash? last = @lex.last - last&.type == :on_tstring_end + return false unless last + + last.type == :on_tstring_end || (last.type == :on_sp && last.token == TRAILING_SLASH) end else def trailing_slash? From ab122c455f2a417fd7abb6cff604a1ddd747d39d Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 20 Jan 2026 15:32:33 -0600 Subject: [PATCH 3/3] 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. --- CHANGELOG.md | 2 +- lib/syntax_suggest/code_line.rb | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2119304..9302e75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## HEAD (unreleased) -- Fix: Correctly identify trailing slashes when using Prism > 1.8.0. (https://github.com/ruby/syntax_suggest/pull/241) +- Fix: Correctly identify trailing slashes when using Prism > 1.8.0. (https://github.com/ruby/syntax_suggest/pull/243) - Internal: Add tests to multiple versions of prism ## 2.0.2 diff --git a/lib/syntax_suggest/code_line.rb b/lib/syntax_suggest/code_line.rb index 892c273..76ca892 100644 --- a/lib/syntax_suggest/code_line.rb +++ b/lib/syntax_suggest/code_line.rb @@ -180,21 +180,17 @@ def ignore_newline_not_beg? # EOM # expect(lines.first.trailing_slash?).to eq(true) # - if SyntaxSuggest.use_prism_parser? && Prism::VERSION <= "1.8.0" - # Older versions of prism didn't correctly emit on_sp - def trailing_slash? - last = @lex.last - return false unless last - - last.type == :on_tstring_end || (last.type == :on_sp && last.token == TRAILING_SLASH) - end - else - def trailing_slash? - last = @lex.last - return false unless last - return false unless last.type == :on_sp + def trailing_slash? + last = @lex.last + # Older versions of prism diverged slightly from Ripper in compatibility mode + case last&.type + when :on_sp last.token == TRAILING_SLASH + when :on_tstring_end + true + else + false end end