From 2059d28789c5058d5610b81f6f76772a162798c9 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 3 Mar 2024 01:33:00 +0900 Subject: [PATCH] Support Prism as a Ruby parser Follow up https://github.com/rubocop/rubocop-ast/pull/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/ =~ 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)) ``` This lvar-injecting feature appears to have not been supported by Parser gem for a long time: https://github.com/whitequark/parser/issues/69#issuecomment-19506391 ## 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/ =~ 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)) ``` --- .github/workflows/test.yml | 16 ++++++++++++++++ Gemfile | 1 + Rakefile | 7 ++++++- changelog/new_support_prism.md | 1 + lib/rubocop/cop/performance/end_with.rb | 3 ++- lib/rubocop/cop/performance/start_with.rb | 3 ++- lib/rubocop/cop/performance/string_include.rb | 3 ++- rubocop-performance.gemspec | 2 +- spec/rubocop/cop/performance/bind_call_spec.rb | 2 +- .../collection_literal_in_loop_spec.rb | 4 +++- .../cop/performance/delete_prefix_spec.rb | 2 +- .../cop/performance/delete_suffix_spec.rb | 2 +- spec/rubocop/cop/performance/map_compact_spec.rb | 2 +- .../redundant_equality_comparison_block_spec.rb | 2 +- .../cop/performance/redundant_merge_spec.rb | 4 +++- .../rubocop/cop/performance/regexp_match_spec.rb | 2 +- spec/rubocop/cop/performance/select_map_spec.rb | 2 +- .../string_identifier_argument_spec.rb | 16 ++++++++++++---- spec/rubocop/cop/performance/sum_spec.rb | 2 +- .../cop/performance/unfreeze_string_spec.rb | 4 ++-- spec/spec_helper.rb | 4 ++++ 21 files changed, 63 insertions(+), 21 deletions(-) create mode 100644 changelog/new_support_prism.md diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4f25afefe4..2f57347241 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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 diff --git a/Gemfile b/Gemfile index 4ca0a6f351..4d09d0a626 100644 --- a/Gemfile +++ b/Gemfile @@ -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' diff --git a/Rakefile b/Rakefile index 7e0737b0d6..efb05e7e34 100644 --- a/Rakefile +++ b/Rakefile @@ -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' @@ -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| diff --git a/changelog/new_support_prism.md b/changelog/new_support_prism.md new file mode 100644 index 0000000000..d07be6aae1 --- /dev/null +++ b/changelog/new_support_prism.md @@ -0,0 +1 @@ +* [#446](https://github.com/rubocop/rubocop-performance/pull/446): Support Prism as a Ruby parser (experimental). ([@koic][]) diff --git a/lib/rubocop/cop/performance/end_with.rb b/lib/rubocop/cop/performance/end_with.rb index a14cf6df94..ab2e516be7 100644 --- a/lib/rubocop/cop/performance/end_with.rb +++ b/lib/rubocop/cop/performance/end_with.rb @@ -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) diff --git a/lib/rubocop/cop/performance/start_with.rb b/lib/rubocop/cop/performance/start_with.rb index dd7b636d07..6a3604a597 100644 --- a/lib/rubocop/cop/performance/start_with.rb +++ b/lib/rubocop/cop/performance/start_with.rb @@ -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) diff --git a/lib/rubocop/cop/performance/string_include.rb b/lib/rubocop/cop/performance/string_include.rb index c90bd8589b..cba907329e 100644 --- a/lib/rubocop/cop/performance/string_include.rb +++ b/lib/rubocop/cop/performance/string_include.rb @@ -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 diff --git a/rubocop-performance.gemspec b/rubocop-performance.gemspec index 2ac1768b58..495170928b 100644 --- a/rubocop-performance.gemspec +++ b/rubocop-performance.gemspec @@ -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 diff --git a/spec/rubocop/cop/performance/bind_call_spec.rb b/spec/rubocop/cop/performance/bind_call_spec.rb index 49c2bfb334..82f85bafb6 100644 --- a/spec/rubocop/cop/performance/bind_call_spec.rb +++ b/spec/rubocop/cop/performance/bind_call_spec.rb @@ -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) diff --git a/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb b/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb index effbddea5c..92e305de02 100644 --- a/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb +++ b/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb @@ -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) diff --git a/spec/rubocop/cop/performance/delete_prefix_spec.rb b/spec/rubocop/cop/performance/delete_prefix_spec.rb index 731f43183e..e2508d4ffc 100644 --- a/spec/rubocop/cop/performance/delete_prefix_spec.rb +++ b/spec/rubocop/cop/performance/delete_prefix_spec.rb @@ -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/, '') diff --git a/spec/rubocop/cop/performance/delete_suffix_spec.rb b/spec/rubocop/cop/performance/delete_suffix_spec.rb index d74dde888f..967e5d2991 100644 --- a/spec/rubocop/cop/performance/delete_suffix_spec.rb +++ b/spec/rubocop/cop/performance/delete_suffix_spec.rb @@ -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/, '') diff --git a/spec/rubocop/cop/performance/map_compact_spec.rb b/spec/rubocop/cop/performance/map_compact_spec.rb index 4984f993d4..cbc1dd0920 100644 --- a/spec/rubocop/cop/performance/map_compact_spec.rb +++ b/spec/rubocop/cop/performance/map_compact_spec.rb @@ -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 diff --git a/spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb b/spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb index 3d070a415a..01346760f6 100644 --- a/spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb +++ b/spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb @@ -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) diff --git a/spec/rubocop/cop/performance/redundant_merge_spec.rb b/spec/rubocop/cop/performance/redundant_merge_spec.rb index 0b92360663..32f14db185 100644 --- a/spec/rubocop/cop/performance/redundant_merge_spec.rb +++ b/spec/rubocop/cop/performance/redundant_merge_spec.rb @@ -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)') diff --git a/spec/rubocop/cop/performance/regexp_match_spec.rb b/spec/rubocop/cop/performance/regexp_match_spec.rb index 08fc0dd477..a6fc13270e 100644 --- a/spec/rubocop/cop/performance/regexp_match_spec.rb +++ b/spec/rubocop/cop/performance/regexp_match_spec.rb @@ -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) diff --git a/spec/rubocop/cop/performance/select_map_spec.rb b/spec/rubocop/cop/performance/select_map_spec.rb index f472fd0149..0c97ec5671 100644 --- a/spec/rubocop/cop/performance/select_map_spec.rb +++ b/spec/rubocop/cop/performance/select_map_spec.rb @@ -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) diff --git a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb index 34ad8894fe..b8013046ff 100644 --- a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb +++ b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb @@ -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'`. @@ -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. @@ -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}"`. diff --git a/spec/rubocop/cop/performance/sum_spec.rb b/spec/rubocop/cop/performance/sum_spec.rb index d0d7428cb5..951a4f3097 100644 --- a/spec/rubocop/cop/performance/sum_spec.rb +++ b/spec/rubocop/cop/performance/sum_spec.rb @@ -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, :+) diff --git a/spec/rubocop/cop/performance/unfreeze_string_spec.rb b/spec/rubocop/cop/performance/unfreeze_string_spec.rb index 204e028e09..8894f2eebe 100644 --- a/spec/rubocop/cop/performance/unfreeze_string_spec.rb +++ b/spec/rubocop/cop/performance/unfreeze_string_spec.rb @@ -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 @@ -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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e809538ac4..3e67f78a51 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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