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

Internally, the Parser gem API is used directly to parse db/schema.rb.
This has been replaced with `RuboCop::AST::ProcessedSource` from RuboCop AST 1.31,
which is compatible with the Prism parser API of the Parser gem API.

This change resolves error (e.g., `uninitialized constant Parser::Ruby33`)
related to db/schema.rb parsing when Prism is specified as the parser engine,
enabling paring with Prism.
  • Loading branch information
koic committed Mar 3, 2024
1 parent 6dc0e59 commit d8ce294
Show file tree
Hide file tree
Showing 20 changed files with 87 additions and 38 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 @@
* [#1245](https://github.com/rubocop/rubocop-rails/pull/1245): Support Prism as a Ruby parser. ([@koic][])
7 changes: 6 additions & 1 deletion lib/rubocop/cop/mixin/active_record_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ def external_dependency_checksum
end

def schema
RuboCop::Rails::SchemaLoader.load(target_ruby_version)
# For compatibility with RuboCop 1.61.0 or lower.
if respond_to?(:parser_engine)
RuboCop::Rails::SchemaLoader.load(target_ruby_version, parser_engine)
else
RuboCop::Rails::SchemaLoader.load(target_ruby_version, :parser_whitequark)
end
end

def table_name(class_node)
Expand Down
20 changes: 5 additions & 15 deletions lib/rubocop/rails/schema_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ module SchemaLoader
# So a cop that uses the loader should handle `nil` properly.
#
# @return [Schema, nil]
def load(target_ruby_version)
def load(target_ruby_version, parser_engine)
return @load if defined?(@load)

@load = load!(target_ruby_version)
@load = load!(target_ruby_version, parser_engine)
end

def reset!
Expand All @@ -38,23 +38,13 @@ def db_schema_path

private

def load!(target_ruby_version)
def load!(target_ruby_version, parser_engine)
path = db_schema_path
return unless path

ast = parse(path, target_ruby_version)
Schema.new(ast) if ast
end

def parse(path, target_ruby_version)
klass_name = :"Ruby#{target_ruby_version.to_s.sub('.', '')}"
klass = ::Parser.const_get(klass_name)
parser = klass.new(RuboCop::AST::Builder.new)
ast = RuboCop::ProcessedSource.new(File.read(path), target_ruby_version, path, parser_engine: parser_engine).ast

buffer = Parser::Source::Buffer.new(path, 1)
buffer.source = path.read

parser.parse(buffer)
Schema.new(ast) if ast
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion rubocop-rails.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ Gem::Specification.new do |s|
# introduced in rack 1.1
s.add_runtime_dependency 'rack', '>= 1.1'
s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 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
4 changes: 3 additions & 1 deletion spec/rubocop/cop/rails/blank_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

RSpec.describe RuboCop::Cop::Rails::Blank, :config do
shared_examples 'offense' do |source, correction, message|
it 'registers an offense and corrects' 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 and corrects', broken_on: :prism do
expect_offense(<<~RUBY, source: source, message: message)
#{source}
^{source} #{message}
Expand Down
4 changes: 3 additions & 1 deletion spec/rubocop/cop/rails/exit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
RUBY
end

it 'registers an offense for an exit! call 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 for an exit! call with no receiver', broken_on: :prism do
expect_offense(<<~RUBY)
exit!
^^^^^ Do not use `exit` in Rails applications.
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/rails/index_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
end
end

context 'when using Ruby 2.5 or older', :ruby25 do
context 'when using Ruby 2.5 or older', :ruby25, unsupported_on: :prism do
it 'does not register an offense for `to_h { ... }`' do
expect_no_offenses(<<~RUBY)
x.to_h { |el| [el.to_sym, el] }
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/rails/index_with_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
end
end

context 'when using Ruby 2.5 or older', :ruby25 do
context 'when using Ruby 2.5 or older', :ruby25, unsupported_on: :prism do
it 'does not register an offense for `to_h { ... }`' do
expect_no_offenses(<<~RUBY)
x.to_h { |el| [el, el.to_sym] }
Expand Down
4 changes: 3 additions & 1 deletion spec/rubocop/cop/rails/present_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

RSpec.describe RuboCop::Cop::Rails::Present, :config do
shared_examples 'offense' do |source, correction, message|
it 'registers an offense and corrects' 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 and corrects', broken_on: :prism do
expect_offense(<<~RUBY, source: source, message: message)
#{source}
^{source} #{message}
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/rails/root_pathname_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
end
end

context 'when using `Dir.glob` on Ruby 2.4 or lower', :ruby24 do
context 'when using `Dir.glob` on Ruby 2.4 or lower', :ruby24, unsupported_on: :prism do
it 'does not registers an offense' do
expect_no_offenses(<<~RUBY)
Dir.glob(Rails.root.join('**/*.rb'))
Expand Down
11 changes: 8 additions & 3 deletions spec/rubocop/cop/rails/safe_navigation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

it_behaves_like 'accepts', 'non try! method calls', 'join'

context 'target_ruby_version < 2.3', :ruby22 do
context 'target_ruby_version < 2.3', :ruby22, unsupported_on: :prism do
it_behaves_like 'accepts', 'try! with a single parameter', 'try!(:join)'
it_behaves_like 'accepts', 'try! with a multiple parameters', 'try!(:join, ",")'
it_behaves_like 'accepts', 'try! with a block', 'try!(:map) { |e| e.some_method }'
Expand Down Expand Up @@ -93,7 +93,7 @@
context 'convert try and try!' do
let(:cop_config) { { 'ConvertTry' => true } }

context 'target_ruby_version < 2.3', :ruby22 do
context 'target_ruby_version < 2.3', :ruby22, unsupported_on: :prism do
it_behaves_like 'accepts', 'try! with a single parameter', 'try!(:join)'
it_behaves_like 'accepts', 'try! with a multiple parameters', 'try!(:join, ",")'
it_behaves_like 'accepts', 'try! with a block', 'try!(:map) { |e| e.some_method }'
Expand All @@ -119,7 +119,12 @@
it_behaves_like 'autocorrect', 'try! with 2 parameters', '[1, 2].try!(:join, ",")', '[1, 2]&.join(",")'
it_behaves_like 'autocorrect', 'try! with multiple parameters',
'[1, 2].try!(:join, bar, baz)', '[1, 2]&.join(bar, baz)'
it_behaves_like 'autocorrect', 'try! without receiver', 'try!(:join)', 'self&.join'
# 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 'skip test when parser engine is prism', broken_on: :prism do
it_behaves_like 'autocorrect', 'try! without receiver', 'try!(:join)', 'self&.join'
end

it_behaves_like 'autocorrect', 'try! with a block',
['[foo, bar].try!(:map) do |e|',
' e.some_method',
Expand Down
4 changes: 3 additions & 1 deletion spec/rubocop/cop/rails/save_bang_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@
end
end

it "when using #{method} without arguments" 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 "when using #{method} without arguments", broken_on: :prism do
expect_offense(<<~RUBY, method: method)
#{method}
^{method} Use `#{method}!` instead of `#{method}` if the return value is not checked.
Expand Down
12 changes: 9 additions & 3 deletions spec/rubocop/cop/rails/skips_model_validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,27 @@
RUBY
end

it "registers an offense for #{method} with `:returning` keyword 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 for #{method} with `:returning` keyword argument", broken_on: :prism do
expect_offense(<<~RUBY, method: method)
%{method}(attributes, returning: false)
^{method} Avoid using `#{method}` because it skips validations.
RUBY
end

it "registers an offense for #{method} with `:unique_by` keyword 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 for #{method} with `:unique_by` keyword argument", broken_on: :prism do
expect_offense(<<~RUBY, method: method)
%{method}(attributes, unique_by: :username)
^{method} Avoid using `#{method}` because it skips validations.
RUBY
end

it "registers an offense for #{method} with both `:returning` and `:unique_by` keyword arguments" 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 for #{method} with both `:returning` and `:unique_by` keyword arguments", broken_on: :prism do # rubocop:disable Layout/LineLength
expect_offense(<<~RUBY, method: method)
%{method}(attributes, returning: false, unique_by: :username)
^{method} Avoid using `#{method}` because it skips validations.
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/rails/strip_heredoc_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::StripHeredoc, :config do
context 'Ruby <= 2.2', :ruby22 do
context 'Ruby <= 2.2', :ruby22, unsupported_on: :prism do
it 'does not register an offense when using `strip_heredoc`' do
expect_no_offenses(<<~RUBY)
<<-EOS.strip_heredoc
Expand Down
8 changes: 6 additions & 2 deletions spec/rubocop/cop/rails/where_exists_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@
RUBY
end

it 'registers an offense when using implicit receiver and arg' 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 implicit receiver and arg', broken_on: :prism do
expect_offense(<<~RUBY)
where('name = ?', 'john').exists?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `exists?(['name = ?', 'john'])` over `where('name = ?', 'john').exists?`.
Expand Down Expand Up @@ -153,7 +155,9 @@
RUBY
end

it 'registers an offense when using implicit receiver and arg' 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 implicit receiver and arg', broken_on: :prism do
expect_offense(<<~RUBY)
exists?('name = ?', 'john')
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `where('name = ?', 'john').exists?` over `exists?('name = ?', 'john')`.
Expand Down
11 changes: 7 additions & 4 deletions spec/rubocop/rails/schema_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

RSpec.describe RuboCop::Rails::SchemaLoader do
describe '.load' do
require 'parser/ruby27'
let(:target_ruby_version) { 2.7 }
let(:target_ruby_version) do
# The minimum version Prism can parse is 3.3.
ENV['PARSER_ENGINE'] == 'parser_prism' ? 3.3 : RuboCop::TargetRuby::DEFAULT_VERSION
end
let(:parser_engine) { ENV.fetch('PARSER_ENGINE', :parser_whitequark).to_sym }

around do |example|
described_class.reset!
Expand All @@ -13,13 +16,13 @@

context 'without schema.rb' do
it do
expect(described_class.load(target_ruby_version).nil?).to be(true)
expect(described_class.load(target_ruby_version, parser_engine).nil?).to be(true)
end
end

context 'with schema.rb' do
subject(:loaded_schema) do
described_class.load(target_ruby_version)
described_class.load(target_ruby_version, parser_engine)
end

let(:rails_root) { Pathname(Dir.mktmpdir) }
Expand Down
5 changes: 5 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

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 d8ce294

Please sign in to comment.