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

Conflict between Lint/RedundantCopDisableDirective and Lint/SuppressedException #7886

Closed
olliebennett opened this issue Apr 17, 2020 · 0 comments
Labels

Comments

@olliebennett
Copy link
Contributor

olliebennett commented Apr 17, 2020

My previously valid usage of # rubocop:disable Lint/SuppressedException is being flagged by the Lint/RedundantCopDisableDirective cop. Upon removing the rubocop:disable, the Lint/SuppressedException cop raises a violation again.


Expected behavior

My usage of rubocop:disable should not be flagged as a Lint/RedundantCopDisableDirective; it seems to be legitimately disabling the Lint/SuppressedException cop.

Actual behavior

My usage of rubocop:disable is flagged.

Steps to reproduce the problem

Create the file spec/dummy_spec.rb;

RSpec.describe Dummy do
  it 'dummy spec' do
    # This rescue is here to ensure the test does not fail because of the `raise`
    # rubocop:disable Lint/SuppressedException
    expect { begin subject; rescue ActiveRecord::Rollback; end }.not_to(change(Post, :count))
    # rubocop:enable Lint/SuppressedException
  end
end

Run RuboCop on the file and observe output;

➜ bundle exec rubocop spec/dummy_spec.rb
spec/dummy_spec.rb:4:5: W: Lint/RedundantCopDisableDirective: Unnecessary disabling of Lint/SuppressedException.                                                              
    # rubocop:disable Lint/SuppressedException
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 1/1 file |====================================================================== 100 ======================================================================>| Time: 00:00:00 

1 file inspected, 1 offense detected

Edit the file (to remove the apparently invalid rubocop:disable and rubocop:enable) as follows;

RSpec.describe Dummy do
  it 'dummy spec' do
    # This rescue is here to ensure the test does not fail because of the `raise`
    expect { begin subject; rescue ActiveRecord::Rollback; end }.not_to(change(Post, :count))
  end
end

Re-run RuboCop as before;

➜ bundle exec rubocop spec/dummy_spec.rb
spec/dummy_spec.rb:4:29: W: Lint/SuppressedException: Do not suppress exceptions.                                                                                             
    expect { begin subject; rescue ActiveRecord::Rollback; end }.not_to(change(Post, :count))
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 1/1 file |====================================================================== 100 ======================================================================>| Time: 00:00:00 

1 file inspected, 1 offense detected

RuboCop version

This has occurred since updating to RuboCop 0.81, and continues in RuboCop 0.82. It did not occur for RuboCop 0.80. Haven't had a chance to bisect the exact commit where it was introduced (but happy to if the cause isn't clear).

➜ bundle exec rubocop -V
0.82.0 (using Parser 2.7.1.1, running on ruby 2.7.0 x86_64-darwin18)
@koic koic added the bug label Apr 19, 2020
jonas054 added a commit to jonas054/rubocop that referenced this issue May 17, 2020
To avoid false negatives for single line rescue statements with a
comment on the next line, we look for comments, not on the line
after rescue, but one lines between rescue and the corresponding
end.
gdumani added a commit to gdumani/TicTacToe that referenced this issue Aug 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants