Skip to content

Commit

Permalink
[Fix #5708] Handle comment rubocop:enable all bug (#5810)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mikhail Varabyou authored and bbatsov committed May 14, 2018
1 parent da75dc8 commit d4e25f2
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* [#5809](https://github.com/bbatsov/rubocop/issues/5809): Fix exception `Lint/PercentStringArray` and `Lint/PercentSymbolArray` when the inspected file is binary encoded. ([@akhramov][])
* [#5840](https://github.com/bbatsov/rubocop/issues/5840): Do not register an offense for methods that `nil` responds to in `Lint/SafeNavigationConsistency`. ([@rrosenblum][])
* [#5862](https://github.com/bbatsov/rubocop/issues/5862): Fix an incorrect auto-correct for `Lint/LiteralInInterpolation` if contains numbers. ([@koic][])
* [#5708](https://github.com/bbatsov/rubocop/issues/5708): Fix exception in `Lint/UnneededCopEnableDirective` for instruction '# rubocop:enable **all**'. ([@balbesina][])
* Fix auto-correction of `Rails/HttpPositionalArgumnets` to use `session` instead of `header`. ([@rrosenblum][])

### Changes
Expand Down Expand Up @@ -3360,3 +3361,4 @@
[@JacobEvelyn]: https://github.com/JacobEvelyn
[@shanecav84]: https://github.com/shanecav84
[@thomasbrus]: https://github.com/thomasbrus
[@balbesina]: https://github.com/balbesina
45 changes: 36 additions & 9 deletions lib/rubocop/comment_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,14 @@ def extra_enabled_comments
def extra_enabled_comments_with_names(extras, names)
each_directive do |comment, cop_names, disabled|
next unless comment_only_line?(comment.loc.expression.line)
cop_names.each do |name|
names[name] ||= 0
if disabled
names[name] += 1
elsif names[name] > 0
names[name] -= 1
else
extras << [comment, name]
end

if !disabled && enable_all?(comment)
handle_enable_all(names, extras, comment)
else
handle_switch(cop_names, names, disabled, extras, comment)
end
end

extras
end

Expand Down Expand Up @@ -169,5 +166,35 @@ def non_comment_token_line_numbers
non_comment_tokens.map(&:line).uniq
end
end

def enable_all?(comment)
_, cops = comment.text.match(COMMENT_DIRECTIVE_REGEXP).captures
cops == 'all'
end

def handle_enable_all(names, extras, comment)
enabled_cops = 0
names.each do |name, counter|
next unless counter > 0

names[name] -= 1
enabled_cops += 1
end

extras << [comment, 'all'] if enabled_cops.zero?
end

def handle_switch(cop_names, names, disabled, extras, comment)
cop_names.each do |name|
names[name] ||= 0
if disabled
names[name] += 1
elsif names[name] > 0
names[name] -= 1
else
extras << [comment, name]
end
end
end
end
end
22 changes: 20 additions & 2 deletions lib/rubocop/cop/lint/unneeded_cop_enable_directive.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,28 @@ module Lint
# This cop detects instances of rubocop:enable comments that can be
# removed.
#
# When comment enables all cops at once `rubocop:enable all`
# that cop checks whether any cop was actually enabled.
# @example
# # bad
# foo = 1
# # rubocop:enable Metrics/LineLength
#
# # good
# foo = 1
# @example
# # bad
# # rubocop:disable Metrics/LineLength
# baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarrrrrrrrrrrrr
# # rubocop:enable Metrics/LineLength
# baz
# # rubocop:enable all
#
# # good
# # rubocop:disable Metrics/LineLength
# baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarrrrrrrrrrrrr
# # rubocop:enable all
# baz
class UnneededCopEnableDirective < Cop
include RangeHelp
include SurroundingSpace
Expand All @@ -30,7 +45,7 @@ def investigate(processed_source)
add_offense(
[comment, name],
location: range_of_offense(comment, name),
message: format(MSG, cop: name)
message: format(MSG, cop: all_or_name(name))
)
end
end
Expand Down Expand Up @@ -90,8 +105,11 @@ def range_to_remove(begin_pos, end_pos, comma_pos, comment)
range_class.new(buffer, start, comment.loc.expression.end_pos)
end
end

def all_or_name(name)
name == 'all' ? 'all cops' : name
end
end
end
end
end
# rubocop:enable Lint/UnneededCopEnableDirective
17 changes: 17 additions & 0 deletions manual/cops_lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,9 @@ Enabled | Yes
This cop detects instances of rubocop:enable comments that can be
removed.

When comment enables all cops at once `rubocop:enable all`
that cop checks whether any cop was actually enabled.

### Examples

```ruby
Expand All @@ -2052,6 +2055,20 @@ foo = 1
# good
foo = 1
```
```ruby
# bad
# rubocop:disable Metrics/LineLength
baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarrrrrrrrrrrrr
# rubocop:enable Metrics/LineLength
baz
# rubocop:enable all

# good
# rubocop:disable Metrics/LineLength
baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarrrrrrrrrrrrr
# rubocop:enable all
baz
```

## Lint/UnneededRequireStatement

Expand Down
20 changes: 20 additions & 0 deletions spec/rubocop/cop/lint/unneeded_cop_enable_directive_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@
RUBY
end

context 'all switch' do
it 'registers offense for unnecessary enable all' do
expect_offense(<<-RUBY.strip_indent)
foo
# rubocop:enable all
^^^ Unnecessary enabling of all cops.
RUBY
end

context 'when at least one cop was disabled' do
it 'does not register offence' do
expect_no_offenses(<<-RUBY.strip_indent)
# rubocop:disable Metrics/LineLength
foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
# rubocop:enable all
RUBY
end
end
end

context 'autocorrection' do
context 'when entire comment unnecessarily enables' do
let(:source) do
Expand Down

0 comments on commit d4e25f2

Please sign in to comment.