Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Prism as a Ruby parser #446

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t prism_spec effectively just a duplicate of rspec task when executed from non-prism CI jobs, @koic ? Same in rubocop-rails.
The suggestion is to run the default rake task in the prism ci job, and remove prism_spec. Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally, efforts are made to ensure that prism_spec passes locally. If it becomes too troublesome, it might be removed, but currently, having prism_spec operational is more convenient. So, I'd like to quickly identify issues related to Prism. Cases where tests cannot pass with Prism can be handled with unsupported_on: :prism or broken_on: :prism.


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 rubocop-performance.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ Gem::Specification.new do |s|
}

s.add_runtime_dependency('rubocop', '>= 1.48.1', '< 2.0')
s.add_runtime_dependency('rubocop-ast', '>= 1.30.0', '< 2.0')
s.add_runtime_dependency('rubocop-ast', '>= 1.31.1', '< 2.0')
end
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