From 9febaba7ddc48b8ca3a751cf00afa055f3d22b3d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 18 Feb 2024 02:55:13 +0900 Subject: [PATCH] Support Prism as a Ruby parser This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing https://github.com/rubocop/rubocop/issues/12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser_whitequark`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_whitequark) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in https://github.com/rubocop/rubocop/issues/12600#issuecomment-1933657732, > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > https://github.com/ruby/prism/pull/2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - https://github.com/ruby/prism/issues/2454 has been resolved but not yet released. - https://github.com/ruby/prism/issues/2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+. --- .github/workflows/rubocop.yml | 17 +++ Rakefile | 18 +++ changelog/new_support_prism.md | 1 + docs/modules/ROOT/pages/index.adoc | 11 ++ lib/rubocop/ast/processed_source.rb | 137 +++++++++++++--------- rubocop-ast.gemspec | 1 + spec/rubocop/ast/if_node_spec.rb | 3 +- spec/rubocop/ast/processed_source_spec.rb | 29 ++++- spec/rubocop/ast/range_node_spec.rb | 6 +- spec/rubocop/ast/token_spec.rb | 8 +- spec/rubocop/ast/traversal_spec.rb | 3 +- spec/rubocop/ast/until_node_spec.rb | 3 +- spec/rubocop/ast/while_node_spec.rb | 3 +- spec/spec_helper.rb | 37 ++++-- 14 files changed, 196 insertions(+), 81 deletions(-) create mode 100644 changelog/new_support_prism.md diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index 96df057eb..9c6c7b04a 100644 --- a/.github/workflows/rubocop.yml +++ b/.github/workflows/rubocop.yml @@ -76,6 +76,23 @@ jobs: - name: spec if: "matrix.coverage != true && matrix.internal_investigation != true" run: bundle exec rake spec + prism: + runs-on: ubuntu-latest + name: Prism + steps: + - uses: actions/checkout@v4 + - name: set up Ruby + uses: ruby/setup-ruby@v1 + with: + # Specify the minimum Ruby version 2.7 required for Prism to run. + ruby-version: 2.7 + bundler-cache: true + - name: spec + env: + # Specify the minimum Ruby version 3.3 required for Prism to analyze. + PARSER_ENGINE: parser_prism + TARGET_RUBY_VERSION: 3.3 + run: bundle exec rake prism_spec rubocop_specs: name: >- Main Gem Specs | RuboCop: ${{ matrix.rubocop }} | ${{ matrix.ruby }} diff --git a/Rakefile b/Rakefile index 5ccb68fce..bf961497f 100644 --- a/Rakefile +++ b/Rakefile @@ -22,6 +22,23 @@ RSpec::Core::RakeTask.new(spec: :generate) do |spec| spec.pattern = FileList['spec/**/*_spec.rb'] end +desc 'Run RSpec code examples with Prism' +task prism_spec: :generate do + original_parser_engine = ENV.fetch('PARSER_ENGINE', nil) + original_target_ruby_version = ENV.fetch('TARGET_RUBY_VERSION', nil) + + RSpec::Core::RakeTask.new(prism_spec: :generate) do |spec| + # Specify the minimum Ruby version 3.3 required for Prism to analyze. + ENV['PARSER_ENGINE'] = 'parser_prism' + ENV['TARGET_RUBY_VERSION'] = '3.3' + + spec.pattern = FileList['spec/**/*_spec.rb'] + end + + ENV['PARSER_ENGINE'] = original_parser_engine + ENV['TARGET_RUBY_VERSION'] = original_target_ruby_version +end + desc 'Run RSpec with code coverage' task :coverage do ENV['COVERAGE'] = 'true' @@ -35,5 +52,6 @@ end task default: %i[ spec + prism_spec internal_investigation ] diff --git a/changelog/new_support_prism.md b/changelog/new_support_prism.md new file mode 100644 index 000000000..2143409c1 --- /dev/null +++ b/changelog/new_support_prism.md @@ -0,0 +1 @@ +* [#277](https://github.com/rubocop/rubocop-ast/pull/277): Support Prism as a Ruby parser (experimental). ([@koic][]) diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc index 506a4246a..d254a86f5 100644 --- a/docs/modules/ROOT/pages/index.adoc +++ b/docs/modules/ROOT/pages/index.adoc @@ -82,3 +82,14 @@ source = RuboCop::AST::ProcessedSource.new(code, 2.7) rule = MyRule.new source.ast.each_node { |n| rule.process(n) } ---- + +In RuboCop AST, you can specify Prism as the parser engine backend by setting `parser_engine: :parser_prism`: + +```ruby +# Using the Parser gem with `parser_engine: parser` is the default. +ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) +``` + +This is an experimental feature. If you encounter any incompatibilities between +Prism and the Parser gem, please check the following URL: +https://github.com/ruby/prism/issues?q=is%3Aissue+is%3Aopen+label%3Arubocop diff --git a/lib/rubocop/ast/processed_source.rb b/lib/rubocop/ast/processed_source.rb index 9a9a5bd29..7f2c09d5b 100644 --- a/lib/rubocop/ast/processed_source.rb +++ b/lib/rubocop/ast/processed_source.rb @@ -15,15 +15,23 @@ class ProcessedSource INVALID_LEVELS = %i[error fatal].freeze private_constant :INVALID_LEVELS + PARSER_ENGINES = %i[parser_whitequark parser_prism].freeze + private_constant :PARSER_ENGINES + attr_reader :path, :buffer, :ast, :comments, :tokens, :diagnostics, - :parser_error, :raw_source, :ruby_version + :parser_error, :raw_source, :ruby_version, :parser_engine - def self.from_file(path, ruby_version) + def self.from_file(path, ruby_version, parser_engine: :parser_whitequark) file = File.read(path, mode: 'rb') - new(file, ruby_version, path) + new(file, ruby_version, path, parser_engine: parser_engine) end - def initialize(source, ruby_version, path = nil) + def initialize(source, ruby_version, path = nil, parser_engine: :parser_whitequark) + unless PARSER_ENGINES.include?(parser_engine) + raise ArgumentError, 'The keyword argument `parser_engine` accepts ' \ + "`parser` or `parser_prism`, but `#{parser_engine}` was passed." + end + # Defaults source encoding to UTF-8, regardless of the encoding it has # been read with, which could be non-utf8 depending on the default # external encoding. @@ -33,9 +41,10 @@ def initialize(source, ruby_version, path = nil) @path = path @diagnostics = [] @ruby_version = ruby_version + @parser_engine = parser_engine @parser_error = nil - parse(source, ruby_version) + parse(source, ruby_version, parser_engine) end def ast_with_comments @@ -193,7 +202,7 @@ def comment_index end end - def parse(source, ruby_version) + def parse(source, ruby_version, parser_engine) buffer_name = @path || STRING_SOURCE_NAME @buffer = Parser::Source::Buffer.new(buffer_name, 1) @@ -207,7 +216,7 @@ def parse(source, ruby_version) return end - @ast, @comments, @tokens = tokenize(create_parser(ruby_version)) + @ast, @comments, @tokens = tokenize(create_parser(ruby_version, parser_engine)) end def tokenize(parser) @@ -227,61 +236,77 @@ def tokenize(parser) end # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength - def parser_class(ruby_version) - case ruby_version - when 1.9 - require 'parser/ruby19' - Parser::Ruby19 - when 2.0 - require 'parser/ruby20' - Parser::Ruby20 - when 2.1 - require 'parser/ruby21' - Parser::Ruby21 - when 2.2 - require 'parser/ruby22' - Parser::Ruby22 - when 2.3 - require 'parser/ruby23' - Parser::Ruby23 - when 2.4 - require 'parser/ruby24' - Parser::Ruby24 - when 2.5 - require 'parser/ruby25' - Parser::Ruby25 - when 2.6 - require 'parser/ruby26' - Parser::Ruby26 - when 2.7 - require 'parser/ruby27' - Parser::Ruby27 - when 2.8, 3.0 - require 'parser/ruby30' - Parser::Ruby30 - when 3.1 - require 'parser/ruby31' - Parser::Ruby31 - when 3.2 - require 'parser/ruby32' - Parser::Ruby32 - when 3.3 - require 'parser/ruby33' - Parser::Ruby33 - when 3.4 - require 'parser/ruby34' - Parser::Ruby34 - else - raise ArgumentError, - "RuboCop found unknown Ruby version: #{ruby_version.inspect}" + def parser_class(ruby_version, parser_engine) + case parser_engine + when :parser_whitequark + case ruby_version + when 1.9 + require 'parser/ruby19' + Parser::Ruby19 + when 2.0 + require 'parser/ruby20' + Parser::Ruby20 + when 2.1 + require 'parser/ruby21' + Parser::Ruby21 + when 2.2 + require 'parser/ruby22' + Parser::Ruby22 + when 2.3 + require 'parser/ruby23' + Parser::Ruby23 + when 2.4 + require 'parser/ruby24' + Parser::Ruby24 + when 2.5 + require 'parser/ruby25' + Parser::Ruby25 + when 2.6 + require 'parser/ruby26' + Parser::Ruby26 + when 2.7 + require 'parser/ruby27' + Parser::Ruby27 + when 2.8, 3.0 + require 'parser/ruby30' + Parser::Ruby30 + when 3.1 + require 'parser/ruby31' + Parser::Ruby31 + when 3.2 + require 'parser/ruby32' + Parser::Ruby32 + when 3.3 + require 'parser/ruby33' + Parser::Ruby33 + when 3.4 + require 'parser/ruby34' + Parser::Ruby34 + else + raise ArgumentError, "RuboCop found unknown Ruby version: #{ruby_version.inspect}" + end + when :parser_prism + require 'prism' + + case ruby_version + when 3.3 + require 'prism/translation/parser33' + Prism::Translation::Parser33 + when 3.4 + require 'prism/translation/parser34' + Prism::Translation::Parser34 + else + raise ArgumentError, 'RuboCop supports target Ruby versions 3.3 and above with Prism. ' \ + "Specified target Ruby version: #{ruby_version.inspect}" + end end end # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength - def create_parser(ruby_version) + def create_parser(ruby_version, parser_engine) builder = RuboCop::AST::Builder.new - parser_class(ruby_version).new(builder).tap do |parser| + parser_class(ruby_version, parser_engine).new(builder).tap do |parser| # On JRuby there's a risk that we hang in tokenize() if we # don't set the all errors as fatal flag. The problem is caused by a bug # in Racc that is discussed in issue #93 of the whitequark/parser diff --git a/rubocop-ast.gemspec b/rubocop-ast.gemspec index 7d79bfd7e..cc3436b67 100644 --- a/rubocop-ast.gemspec +++ b/rubocop-ast.gemspec @@ -34,6 +34,7 @@ Gem::Specification.new do |s| } s.add_runtime_dependency('parser', '>= 3.3.0.4') + s.add_runtime_dependency('prism', '>= 0.24.0') ##### Do NOT add `rubocop` (or anything depending on `rubocop`) here. See Gemfile end diff --git a/spec/rubocop/ast/if_node_spec.rb b/spec/rubocop/ast/if_node_spec.rb index ace33659e..3876f87b5 100644 --- a/spec/rubocop/ast/if_node_spec.rb +++ b/spec/rubocop/ast/if_node_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -RSpec.describe RuboCop::AST::IfNode do +# FIXME: `broken_on: :prism` can be removed when Prism > 0.24.0 will be released. +RSpec.describe RuboCop::AST::IfNode, broken_on: :prism do subject(:if_node) { parse_source(source).ast } describe '.new' do diff --git a/spec/rubocop/ast/processed_source_spec.rb b/spec/rubocop/ast/processed_source_spec.rb index 9c53cbd20..00780b220 100644 --- a/spec/rubocop/ast/processed_source_spec.rb +++ b/spec/rubocop/ast/processed_source_spec.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true RSpec.describe RuboCop::AST::ProcessedSource do - subject(:processed_source) { described_class.new(source, ruby_version, path) } + subject(:processed_source) do + described_class.new(source, ruby_version, path, parser_engine: parser_engine) + end let(:source) { <<~RUBY } # an awesome method @@ -25,6 +27,17 @@ def some_method is_expected.to be_a(described_class) end end + + context 'when using invalid `parser_engine` argument' do + let(:parser_engine) { :unknown_parser_engine } + + it 'raises a Errno::ENOENT when the file does not exist' do + expect { processed_source }.to raise_error(ArgumentError) do |e| + expect(e.message).to eq 'The keyword argument `parser_engine` accepts `parser` or ' \ + '`parser_prism`, but `unknown_parser_engine` was passed.' + end + end + end end describe '.from_file' do @@ -36,7 +49,9 @@ def some_method Dir.chdir(org_pwd) end - let(:processed_source) { described_class.from_file(path, ruby_version) } + let(:processed_source) do + described_class.from_file(path, ruby_version, parser_engine: parser_engine) + end it 'returns an instance of ProcessedSource' do is_expected.to be_a(described_class) @@ -186,7 +201,9 @@ def some_method end end - context 'when the source is valid but has some warning diagnostics' do + # FIXME: `broken_on: :prism` can be removed when + # https://github.com/ruby/prism/issues/2454 will be released. + context 'when the source is valid but has some warning diagnostics', broken_on: :prism do let(:source) { 'do_something *array' } it 'returns true' do @@ -442,7 +459,8 @@ def some_method end # rubocop:enable RSpec/RedundantPredicateMatcher - describe '#preceding_line' do + # FIXME: https://github.com/ruby/prism/issues/2467 + describe '#preceding_line', broken_on: :prism do let(:source) { <<~RUBY } [ line, 1 ] { line: 2 } @@ -458,7 +476,8 @@ def some_method end end - describe '#following_line' do + # FIXME: https://github.com/ruby/prism/issues/2467 + describe '#following_line', broken_on: :prism do let(:source) { <<~RUBY } [ line, 1 ] { line: 2 } diff --git a/spec/rubocop/ast/range_node_spec.rb b/spec/rubocop/ast/range_node_spec.rb index 4f6b8062d..07fcdf899 100644 --- a/spec/rubocop/ast/range_node_spec.rb +++ b/spec/rubocop/ast/range_node_spec.rb @@ -22,8 +22,7 @@ it { is_expected.to be_range_type } end - context 'with an infinite range' do - let(:ruby_version) { 2.6 } + context 'with an infinite range', :ruby26 do let(:source) do '1..' end @@ -32,8 +31,7 @@ it { is_expected.to be_range_type } end - context 'with a beignless range' do - let(:ruby_version) { 2.7 } + context 'with a beignless range', :ruby27 do let(:source) do '..42' end diff --git a/spec/rubocop/ast/token_spec.rb b/spec/rubocop/ast/token_spec.rb index 5b06da910..65d6866b0 100644 --- a/spec/rubocop/ast/token_spec.rb +++ b/spec/rubocop/ast/token_spec.rb @@ -342,7 +342,9 @@ def foo end describe '#left_brace?' do - it 'returns true for left hash brace tokens' do + # FIXME: `broken_on: :prism` can be removed when + # https://github.com/ruby/prism/issues/2454 will be released. + it 'returns true for left hash brace tokens', broken_on: :prism do expect(left_hash_brace_token).to be_left_brace end @@ -357,7 +359,9 @@ def foo expect(left_block_brace_token).to be_left_curly_brace end - it 'returns false for non left block brace tokens' do + # FIXME: `broken_on: :prism` can be removed when + # https://github.com/ruby/prism/issues/2454 will be released. + it 'returns false for non left block brace tokens', broken_on: :prism do expect(left_hash_brace_token).not_to be_left_curly_brace expect(right_block_brace_token).not_to be_left_curly_brace end diff --git a/spec/rubocop/ast/traversal_spec.rb b/spec/rubocop/ast/traversal_spec.rb index 3bdad22de..813ed079f 100644 --- a/spec/rubocop/ast/traversal_spec.rb +++ b/spec/rubocop/ast/traversal_spec.rb @@ -86,7 +86,8 @@ def initialize let(:source) { "foo=bar=baz=nil; #{example}" } - it 'traverses all nodes' do + # FIXME: `broken_on: :prism` can be removed when Prism > 0.24.0 will be released. + it 'traverses all nodes', broken_on: :prism do actual = node.each_node.count expect(traverse.hits).to eql(actual) end diff --git a/spec/rubocop/ast/until_node_spec.rb b/spec/rubocop/ast/until_node_spec.rb index 25099a88d..0fd0e45e0 100644 --- a/spec/rubocop/ast/until_node_spec.rb +++ b/spec/rubocop/ast/until_node_spec.rb @@ -29,7 +29,8 @@ it { expect(until_node.inverse_keyword).to eq('while') } end - describe '#do?' do + # FIXME: `broken_on: :prism` can be removed when Prism > 0.24.0 will be released. + describe '#do?', broken_on: :prism do context 'with a do keyword' do let(:source) { 'until foo do; bar; end' } diff --git a/spec/rubocop/ast/while_node_spec.rb b/spec/rubocop/ast/while_node_spec.rb index 29abb69af..8a8587a55 100644 --- a/spec/rubocop/ast/while_node_spec.rb +++ b/spec/rubocop/ast/while_node_spec.rb @@ -30,7 +30,8 @@ end describe '#do?' do - context 'with a do keyword' do + # FIXME: `broken_on: :prism` can be removed when Prism > 0.24.0 will be released. + context 'with a do keyword', broken_on: :prism do let(:source) { 'while foo do; bar; end' } it { is_expected.to be_do } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b4bb88634..4d27e4a56 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,35 +20,43 @@ end RSpec.shared_context 'ruby 2.3', :ruby23 do - let(:ruby_version) { 2.3 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 2.3 } end RSpec.shared_context 'ruby 2.4', :ruby24 do - let(:ruby_version) { 2.4 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 2.4 } end RSpec.shared_context 'ruby 2.5', :ruby25 do - let(:ruby_version) { 2.5 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 2.5 } end RSpec.shared_context 'ruby 2.6', :ruby26 do - let(:ruby_version) { 2.6 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 2.6 } end RSpec.shared_context 'ruby 2.7', :ruby27 do - let(:ruby_version) { 2.7 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 2.7 } end RSpec.shared_context 'ruby 3.0', :ruby30 do - let(:ruby_version) { 3.0 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 3.0 } end RSpec.shared_context 'ruby 3.1', :ruby31 do - let(:ruby_version) { 3.1 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 3.1 } end RSpec.shared_context 'ruby 3.2', :ruby32 do - let(:ruby_version) { 3.2 } + # Prism supports parsing Ruby 3.3+. + let(:ruby_version) { ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : 3.2 } end RSpec.shared_context 'ruby 3.3', :ruby33 do @@ -63,7 +71,13 @@ module DefaultRubyVersion extend RSpec::SharedContext - let(:ruby_version) { 2.4 } + let(:ruby_version) { ENV.fetch('TARGET_RUBY_VERSION', 2.4).to_f } +end + +module DefaultParserEngine + extend RSpec::SharedContext + + let(:parser_engine) { ENV.fetch('PARSER_ENGINE', :parser_whitequark).to_sym } end module RuboCop @@ -80,7 +94,7 @@ module ParseSourceHelper def parse_source(source) lookup = nil ruby = source.gsub(/>>(.*)< { raise "No node corresponds to source '#{lookup}'" } @@ -95,6 +109,7 @@ def parse_source(source) RSpec.configure do |config| config.include ParseSourceHelper config.include DefaultRubyVersion + config.include DefaultParserEngine config.shared_context_metadata_behavior = :apply_to_host_groups config.filter_run_when_matching :focus @@ -112,6 +127,8 @@ def parse_source(source) mocks.verify_partial_doubles = true end + config.filter_run_excluding broken_on: :prism if ENV['PARSER_ENGINE'] == 'parser_prism' + config.order = :random Kernel.srand config.seed end