Skip to content

Commit

Permalink
[Fix rubocop#7814] Fix a false positive for Migrate/DepartmentName cop
Browse files Browse the repository at this point in the history
### Summary

Fixes rubocop#7814.

This PR fixes a false positive for `Migrate/DepartmentName` cop
when inspecting an unexpected disable comment format.

e.g. `# rubocop:disable Style:BlockComments`

The above expected format is `# rubocop:disable Style/BlockComments`.

### Other Information

`Migration/DepartmentName` cop's role is to complement a department name.
The role would be simple if another feature could detect unexpected
disable comment format.
  • Loading branch information
koic committed Mar 24, 2020
1 parent fcfa953 commit cf3a020
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -44,6 +44,7 @@
* [#7682](https://github.com/rubocop-hq/rubocop/issues/7682): Fix `Style/InverseMethods` autofix leaving parenthesis. ([@tejasbubane][])
* [#7745](https://github.com/rubocop-hq/rubocop/issues/7745): Suppress a pending cop warnings when pending cop's department is disabled. ([@koic][])
* [#7759](https://github.com/rubocop-hq/rubocop/issues/7759): Fix an error for `Layout/LineLength` cop when using lambda syntax that argument is not enclosed in parentheses. ([@koic][])
* [#7814](https://github.com/rubocop-hq/rubocop/issues/7814): Fix a false positive for `Migrate/DepartmentName` cop when inspecting an unexpected disabled comment format. ([@koic][])

### Changes

Expand Down
31 changes: 22 additions & 9 deletions lib/rubocop/cop/migration/department_name.rb
Expand Up @@ -14,19 +14,24 @@ class DepartmentName < Cop
/\A(# *rubocop *: *((dis|en)able|todo) +)(.*)/.freeze

# The token that makes up a disable comment.
# The token used after `# rubocop: disable` are `A-z`, `/`, and `,`.
# Also `A-z` includes `all`.
DISABLING_COPS_CONTENT_TOKEN = %r{[A-z/,]+}.freeze
# The allowed specification for comments after `# rubocop: disable` is
# `DepartmentName/CopName` or` all`.
DISABLING_COPS_CONTENT_TOKEN = %r{[A-z]+/[A-z]+|all}.freeze

def investigate(processed_source)
processed_source.each_comment do |comment|
next if comment.text !~ DISABLE_COMMENT_FORMAT

offset = Regexp.last_match(1).length
Regexp.last_match(4).scan(%r{[\w/]+|\W+}) do |name|
break unless valid_content_token?(name.strip)

check_cop_name(name, comment, offset)
Regexp.last_match(4).scan(/[^,]+|[\W]+/) do |name|
trimmed_name = name.strip

break if contain_plain_comment?(trimmed_name)

unless valid_content_token?(trimmed_name)
check_cop_name(trimmed_name, comment, offset)
end

offset += name.length
end
Expand All @@ -47,16 +52,24 @@ def autocorrect(range)

private

def check_cop_name(name, comment, offset)
return if name !~ /^[A-Z]/ || name =~ %r{/}
def disable_comment_offset
Regexp.last_match(1).length
end

def check_cop_name(name, comment, offset)
start = comment.location.expression.begin_pos + offset
range = range_between(start, start + name.length)

add_offense(range, location: range)
end

def valid_content_token?(content_token)
!DISABLING_COPS_CONTENT_TOKEN.match(content_token).nil?
!/\W+/.match(content_token).nil? ||
!DISABLING_COPS_CONTENT_TOKEN.match(content_token).nil?
end

def contain_plain_comment?(name)
name == '#'
end

def qualified_legacy_cop_name(cop_name)
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/migration/department_name_spec.rb
Expand Up @@ -89,4 +89,16 @@
RUBY
end
end

# `Migration/DepartmentName` cop's role is to complement a department name.
# The role would be simple if another feature could detect unexpected
# disable comment format.
context 'when an unexpected disable comment format' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
# rubocop:disable Style:Alias
alias :ala :bala
RUBY
end
end
end

0 comments on commit cf3a020

Please sign in to comment.