Add job for CI using Prism parser

rubocop / rubocop-rspec

Code style checking for RSpec files.

https://docs.rubocop.org/rubocop-rspec

MIT License

811 stars 276 forks source link

Add job for CI using Prism parser #1980

Closed bquorning closed 3 weeks ago

bquorning commented 1 month ago

Inspired by https://github.com/rubocop/rubocop-capybara/pull/112

Before submitting the PR make sure the following are checked:

[x] Feature branch is up-to-date with master (if not - rebase it).
[x] Squashed related commits together.
[ ] Added tests.
[ ] Updated documentation.
[ ] Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
[x] The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

pirj commented 1 month ago

But ruby-head is prism already, isn’t this sufficient?

bquorning commented 1 month ago

Ah yes, probably so. I'll keep this in draft and look at it again next week.

Earlopain commented 1 month ago

My understanding is that it will still use the plain parser gem, even with ruby-head/ruby-3.4 The prism default is runtime-only. Users need to set it in their config files still, even when targeting ruby 3.3 or higher.

pirj commented 1 month ago

Thanks for the hint, @Earlopain !

I’m good with a separate ci job with prism. Do we need to turn it on in RuboCop somehow? 🤔

Earlopain commented 1 month ago

As a user, you would set AllCops/ParserEngine. For extensions, this should have you covered: https://github.com/rubocop/rubocop/blob/9f2ffa2270bbc174b2a622350d120b05672a1ce5/lib/rubocop/rspec/cop_helper.rb#L13. I'm not immediatly finding the place where you inherit this, but you probably do somewhere? Since you use the same assertion methods from RuboCop::RSpec::ExpectOffense.

For reference, here's the PR for rubocop-rails and it basically only sets the env variable. https://github.com/rubocop/rubocop-rails/pull/1245. The other changes are for places that construct ProcessedSource themselves but I don't believe that will be the case here.

bquorning commented 3 weeks ago

I changed it to run only rake spec and not all of rake. Does it look ok now?