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

Fix wrong usage of Array#reject!: this might return nil #612

Merged
merged 1 commit into from
May 13, 2022

Conversation

r7kamura
Copy link
Contributor

@r7kamura r7kamura commented May 13, 2022

About

I found a behavior that seems to be a bug and would like to correct it.

In some condition, the #all_rules might raise NoMethodError.

Details

Why?

reject! は要素が 1 つ以上削除されれば self を、 1 つも削除されなければ nil を返します。

How to reproduce

I coudn't write test about this behavior, but let me share how to reproduce this:

Modify the example .rubocop_todo.yml:

diff --git a/spec/fixtures/.rubocop_todo.yml b/spec/fixtures/.rubocop_todo.yml
index 7977dc7..4b73d25 100644
--- a/spec/fixtures/.rubocop_todo.yml
+++ b/spec/fixtures/.rubocop_todo.yml
@@ -1,11 +1,3 @@
-# This configuration was generated by
-# `rubocop --auto-gen-config`
-# on 2018-10-14 13:25:00 +0900 using RuboCop version 0.59.2.
-# The point is for the user to remove these configuration records
-# one by one as the offenses are removed from the code base.
-# Note that changes in the inspected code, or installation of new
-# versions of RuboCop, may require this file to be generated again.
-
 # Offense count: 1
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle.

Then run rspec:

$ bundle exec rspec spec/lib/rubocop_challenger/rubocop/todo_reader_spec.rb:149
Run options: include {:locations=>{"./spec/lib/rubocop_challenger/rubocop/todo_reader_spec.rb"=>[149]}}

RubocopChallenger::Rubocop::TodoReader
  #any_rule
    returns a auto correctable rule at random (FAILED - 1)

Failures:

  1) RubocopChallenger::Rubocop::TodoReader#any_rule returns a auto correctable rule at random
     Failure/Error:
       file_contents
       .split(/\n{2,}/)
       .map! { |content| Rule.new(content) }
       .reject! { |rule| invalid?(rule) }
       .sort!
     
     NoMethodError:
       undefined method `sort!' for nil:NilClass
     # ./lib/rubocop_challenger/rubocop/todo_reader.rb:27:in `all_rules'
     # ./lib/rubocop_challenger/rubocop/todo_reader.rb:32:in `auto_correctable_rules'
     # ./lib/rubocop_challenger/rubocop/todo_reader.rb:47:in `any_rule'
     # ./spec/lib/rubocop_challenger/rubocop/todo_reader_spec.rb:150:in `block (3 levels) in <top (required)>'

Finished in 0.00117 seconds (files took 0.28482 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/lib/rubocop_challenger/rubocop/todo_reader_spec.rb:149 # RubocopChallenger::Rubocop::TodoReader#any_rule returns a auto correctable rule at random

Coverage report generated for RSpec to /home/r7kamura/ghq/github.com/r7kamura/rubocop_challenger/coverage. 229 / 386 LOC (59.33%) covered.
Stopped processing SimpleCov as a previous error not related to SimpleCov has been detected

Other concerns

I found this while investigating another problem. The recent rubocop has --no-offense-counts option and it's enabled in my project, so rubocop_challenger doesn't work well there...

@r7kamura
Copy link
Contributor Author

This is the error log where I found this bug:

Switched to a new branch 'rubocop-challenge/20220513042015'
Bundler attempted to update rubocop but its version stayed the same
Bundler attempted to update rubocop-performance but its version stayed the same
Bundler attempted to update rubocop-rails but its version stayed the same
Bundler attempted to update rubocop-rake but its version stayed the same
Bundler attempted to update rubocop-rspec but its version stayed the same
Bundler attempted to update rubocop-thread_safety but its version stayed the same
cannot load such file -- rubocop-rails_deprecation
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader_resolver.rb:17:in `require'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader_resolver.rb:17:in `block (2 levels) in resolve_requires'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader_resolver.rb:13:in `each'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader_resolver.rb:13:in `block in resolve_requires'
<internal:kernel>:90:in `tap'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader_resolver.rb:12:in `resolve_requires'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader.rb:44:in `load_file'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_loader.rb:103:in `configuration_from_file'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_store.rb:68:in `for_dir'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/config_store.rb:47:in `for_pwd'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli/command/auto_genenerate_config.rb:30:in `maybe_run_line_length_cop'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli/command/auto_genenerate_config.rb:23:in `run'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli/command.rb:11:in `run'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli/environment.rb:18:in `run'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli.rb:71:in `run_command'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli.rb:76:in `execute_runners'
/usr/local/bundle/gems/rubocop-1.29.1/lib/rubocop/cli.rb:47:in `run'
/usr/local/bundle/gems/rubocop-1.29.1/exe/rubocop:12:in `block in <top (required)>'
/usr/local/lib/ruby/3.1.0/benchmark.rb:311:in `realtime'
/usr/local/bundle/gems/rubocop-1.29.1/exe/rubocop:12:in `<top (required)>'
/usr/local/bundle/bin/rubocop:25:in `load'
/usr/local/bundle/bin/rubocop:25:in `<top (required)>'
/usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `load'
/usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `kernel_load'
/usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:23:in `run'
/usr/local/lib/ruby/3.1.0/bundler/cli.rb:484:in `exec'
/usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
/usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
/usr/local/lib/ruby/3.1.0/bundler/cli.rb:31:in `dispatch'
/usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
/usr/local/lib/ruby/3.1.0/bundler/cli.rb:25:in `start'
/usr/local/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:48:in `block in <top (required)>'
/usr/local/lib/ruby/3.1.0/bundler/friendly_errors.rb:103:in `with_friendly_errors'
/usr/local/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:36:in `<top (required)>'
/usr/local/bin/bundle:25:in `load'
/usr/local/bin/bundle:25:in `<main>'
/usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/todo_reader.rb:27:in `all_rules': undefined method `sort!' for nil:NilClass (NoMethodError)

          .sort!
          ^^^^^^
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/todo_reader.rb:32:in `auto_correctable_rules'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/todo_reader.rb:47:in `any_rule'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/challenge.rb:41:in `target_rule'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/challenge.rb:32:in `verify_target_rule'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/challenge.rb:25:in `exec'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/rubocop/challenge.rb:8:in `exec'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/go.rb:97:in `rubocop_challenge!'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/go.rb:42:in `exec'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/lib/rubocop_challenger/cli.rb:74:in `go'
	from /usr/local/bundle/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
	from /usr/local/bundle/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
	from /usr/local/bundle/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
	from /usr/local/bundle/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
	from /usr/local/bundle/gems/rubocop_challenger-2.7.0/exe/rubocop_challenger:6:in `<top (required)>'
	from /usr/local/bundle/bin/rubocop_challenger:25:in `load'
	from /usr/local/bundle/bin/rubocop_challenger:25:in `<top (required)>'
	from /usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `load'
	from /usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:58:in `kernel_load'
	from /usr/local/lib/ruby/3.1.0/bundler/cli/exec.rb:23:in `run'
	from /usr/local/lib/ruby/3.1.0/bundler/cli.rb:484:in `exec'
	from /usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
	from /usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
	from /usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
	from /usr/local/lib/ruby/3.1.0/bundler/cli.rb:31:in `dispatch'
	from /usr/local/lib/ruby/3.1.0/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
	from /usr/local/lib/ruby/3.1.0/bundler/cli.rb:25:in `start'
	from /usr/local/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:48:in `block in <top (required)>'
	from /usr/local/lib/ruby/3.1.0/bundler/friendly_errors.rb:103:in `with_friendly_errors'
	from /usr/local/lib/ruby/gems/3.1.0/gems/bundler-2.3.7/libexec/bundle:36:in `<top (required)>'
	from /usr/local/bin/bundle:25:in `load'
	from /usr/local/bin/bundle:25:in `<main>'
undefined method `sort!' for nil:NilClass

          .sort!
          ^^^^^^

Copy link
Owner

@ryz310 ryz310 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you so much!

@ryz310 ryz310 merged commit 8941886 into ryz310:master May 13, 2022
@ryz310
Copy link
Owner

ryz310 commented May 13, 2022

@r7kamura

Other concerns

I found this while investigating another problem. The recent rubocop has --no-offense-counts option and it's enabled in my project, so rubocop_challenger doesn't work well there...

I see. I understand why you use the "random mode" as default on your custom action.
I'll consider about #613. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants