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

Style/RaiseArgs cop only reports the first instance encountered #8759

Closed
pbernays opened this issue Sep 21, 2020 · 5 comments
Closed

Style/RaiseArgs cop only reports the first instance encountered #8759

pbernays opened this issue Sep 21, 2020 · 5 comments

Comments

@pbernays
Copy link
Contributor

pbernays commented Sep 21, 2020

As the title suggests, Style/RaiseArgs only reports one instance at a time. I'm not clear on the context of compact vs exploded, nor the idea behind opposite_style_detected, but commenting out line 106 (return unless opposite_style_detected) of the cop seems to take care of business. This line was introduced after 0.89.0 and the cop works fine before that.


Expected behavior

Multiple instances of raising an error that instantiates an error class with a message all report the Style/RaiseArgs cop.

Actual behavior

(Edited)
Only the instances that Rubocop encounters before subsequently encountering the allowed form of raising errors are reported. All other instances are ignored.

Steps to reproduce the problem

(Edited)

rubocop_test.rb:

# frozen_string_literal: true

raise ArgumentError.new('foobar') if ENV['foobar']
raise ArgumentError, 'foobar' if ENV['foobar']

raise ArgumentError.new('foobar')

$ rubocop
Inspecting 1 file
C

Offenses:

rubocop_test.rb:3:1: C: Style/RaiseArgs: Provide an exception class and message as arguments to raise.
raise ArgumentError.new('foobar') if ENV['foobar']
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable

RuboCop version

0.91.0

@KaraAJC
Copy link

KaraAJC commented Sep 25, 2020

I am getting similar behavior running the Layout/SpaceInsideBlockBraces cop, where only the first encounter is reported, and I have to repeatedly run rubocop to find each following issue.

@fatkodima
Copy link
Contributor

Can't reproduce.
Do you have any rubocop-related gems, like rubocop-rails, etc? Which versions do they have?

@pbernays
Copy link
Contributor Author

Yes, we have rubocop-rails 2.8.1, rubocop-rspec 1.2.9, and rubocop-thread_safety 0.3.4. However, I ran rubocop --auto-gen-config without the plugins and a bare .rubocop.yml against the code base with four instances of raise ArgumentError.new("foobar") across two files. The generated .rubocop_todo.yml had:

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: compact, exploded
Style/RaiseArgs:
  Exclude:
    - 'config/environments/development.rb'

where I would have expected 4 offences and config/environments/test.rb to be excluded as well. Running rubocop from within config or config/environments, or passing those directories as arguments to rubocop correctly reports all 4 offences. I'll keep moving the raises around to try isolating an area of the code base that reproduces the problem without running on the whole project.

@pbernays
Copy link
Contributor Author

I isolated the problem down to one file and edited the steps to reproduce in the initial comment.

@pbernays
Copy link
Contributor Author

pbernays commented Sep 29, 2020

Other examples where this fails, including @KaraAJC's finding:

# frozen_string_literal: true

raise ArgumentError, 'foobar' if ENV['foobar']
raise ArgumentError.new('foobar') if ENV['foobar']
raise ArgumentError.new('foobar') if ENV['foobar']

[:foo].each { puts 3 }
[:bar].each {puts 3}
[].each {puts 3}

String.new
String.
  new
String.
  new

def foobar1(foo = nil, bar = nil)
  puts [foo, bar]
end

def foobar2(foo=nil, bar=nil)
  puts [foo, bar]
end

def foobar3(foo=nil, bar=nil)
  puts [foo, bar]
end

Also related to #8799 and #8801.

I don't know what the intent is with disabling an enforced style mid-run, but any cop that uses return unless opposite_style_detected does exactly that because of:
https://github.com/rubocop-hq/rubocop/blob/e18f297d134e7756ade1c18a81ea93e8575be7b1/lib/rubocop/cop/mixin/configurable_enforced_style.rb#L49

@koic koic closed this as completed in 40542a7 Sep 30, 2020
koic added a commit that referenced this issue Sep 30, 2020
[Fix #8759] Fix multiple offense detection for Style/RaiseArgs
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

No branches or pull requests

3 participants