Skip to content

Commit

Permalink
Support Prism as a Ruby parser
Browse files Browse the repository at this point in the history
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism,
as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600.

## Configuration

By default, analysis is performed using the Parser gem, so the default value
for the newly added `parser_engine` is `parser`:

```ruby
ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser)
```

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 rubocop/rubocop#12600 (comment),
> the plan was to set `parser_engine: prism`.
>
> However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`:
> ruby/prism#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`:

- ruby/prism#2454 has been resolved but not yet released.
- ruby/prism#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+.
  • Loading branch information
koic committed Feb 24, 2024
1 parent fa41405 commit 6de4a65
Show file tree
Hide file tree
Showing 14 changed files with 196 additions and 81 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
18 changes: 18 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -35,5 +52,6 @@ end

task default: %i[
spec
prism_spec
internal_investigation
]
1 change: 1 addition & 0 deletions changelog/new_support_prism.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#273](https://github.com/rubocop/rubocop-ast/pull/273): Support Prism as a Ruby parser (experimental). ([@koic][])
11 changes: 11 additions & 0 deletions docs/modules/ROOT/pages/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
137 changes: 81 additions & 56 deletions lib/rubocop/ast/processed_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,23 @@ class ProcessedSource
INVALID_LEVELS = %i[error fatal].freeze
private_constant :INVALID_LEVELS

PARSER_ENGINES = %i[parser 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)
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)
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.
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand All @@ -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
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
Expand Down
1 change: 1 addition & 0 deletions rubocop-ast.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion spec/rubocop/ast/if_node_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
29 changes: 24 additions & 5 deletions spec/rubocop/ast/processed_source_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand All @@ -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 }
Expand Down
6 changes: 2 additions & 4 deletions spec/rubocop/ast/range_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions spec/rubocop/ast/token_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/rubocop/ast/traversal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 6de4a65

Please sign in to comment.