Merge pull request #211 from ruby/schneems/prism-numero-uno-rebased

schneems · web-flow · commit 457c1ebaf195 · 2023-12-05T11:51:20.000-06:00
Add support for the Prism parser
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -43,3 +43,22 @@ jobs:
       - name: test
         run: bin/rake test
         continue-on-error: ${{ matrix.ruby == 'head' }}
+
+  test-disable-prism:
+    needs: ruby-versions
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }}
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4.1.1
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby }}
+          bundler-cache: true
+      - name: test
+        run: SYNTAX_SUGGEST_DISABLE_PRISM=1 bin/rake test
+        continue-on-error: ${{ matrix.ruby == 'head' }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## HEAD (unreleased)
 
+- Support prism parser (https://github.com/ruby/syntax_suggest/pull/208).
 - No longer supports EOL versions of Ruby. (https://github.com/ruby/syntax_suggest/pull/210)
 - Handle Ruby 3.3 new eval source location format (https://github.com/ruby/syntax_suggest/pull/200).
 
diff --git a/Gemfile b/Gemfile
@@ -12,3 +12,4 @@ gem "standard"
 gem "ruby-prof"
 
 gem "benchmark-ips"
+gem "prism"
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -7,33 +7,34 @@ GEM
   remote: https://rubygems.org/
   specs:
     ast (2.4.2)
-    benchmark-ips (2.9.2)
-    diff-lcs (1.4.4)
+    benchmark-ips (2.12.0)
+    diff-lcs (1.5.0)
     json (2.7.0)
     language_server-protocol (3.17.0.3)
     lint_roller (1.1.0)
     parallel (1.23.0)
     parser (3.2.2.4)
       ast (~> 2.4.1)
       racc
+    prism (0.18.0)
     racc (1.7.3)
     rainbow (3.1.1)
     rake (12.3.3)
     regexp_parser (2.8.3)
     rexml (3.2.6)
-    rspec (3.10.0)
-      rspec-core (~> 3.10.0)
-      rspec-expectations (~> 3.10.0)
-      rspec-mocks (~> 3.10.0)
-    rspec-core (3.10.0)
-      rspec-support (~> 3.10.0)
-    rspec-expectations (3.10.0)
+    rspec (3.12.0)
+      rspec-core (~> 3.12.0)
+      rspec-expectations (~> 3.12.0)
+      rspec-mocks (~> 3.12.0)
+    rspec-core (3.12.2)
+      rspec-support (~> 3.12.0)
+    rspec-expectations (3.12.3)
       diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.10.0)
-    rspec-mocks (3.10.0)
+      rspec-support (~> 3.12.0)
+    rspec-mocks (3.12.6)
       diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.10.0)
-    rspec-support (3.10.0)
+      rspec-support (~> 3.12.0)
+    rspec-support (3.12.1)
     rubocop (1.57.2)
       json (~> 2.3)
       language_server-protocol (>= 3.17.0)
@@ -50,9 +51,9 @@ GEM
     rubocop-performance (1.19.1)
       rubocop (>= 1.7.0, < 2.0)
       rubocop-ast (>= 0.4.0)
-    ruby-prof (1.4.3)
+    ruby-prof (1.6.3)
     ruby-progressbar (1.13.0)
-    stackprof (0.2.16)
+    stackprof (0.2.25)
     standard (1.32.1)
       language_server-protocol (~> 3.17.0.2)
       lint_roller (~> 1.0)
@@ -72,6 +73,7 @@ PLATFORMS
 
 DEPENDENCIES
   benchmark-ips
+  prism
   rake (~> 12.0)
   rspec (~> 3.0)
   ruby-prof
@@ -80,4 +82,4 @@ DEPENDENCIES
   syntax_suggest!
 
 BUNDLED WITH
-   2.3.14
+   2.4.21
diff --git a/README.md b/README.md
@@ -122,7 +122,7 @@ Unmatched `(', missing `)' ?
   5  end
 ```
 
-- Any ambiguous or unknown errors will be annotated by the original ripper error output:
+- Any ambiguous or unknown errors will be annotated by the original parser error output:
 
 <!--
 class Dog
@@ -133,7 +133,7 @@ end
 -->
 
 ```
-syntax error, unexpected end-of-input
+Expected an expression after the operator
 
   1  class Dog
   2    def meals_last_month
diff --git a/lib/syntax_suggest/api.rb b/lib/syntax_suggest/api.rb
@@ -5,9 +5,28 @@
 require "tmpdir"
 require "stringio"
 require "pathname"
-require "ripper"
 require "timeout"
 
+# We need Ripper loaded for `Prism.lex_compat` even if we're using Prism
+# for lexing and parsing
+require "ripper"
+
+# Prism is the new parser, replacing Ripper
+#
+# We need to "dual boot" both for now because syntax_suggest
+# supports older rubies that do not ship with syntax suggest.
+#
+# We also need the ability to control loading of this library
+# so we can test that both modes work correctly in CI.
+if (value = ENV["SYNTAX_SUGGEST_DISABLE_PRISM"])
+  warn "Skipping loading prism due to SYNTAX_SUGGEST_DISABLE_PRISM=#{value}"
+else
+  begin
+    require "prism"
+  rescue LoadError
+  end
+end
+
 module SyntaxSuggest
   # Used to indicate a default value that cannot
   # be confused with another input.
@@ -16,6 +35,14 @@ module SyntaxSuggest
   class Error < StandardError; end
   TIMEOUT_DEFAULT = ENV.fetch("SYNTAX_SUGGEST_TIMEOUT", 1).to_i
 
+  # SyntaxSuggest.use_prism_parser? [Private]
+  #
+  # Tells us if the prism parser is available for use
+  # or if we should fallback to `Ripper`
+  def self.use_prism_parser?
+    defined?(Prism)
+  end
+
   # SyntaxSuggest.handle_error [Public]
   #
   # Takes a `SyntaxError` exception, uses the
@@ -129,11 +156,20 @@ def self.valid_without?(without_lines:, code_lines:)
   # SyntaxSuggest.invalid? [Private]
   #
   # Opposite of `SyntaxSuggest.valid?`
-  def self.invalid?(source)
-    source = source.join if source.is_a?(Array)
-    source = source.to_s
+  if defined?(Prism)
+    def self.invalid?(source)
+      source = source.join if source.is_a?(Array)
+      source = source.to_s
 
-    Ripper.new(source).tap(&:parse).error?
+      Prism.parse(source).failure?
+    end
+  else
+    def self.invalid?(source)
+      source = source.join if source.is_a?(Array)
+      source = source.to_s
+
+      Ripper.new(source).tap(&:parse).error?
+    end
   end
 
   # SyntaxSuggest.valid? [Private]
@@ -191,7 +227,6 @@ def self.valid?(source)
 require_relative "code_line"
 require_relative "code_block"
 require_relative "block_expand"
-require_relative "ripper_errors"
 require_relative "priority_queue"
 require_relative "unvisited_lines"
 require_relative "around_block_scan"
diff --git a/lib/syntax_suggest/clean_document.rb b/lib/syntax_suggest/clean_document.rb
@@ -47,9 +47,9 @@ module SyntaxSuggest
   # ## Heredocs
   #
   # A heredoc is an way of defining a multi-line string. They can cause many
-  # problems. If left as a single line, Ripper would try to parse the contents
+  # problems. If left as a single line, the parser would try to parse the contents
   # as ruby code rather than as a string. Even without this problem, we still
-  # hit an issue with indentation
+  # hit an issue with indentation:
   #
   #    1 foo = <<~HEREDOC
   #    2  "Be yourself; everyone else is already taken.""
diff --git a/lib/syntax_suggest/code_block.rb b/lib/syntax_suggest/code_block.rb
@@ -81,7 +81,7 @@ def valid?
         # lines then the result cannot be invalid
         #
         # That means there's no reason to re-check all
-        # lines with ripper (which is expensive).
+        # lines with the parser (which is expensive).
         # Benchmark in commit message
         @valid = if lines.all? { |l| l.hidden? || l.empty? }
           true
diff --git a/lib/syntax_suggest/code_line.rb b/lib/syntax_suggest/code_line.rb
@@ -180,12 +180,19 @@ def ignore_newline_not_beg?
     #     EOM
     #     expect(lines.first.trailing_slash?).to eq(true)
     #
-    def trailing_slash?
-      last = @lex.last
-      return false unless last
-      return false unless last.type == :on_sp
+    if SyntaxSuggest.use_prism_parser?
+      def trailing_slash?
+        last = @lex.last
+        last&.type == :on_tstring_end
+      end
+    else
+      def trailing_slash?
+        last = @lex.last
+        return false unless last
+        return false unless last.type == :on_sp
 
-      last.token == TRAILING_SLASH
+        last.token == TRAILING_SLASH
+      end
     end
 
     # Endless method detection
diff --git a/lib/syntax_suggest/explain_syntax.rb b/lib/syntax_suggest/explain_syntax.rb
@@ -2,7 +2,21 @@
 
 require_relative "left_right_lex_count"
 
+if !SyntaxSuggest.use_prism_parser?
+  require_relative "ripper_errors"
+end
+
 module SyntaxSuggest
+  class GetParseErrors
+    def self.errors(source)
+      if SyntaxSuggest.use_prism_parser?
+        Prism.parse(source).errors.map(&:message)
+      else
+        RipperErrors.new(source).call.errors
+      end
+    end
+  end
+
   # Explains syntax errors based on their source
   #
   # example:
@@ -15,8 +29,8 @@ module SyntaxSuggest
   #   # => "Unmatched keyword, missing `end' ?"
   #
   # When the error cannot be determined by lexical counting
-  # then ripper is run against the input and the raw ripper
-  # errors returned.
+  # then the parser is run against the input and the raw
+  # errors are returned.
   #
   # Example:
   #
@@ -91,10 +105,10 @@ def why(miss)
     # Returns an array of syntax error messages
     #
     # If no missing pairs are found it falls back
-    # on the original ripper error messages
+    # on the original error messages
     def errors
       if missing.empty?
-        return RipperErrors.new(@code_lines.map(&:original).join).call.errors
+        return GetParseErrors.errors(@code_lines.map(&:original).join).uniq
       end
 
       missing.map { |miss| why(miss) }
diff --git a/lib/syntax_suggest/lex_all.rb b/lib/syntax_suggest/lex_all.rb
@@ -3,34 +3,53 @@
 module SyntaxSuggest
   # Ripper.lex is not guaranteed to lex the entire source document
   #
-  # lex = LexAll.new(source: source)
-  # lex.each do |value|
-  #   puts value.line
-  # end
+  # This class guarantees the whole document is lex-ed by iteratively
+  # lexing the document where ripper stopped.
+  #
+  # Prism likely doesn't have the same problem. Once ripper support is removed
+  # we can likely reduce the complexity here if not remove the whole concept.
+  #
+  # Example usage:
+  #
+  #   lex = LexAll.new(source: source)
+  #   lex.each do |value|
+  #     puts value.line
+  #   end
   class LexAll
     include Enumerable
 
     def initialize(source:, source_lines: nil)
-      @lex = Ripper::Lexer.new(source, "-", 1).parse.sort_by(&:pos)
-      lineno = @lex.last.pos.first + 1
+      @lex = self.class.lex(source, 1)
+      lineno = @lex.last[0][0] + 1
       source_lines ||= source.lines
       last_lineno = source_lines.length
 
       until lineno >= last_lineno
         lines = source_lines[lineno..]
 
         @lex.concat(
-          Ripper::Lexer.new(lines.join, "-", lineno + 1).parse.sort_by(&:pos)
+          self.class.lex(lines.join, lineno + 1)
         )
-        lineno = @lex.last.pos.first + 1
+
+        lineno = @lex.last[0].first + 1
       end
 
       last_lex = nil
       @lex.map! { |elem|
-        last_lex = LexValue.new(elem.pos.first, elem.event, elem.tok, elem.state, last_lex)
+        last_lex = LexValue.new(elem[0].first, elem[1], elem[2], elem[3], last_lex)
       }
     end
 
+    if SyntaxSuggest.use_prism_parser?
+      def self.lex(source, line_number)
+        Prism.lex_compat(source, line: line_number).value.sort_by { |values| values[0] }
+      end
+    else
+      def self.lex(source, line_number)
+        Ripper::Lexer.new(source, "-", line_number).parse.sort_by(&:pos)
+      end
+    end
+
     def to_a
       @lex
     end
diff --git a/lib/syntax_suggest/ripper_errors.rb b/lib/syntax_suggest/ripper_errors.rb
@@ -1,7 +1,10 @@
 # frozen_string_literal: true
 
 module SyntaxSuggest
-  # Capture parse errors from ripper
+  # Capture parse errors from Ripper
+  #
+  # Prism returns the errors with their messages, but Ripper
+  # does not. To get them we must make a custom subclass.
   #
   # Example:
   #
diff --git a/spec/unit/api_spec.rb b/spec/unit/api_spec.rb
@@ -8,6 +8,12 @@
 
 module SyntaxSuggest
   RSpec.describe "Top level SyntaxSuggest api" do
+    it "doesn't load prism if env var is set" do
+      skip("SYNTAX_SUGGEST_DISABLE_PRISM not set") unless ENV["SYNTAX_SUGGEST_DISABLE_PRISM"]
+
+      expect(SyntaxSuggest.use_prism_parser?).to be_falsey
+    end
+
     it "has a `handle_error` interface" do
       fake_error = Object.new
       def fake_error.message
diff --git a/spec/unit/explain_syntax_spec.rb b/spec/unit/explain_syntax_spec.rb
diff --git a/spec/unit/lex_all_spec.rb b/spec/unit/lex_all_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -12,3 +12,4 @@ gem "standard"`
`12`	`12`	`gem "ruby-prof"`
`13`	`13`
`14`	`14`	`gem "benchmark-ips"`
	`15`	`+gem "prism"`
Original file line number	Diff line number	Diff line change
`@@ -47,9 +47,9 @@ module SyntaxSuggest`
`47`	`47`	`# ## Heredocs`
`48`	`48`	`#`
`49`	`49`	`# A heredoc is an way of defining a multi-line string. They can cause many`
`50`		`- # problems. If left as a single line, Ripper would try to parse the contents`
	`50`	`+ # problems. If left as a single line, the parser would try to parse the contents`
`51`	`51`	`# as ruby code rather than as a string. Even without this problem, we still`
`52`		`- # hit an issue with indentation`
	`52`	`+ # hit an issue with indentation:`
`53`	`53`	`#`
`54`	`54`	`# 1 foo = <<~HEREDOC`
`55`	`55`	`# 2 "Be yourself; everyone else is already taken.""`
Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ def valid?`
`81`	`81`	`# lines then the result cannot be invalid`
`82`	`82`	`#`
`83`	`83`	`# That means there's no reason to re-check all`
`84`		`- # lines with ripper (which is expensive).`
	`84`	`+ # lines with the parser (which is expensive).`
`85`	`85`	`# Benchmark in commit message`
`86`	`86`	`@valid = if lines.all? { \|l\| l.hidden? \|\| l.empty? }`
`87`	`87`	`true`