prontolabs / pronto-rubocop

Pronto runner for Rubocop, ruby code analyzer
MIT License
83 stars 74 forks source link

pronto-rubocop crashes with undefined method `rewrite' for nil:NilClass (NoMethodError) #83

Closed d4rky-pl closed 1 year ago

d4rky-pl commented 1 year ago

I'm investigating an issue after upgrading to the latest rubocop and pronto-rubocop in our project. It seems that it's sometimes crashing with

/path/vendor/bundle/ruby/3.0.0/gems/pronto-rubocop-0.11.4/lib/pronto/rubocop/offense_line.rb:43:in `corrected_lines': undefined method `rewrite' for nil:NilClass (NoMethodError)

Further investigation suggests that RuboCop::Cop::Base::InvestigationReport instance has corrector set to nil. This causes corrected_lines to fail. The cop that causes this is RuboCop::Cop::Metrics::ClassLength but I have no idea how to debug it further.

d4rky-pl commented 1 year ago

I've managed to narrow it down a bit, it seems to be working fine in rubocop 1.40.0 but starts failing in >= 1.41.0. There seems to have been some changes around the correctors in 1.41.0 which may cause the incompatibility.

Downgrading rubocop to 1.40.0 have solved our problem temporarily.

ashkulz commented 1 year ago

@d4rky-pl is it possible to create a test case? If I see a recent run, it used rubocop 1.43.0. I'd want to enhance the tests to cover the scenario you're hitting before we can actually fix it.

jesseclark commented 1 year ago

I just hit this one too. It fails consistently on one PR but works fine on others.

ashkulz commented 1 year ago

@jesseclark which cops are enabled? Also, is it possible to make to look at the diff and figure out which cop is triggering this?

I would love to have a small testcase which is reproducible :sweat_smile:

jesseclark commented 1 year ago

@ashkulz The only cop violation I see in the files that changed is Rails/LexicallyScopedActionFilter.

Getting the full list of enabled/disabled cops might be a bit more involved but I could do so if that isn't enough to go on.

jesseclark commented 1 year ago

Just for a little extra verification, I modified one of the lines that cause the violation and opened a new PR and saw the NoMethodError from Pronto. I then disabled Rails/LexicallyScopedActionFilter and pushed that. Pronto ran fine.

The code that is generating the violation is in a Rails Controller where we have a before_action call scoped to only: :show but the show action isn't defined in the controller. If that helps you to setup a test case.

ashkulz commented 1 year ago

@d4rky-pl / @jesseclark, can you check if it works for you with the linked PR?

d4rky-pl commented 1 year ago

@ashkulz apologies, I was not able to get back to this topic until today. I can confirm the linked PR fixes the problem with rubocop > 1.40 both locally and our CI.

ashkulz commented 1 year ago

This has been pushed to RubyGems :tada:

jesseclark commented 1 year ago

Thanks @ashkulz!