Skip to content

Commit

Permalink
Add Style/DoubleCopDisableDirective cop
Browse files Browse the repository at this point in the history
This cop is the easiest solution to the problem of `--disable-uncorrectable`
generating lines with more than one `rubocop:disable` comment.
  • Loading branch information
jonas054 authored and bbatsov committed Jul 16, 2019
1 parent 2c4243c commit c9f71dc
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 2 deletions.
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,11 @@ Style/DocumentationMethod:
- 'test/**/*'
RequireForNonPublicMethods: false

Style/DoubleCopDisableDirective:
Description: 'Checks for double rubocop:disable comments on a single line.'
Enabled: true
VersionAdded: '0.73'

Style/DoubleNegation:
Description: 'Checks for uses of double negation (!!).'
StyleGuide: '#no-bang-bang'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@
require_relative 'rubocop/cop/style/dir'
require_relative 'rubocop/cop/style/documentation_method'
require_relative 'rubocop/cop/style/documentation'
require_relative 'rubocop/cop/style/double_cop_disable_directive'
require_relative 'rubocop/cop/style/double_negation'
require_relative 'rubocop/cop/style/each_for_simple_loop'
require_relative 'rubocop/cop/style/each_with_object'
Expand Down
49 changes: 49 additions & 0 deletions lib/rubocop/cop/style/double_cop_disable_directive.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

# rubocop:disable Lint/UnneededCopDisableDirective
# rubocop:disable Style/DoubleCopDisableDirective

module RuboCop
module Cop
module Style
# Detects double disable comments on one line. This is mostly to catch
# automatically generated comments that need to be regenerated.
#
# @example
# # bad
# def f # rubocop:disable Style/For # rubocop:disable Metrics/AbcSize
# end
#
# # good
# # rubocop:disable Metrics/AbcSize
# def f # rubocop:disable Style/For
# end
# # rubocop:enable Metrics/AbcSize
#
# # if both fit on one line
# def f # rubocop:disable Style/For, Metrics/AbcSize
# end
#
class DoubleCopDisableDirective < Cop
# rubocop:enable Style/For, Style/DoubleCopDisableDirective
# rubocop:enable Lint/UnneededCopDisableDirective, Metrics/AbcSize
MSG = 'More than one disable comment on one line.'

def investigate(processed_source)
processed_source.comments.each do |comment|
next unless comment.text.scan('# rubocop:disable').size > 1

add_offense(comment)
end
end

def autocorrect(comment)
lambda do |corrector|
corrector.replace(comment.loc.expression,
comment.text[/# rubocop:disable \S+/])
end
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ In the following section you find all available cops:
* [Style/Dir](cops_style.md#styledir)
* [Style/Documentation](cops_style.md#styledocumentation)
* [Style/DocumentationMethod](cops_style.md#styledocumentationmethod)
* [Style/DoubleCopDisableDirective](cops_style.md#styledoublecopdisabledirective)
* [Style/DoubleNegation](cops_style.md#styledoublenegation)
* [Style/EachForSimpleLoop](cops_style.md#styleeachforsimpleloop)
* [Style/EachWithObject](cops_style.md#styleeachwithobject)
Expand Down
27 changes: 27 additions & 0 deletions manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,33 @@ Name | Default value | Configurable values
Exclude | `spec/**/*`, `test/**/*` | Array
RequireForNonPublicMethods | `false` | Boolean

## Style/DoubleCopDisableDirective

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 0.73 | -

Detects double disable comments on one line. This is mostly to catch
automatically generated comments that need to be regenerated.

### Examples

```ruby
# bad
def f # rubocop:disable Style/For # rubocop:disable Metrics/AbcSize
end

# good
# rubocop:disable Metrics/AbcSize
def f # rubocop:disable Style/For
end
# rubocop:enable Metrics/AbcSize

# if both fit on one line
def f # rubocop:disable Style/For, Metrics/AbcSize
end
```

## Style/DoubleNegation

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
72 changes: 70 additions & 2 deletions spec/rubocop/cli/cli_disable_uncorrectable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 7: [Corrected] Layout/SpaceAroundOperators: Surrounding space missing for operator ==.
1 file inspected, 2 offenses detected, 2 offenses corrected
OUTPUT
expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
Expand All @@ -28,7 +28,7 @@
RUBY
end

context 'if a one-line disable statement fits' do
context 'if one one-line disable statement fits' do
it 'adds it' do
create_file('example.rb', <<-RUBY.strip_indent)
def is_example
Expand Down Expand Up @@ -79,6 +79,74 @@ def is_example # rubocop:disable Naming/PredicateName
RUBY
end
end

context "but there are more offenses on the line and they don't all " \
'fit' do
it 'adds both one-line and before-and-after disable statements' do
create_file('example.rb', <<-RUBY.strip_indent)
# Chess engine.
class Chess
def choose_move(who_to_move)
legal_moves = all_legal_moves_that_dont_put_me_in_check(who_to_move)
return nil if legal_moves.empty?
mating_move = checkmating_move(legal_moves)
return mating_move if mating_move
best_moves = checking_moves(legal_moves)
best_moves = castling_moves(legal_moves) if best_moves.empty?
best_moves = taking_moves(legal_moves) if best_moves.empty?
best_moves = legal_moves if best_moves.empty?
best_moves = remove_dangerous_moves(best_moves, who_to_move)
best_moves = legal_moves if best_moves.empty?
best_moves.sample
end
end
RUBY
expect(exit_code).to eq(0)
expect($stderr.string).to eq('')
expect($stdout.string).to eq(<<-OUTPUT.strip_indent)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 3: 3: [Corrected] Metrics/AbcSize: Assignment Branch Condition size for choose_move is too high. [15.62/15]
C: 3: 3: [Corrected] Metrics/CyclomaticComplexity: Cyclomatic complexity for choose_move is too high. [7/6]
C: 3: 3: [Corrected] Metrics/MethodLength: Method has too many lines. [11/10]
C: 5: 3: [Corrected] Metrics/AbcSize: Assignment Branch Condition size for choose_move is too high. [15.62/15]
C: 5: 3: [Corrected] Metrics/MethodLength: Method has too many lines. [11/10]
C: 5: 32: [Corrected] Style/DoubleCopDisableDirective: More than one disable comment on one line.
1 file inspected, 7 offenses detected, 7 offenses corrected
OUTPUT
expect(IO.read('example.rb')).to eq(<<-RUBY.strip_indent)
# frozen_string_literal: true
# Chess engine.
class Chess
# rubocop:disable Metrics/MethodLength
# rubocop:disable Metrics/AbcSize
def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity
legal_moves = all_legal_moves_that_dont_put_me_in_check(who_to_move)
return nil if legal_moves.empty?
mating_move = checkmating_move(legal_moves)
return mating_move if mating_move
best_moves = checking_moves(legal_moves)
best_moves = castling_moves(legal_moves) if best_moves.empty?
best_moves = taking_moves(legal_moves) if best_moves.empty?
best_moves = legal_moves if best_moves.empty?
best_moves = remove_dangerous_moves(best_moves, who_to_move)
best_moves = legal_moves if best_moves.empty?
best_moves.sample
end
# rubocop:enable Metrics/AbcSize
# rubocop:enable Metrics/MethodLength
end
RUBY
end
end
end

context "if a one-line disable statement doesn't fit" do
Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cop/style/double_cop_disable_directive_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::DoubleCopDisableDirective do
subject(:cop) { described_class.new }

it 'registers an offense when using `#bad_method`' do
expect_offense(<<~RUBY)
def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/MethodLength
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ More than one disable comment on one line.
end
RUBY
expect_correction(<<~RUBY)
def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity
end
RUBY
end

it 'does not register an offense when using `#good_method`' do
expect_no_offenses(<<~RUBY)
def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity
end
RUBY
end
end

0 comments on commit c9f71dc

Please sign in to comment.