Skip to content

Commit

Permalink
Support Prism as a Ruby parser
Browse files Browse the repository at this point in the history
Follow up rubocop/rubocop-ast#277

In Prism (`Prism::Translation::Parser`), `match-with-lvasgn` can be distinctly differentiated.

It is unclear whether to conform to the current behavior of the Parser gem, but initially,
`def_node_matcher` has been updated to accept the following incompatibilities for
`Performance/EndWith`, `Performance/StringInclude, and `Performance/StartWith` cops
to ensure it works with Prism 0.24.0 as well.

## Parser gem

Returns an `match_with_lvasgn` node:

```console
$ bundle exec ruby -rparser/ruby33 -ve 'p Parser::Ruby33.parse("/foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
s(:match_with_lvasgn,
  s(:regexp,
    s(:str, "foo"),
    s(:regopt)),
  s(:send, nil, :bar))
```

Returns an `match_with_lvasgn` node:

```console
$ bundle exec ruby -rparser/ruby33 -ve 'p Parser::Ruby33.parse("/(?<foo>)foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
s(:match_with_lvasgn,
  s(:regexp,
    s(:str, "(?<foo>)foo"),
    s(:regopt)),
  s(:send, nil, :bar))
```

This lvar-injecting feature appears to have not been supported by Parser gem for a long time:
whitequark/parser#69 (comment)

## Prism

Returns an `send` node:

```console
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("/foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
s(:send,
  s(:regexp,
    s(:str, "foo"),
    s(:regopt)), :=~,
  s(:send, nil, :bar))
```

Returns an `match_with_lvasgn` node:

```console
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("/(?<foo>)foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
s(:match_with_lvasgn,
  s(:regexp,
    s(:str, "(?<foo>)foo"),
    s(:regopt)),
  s(:send, nil, :bar))
```
  • Loading branch information
koic committed Mar 4, 2024
1 parent 13aa3d1 commit ea2bc14
Show file tree
Hide file tree
Showing 20 changed files with 62 additions and 20 deletions.
16 changes: 16 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,22 @@ jobs:
- name: internal_investigation
run: bundle exec rake internal_investigation

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:
PARSER_ENGINE: parser_prism
run: bundle exec rake prism_spec

documentation_checks:
runs-on: ubuntu-latest
name: Check documentation syntax
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gemspec

gem 'bump', require: false
gem 'prism'
gem 'rake'
gem 'rspec'
gem 'rubocop', github: 'rubocop/rubocop'
Expand Down
7 changes: 6 additions & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ task :spec do
end
end

desc 'Run RSpec with Prism'
task :prism_spec do
sh('PARSER_ENGINE=parser_prism bundle exec rake spec')
end

desc 'Run RSpec with code coverage'
task :coverage do
ENV['COVERAGE'] = 'true'
Expand All @@ -36,7 +41,7 @@ end
desc 'Run RuboCop over itself'
RuboCop::RakeTask.new(:internal_investigation)

task default: %i[documentation_syntax_check spec internal_investigation]
task default: %i[documentation_syntax_check spec prism_spec internal_investigation]

desc 'Generate a new cop template'
task :new_cop, [:cop] do |_task, args|
Expand Down
1 change: 1 addition & 0 deletions changelog/new_support_prism.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#446](https://github.com/rubocop/rubocop-performance/pull/446): Support Prism as a Ruby parser (experimental). ([@koic][])
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/end_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class EndWith < Base
def_node_matcher :redundant_regex?, <<~PATTERN
{(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_end?) (regopt)))
(send (regexp (str $#literal_at_end?) (regopt)) {:match :match?} $_)
(match-with-lvasgn (regexp (str $#literal_at_end?) (regopt)) $_)}
({send match-with-lvasgn} (regexp (str $#literal_at_end?) (regopt)) $_)
(send (regexp (str $#literal_at_end?) (regopt)) :=~ $_)}
PATTERN

def on_send(node)
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/start_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class StartWith < Base
def_node_matcher :redundant_regex?, <<~PATTERN
{(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_start?) (regopt)))
(send (regexp (str $#literal_at_start?) (regopt)) {:match :match?} $_)
(match-with-lvasgn (regexp (str $#literal_at_start?) (regopt)) $_)}
(match-with-lvasgn (regexp (str $#literal_at_start?) (regopt)) $_)
(send (regexp (str $#literal_at_start?) (regopt)) :=~ $_)}
PATTERN

def on_send(node)
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/string_include.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class StringInclude < Base
def_node_matcher :redundant_regex?, <<~PATTERN
{(call $!nil? {:match :=~ :!~ :match?} (regexp (str $#literal?) (regopt)))
(send (regexp (str $#literal?) (regopt)) {:match :match? :===} $_)
(match-with-lvasgn (regexp (str $#literal?) (regopt)) $_)}
(match-with-lvasgn (regexp (str $#literal?) (regopt)) $_)
(send (regexp (str $#literal?) (regopt)) :=~ $_)}
PATTERN

# rubocop:disable Metrics/AbcSize
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/bind_call_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::BindCall, :config do
context 'when TargetRubyVersion <= 2.6', :ruby26 do
context 'when TargetRubyVersion <= 2.6', :ruby26, unsupported_on: :prism do
it 'does not register an offense when using `bind(obj).call(args, ...)`' do
expect_no_offenses(<<~RUBY)
umethod.bind(obj).call(foo, bar)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@
RUBY
end

it 'registers an offense when the method is called with no receiver' do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
it 'registers an offense when the method is called with no receiver', broken_on: :prism do
expect_offense(<<~RUBY)
all? do |e|
[1, 2, 3].include?(e)
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/delete_prefix_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
let(:cop_config) { { 'SafeMultiline' => safe_multiline } }
let(:safe_multiline) { true }

context 'when TargetRubyVersion <= 2.4', :ruby24 do
context 'when TargetRubyVersion <= 2.4', :ruby24, unsupported_on: :prism do
it "does not register an offense when using `gsub(/\Aprefix/, '')`" do
expect_no_offenses(<<~RUBY)
str.gsub(/\\Aprefix/, '')
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/delete_suffix_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
let(:cop_config) { { 'SafeMultiline' => safe_multiline } }
let(:safe_multiline) { true }

context 'when TargetRubyVersion <= 2.4', :ruby24 do
context 'when TargetRubyVersion <= 2.4', :ruby24, unsupported_on: :prism do
it "does not register an offense when using `gsub(/suffix\z/, '')`" do
expect_no_offenses(<<~RUBY)
str.gsub(/suffix\\z/, '')
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/map_compact_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@
end
end

context 'when TargetRubyVersion <= 2.6', :ruby26 do
context 'when TargetRubyVersion <= 2.6', :ruby26, unsupported_on: :prism do
it 'does not register an offense when using `collection.map(&:do_something).compact`' do
expect_no_offenses(<<~RUBY)
collection.map(&:do_something).compact
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@
end
end

context 'when TargetRubyVersion <= 2.4', :ruby24 do
context 'when TargetRubyVersion <= 2.4', :ruby24, unsupported_on: :prism do
# Ruby 2.4 does not support `items.all?(Klass)`.
it 'does not register an offense when using `all?` with `is_a?` comparison block' do
expect_no_offenses(<<~RUBY)
Expand Down
4 changes: 3 additions & 1 deletion spec/rubocop/cop/performance/redundant_merge_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
end
end

context 'when receiver is implicit' do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
context 'when receiver is implicit', broken_on: :prism do
it "doesn't autocorrect" do
new_source = autocorrect_source('merge!(foo: 1, bar: 2)')
expect(new_source).to eq('merge!(foo: 1, bar: 2)')
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/regexp_match_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ def foo
RUBY
end

context 'when Ruby <= 2.3', :ruby23 do
context 'when Ruby <= 2.3', :ruby23, unsupported_on: :prism do
it 'does not register an offense when using `String#match` in condition' do
expect_no_offenses(<<~RUBY)
if 'foo'.match(re)
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/select_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
end
end

context 'when TargetRubyVersion <= 2.6', :ruby26 do
context 'when TargetRubyVersion <= 2.6', :ruby26, unsupported_on: :prism do
it 'does not register an offense when using `select.map`' do
expect_no_offenses(<<~RUBY)
ary.select(&:present?).map(&:to_i)
Expand Down
16 changes: 12 additions & 4 deletions spec/rubocop/cop/performance/string_identifier_argument_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
RUBY
end
else
it "registers an offense when using string argument for `#{method}` method" do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
it "registers an offense when using string argument for `#{method}` method", broken_on: :prism do
expect_offense(<<~RUBY, method: method)
#{method}('do_something')
_{method} ^^^^^^^^^^^^^^ Use `:do_something` instead of `'do_something'`.
Expand All @@ -38,14 +40,18 @@
RUBY
end

it "does not register an offense when using symbol argument for `#{method}` method" do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
it "does not register an offense when using symbol argument for `#{method}` method", broken_on: :prism do
expect_no_offenses(<<~RUBY)
#{method}(:do_something)
RUBY
end

if described_class::INTERPOLATION_IGNORE_METHODS.include?(method)
it 'does not register an offense when using string interpolation for `#{method}` method' do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
it 'does not register an offense when using string interpolation for `#{method}` method', broken_on: :prism do
# NOTE: These methods don't support `::` when passing a symbol. const_get('A::B') is valid
# but const_get(:'A::B') isn't. Since interpolated arguments may contain any content these
# cases are not detected as an offense to prevent false positives.
Expand All @@ -54,7 +60,9 @@
RUBY
end
else
it 'registers an offense when using interpolated string argument' do
# FIXME: `undefined method `[]' for nil` occurs Prism 0.24.0. It has been resolved in
# the development line. This will be resolved in Prism > 0.24.0 and higher releases.
it 'registers an offense when using interpolated string argument', broken_on: :prism do
expect_offense(<<~RUBY, method: method)
#{method}("do_something_\#{var}")
_{method} ^^^^^^^^^^^^^^^^^^^^^ Use `:"do_something_\#{var}"` instead of `"do_something_\#{var}"`.
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/sum_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@
RUBY
end

context 'when Ruby 2.3 or lower', :ruby23 do
context 'when Ruby 2.3 or lower', :ruby23, unsupported_on: :prism do
it "does not register an offense when using `array.#{method}(10, :+)`" do
expect_no_offenses(<<~RUBY)
array.#{method}(10, :+)
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cop/performance/unfreeze_string_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
end
end

context 'when Ruby <= 3.2', :ruby32 do
context 'when Ruby <= 3.2', :ruby32, unsupported_on: :prism do
it 'registers an offense and corrects for an empty string with `.dup`' do
expect_offense(<<~RUBY)
"".dup
Expand Down Expand Up @@ -123,7 +123,7 @@
RUBY
end

context 'when Ruby <= 2.2', :ruby22 do
context 'when Ruby <= 2.2', :ruby22, unsupported_on: :prism do
it 'does not register an offense for an empty string with `.dup`' do
expect_no_offenses(<<~RUBY)
"".dup
Expand Down
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

config.shared_context_metadata_behavior = :apply_to_host_groups
config.filter_run_when_matching :focus
config.filter_run_excluding broken_on: :prism if ENV['PARSER_ENGINE'] == 'parser_prism'

# Prism supports Ruby 3.3+ parsing.
config.filter_run_excluding unsupported_on: :prism if ENV['PARSER_ENGINE'] == 'parser_prism'
config.example_status_persistence_file_path = 'spec/examples.txt'
config.disable_monkey_patching!
config.warnings = true
Expand Down

0 comments on commit ea2bc14

Please sign in to comment.