Skip to content

Commit

Permalink
[Fix #9519] Make it possible to disable an entire department with a d…
Browse files Browse the repository at this point in the history
…irective comment (#9626)

Co-authored-by: Daniel Vandersluis <daniel.vandersluis@gmail.com>
  • Loading branch information
Andrei Eres and dvandersluis committed Jun 14, 2021
1 parent 2815917 commit f313e6f
Show file tree
Hide file tree
Showing 12 changed files with 788 additions and 200 deletions.
@@ -0,0 +1 @@
* [#9626](https://github.com/rubocop/rubocop/pull/9626): Disable all cop department with directive comment. ([@AndreiEres][])
9 changes: 9 additions & 0 deletions docs/modules/ROOT/pages/configuration.adoc
Expand Up @@ -671,6 +671,15 @@ file by adding a comment such as
# rubocop:enable Layout/LineLength, Style/StringLiterals
----

You can also disable entire departments by giving a department name in the comment.

[source,ruby]
----
# rubocop:disable Metrics, Layout/LineLength
[...]
# rubocop:enable Metrics, Layout/LineLength
----

You can also disable _all_ cops with

[source,ruby]
Expand Down
49 changes: 32 additions & 17 deletions lib/rubocop/cop/lint/missing_cop_enable_directive.rb
Expand Up @@ -45,37 +45,52 @@ module Lint
class MissingCopEnableDirective < Base
include RangeHelp

MSG = 'Re-enable %<cop>s cop with `# rubocop:enable` after disabling it.'
MSG_BOUND = 'Re-enable %<cop>s cop within %<max_range>s lines after disabling it.'
MSG = 'Re-enable %<cop>s %<type>s with `# rubocop:enable` after disabling it.'
MSG_BOUND = 'Re-enable %<cop>s %<type>s within %<max_range>s lines after disabling it.'

# rubocop:disable Metrics/AbcSize
def on_new_investigation
max_range = cop_config['MaximumRangeSize']
processed_source.disabled_line_ranges.each do |cop, line_ranges|
line_ranges.each do |line_range|
# This has to remain a strict inequality to handle
# the case when max_range is Float::INFINITY
next if line_range.max - line_range.min < max_range + 2
each_missing_enable do |cop, line_range|
# This has to remain a strict inequality to handle
# the case when max_range is Float::INFINITY
next if line_range.max - line_range.min < max_range + 2

range = source_range(processed_source.buffer, line_range.min, (0..0))
range = source_range(processed_source.buffer, line_range.min, (0..0))
comment = processed_source.comment_at_line(line_range.begin)

add_offense(range, message: message(max_range: max_range, cop: cop))
end
add_offense(range, message: message(cop, comment))
end
end
# rubocop:enable Metrics/AbcSize

private

def message(max_range:, cop:)
def each_missing_enable
processed_source.disabled_line_ranges.each do |cop, line_ranges|
line_ranges.each { |line_range| yield cop, line_range }
end
end

def max_range
@max_range ||= cop_config['MaximumRangeSize']
end

def message(cop, comment, type = 'cop')
if department_enabled?(cop, comment)
type = 'department'
cop = cop.split('/').first
end

if max_range == Float::INFINITY
format(MSG, cop: cop)
format(MSG, cop: cop, type: type)
else
format(MSG_BOUND, cop: cop, max_range: max_range)
format(MSG_BOUND, cop: cop, type: type, max_range: max_range)
end
end

def department_enabled?(cop, comment)
DirectiveComment.new(comment).in_directive_department?(cop)
end
end
end
end
end
# rubocop:enable Lint/RedundantCopDisableDirective, Layout/SpaceAroundOperators
# rubocop:enable Lint/RedundantCopDisableDirective
158 changes: 93 additions & 65 deletions lib/rubocop/cop/lint/redundant_cop_disable_directive.rb
Expand Up @@ -25,11 +25,12 @@ module Lint
#
# # good
# x += 1
class RedundantCopDisableDirective < Base
class RedundantCopDisableDirective < Base # rubocop:todo Metrics/ClassLength
include RangeHelp
extend AutoCorrector

COP_NAME = 'Lint/RedundantCopDisableDirective'
DEPARTMENT_MARKER = 'DEPARTMENT'

attr_accessor :offenses_to_check

Expand All @@ -41,12 +42,9 @@ def initialize(config = nil, options = nil, offenses = nil)
def on_new_investigation
return unless offenses_to_check

cop_disabled_line_ranges = processed_source.disabled_line_ranges

redundant_cops = Hash.new { |h, k| h[k] = Set.new }

each_redundant_disable(cop_disabled_line_ranges,
offenses_to_check) do |comment, redundant_cop|
each_redundant_disable do |comment, redundant_cop|
redundant_cops[comment].add(redundant_cop)
end

Expand All @@ -56,6 +54,14 @@ def on_new_investigation

private

def cop_disabled_line_ranges
processed_source.disabled_line_ranges
end

def disabled_ranges
cop_disabled_line_ranges[COP_NAME] || [0..0]
end

def previous_line_blank?(range)
processed_source.buffer.source_line(range.line - 1).blank?
end
Expand Down Expand Up @@ -97,32 +103,34 @@ def directive_range_in_list(range, ranges)
range_with_surrounding_space(range: range, side: :right, newlines: false)
end

def each_redundant_disable(cop_disabled_line_ranges, offenses,
&block)
disabled_ranges = cop_disabled_line_ranges[COP_NAME] || [0..0]

def each_redundant_disable(&block)
cop_disabled_line_ranges.each do |cop, line_ranges|
each_already_disabled(line_ranges, disabled_ranges) { |comment| yield comment, cop }

each_line_range(line_ranges, disabled_ranges, offenses, cop, &block)
each_already_disabled(cop, line_ranges, &block)
each_line_range(cop, line_ranges, &block)
end
end

def each_line_range(line_ranges, disabled_ranges, offenses,
cop)
line_ranges.each_with_index do |line_range, ix|
comment = processed_source.comment_at_line(line_range.begin)
next if ignore_offense?(disabled_ranges, line_range)
def each_line_range(cop, line_ranges)
line_ranges.each_with_index do |line_range, line_range_index|
next if ignore_offense?(line_range)

redundant_cop = find_redundant(comment, offenses, cop, line_range, line_ranges[ix + 1])
yield comment, redundant_cop if redundant_cop
comment = processed_source.comment_at_line(line_range.begin)
redundant = if all_disabled?(comment)
find_redundant_all(line_range, line_ranges[line_range_index + 1])
elsif department_disabled?(cop, comment)
find_redundant_department(cop, line_range)
else
find_redundant_cop(cop, line_range)
end

yield comment, redundant if redundant
end
end

def each_already_disabled(line_ranges, disabled_ranges)
def each_already_disabled(cop, line_ranges)
line_ranges.each_cons(2) do |previous_range, range|
next if ignore_offense?(disabled_ranges, range)
next if previous_range.end != range.begin
next if ignore_offense?(range)
next unless followed_ranges?(previous_range, range)

# If a cop is disabled in a range that begins on the same line as
# the end of the previous range, it means that the cop was
Expand All @@ -133,42 +141,56 @@ def each_already_disabled(line_ranges, disabled_ranges)
# Comments disabling all cops don't count since it's reasonable
# to disable a few select cops first and then all cops further
# down in the code.
yield comment if comment && !all_disabled?(comment)
yield comment, cop if comment && !all_disabled?(comment)
end
end

# rubocop:todo Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def find_redundant(comment, offenses, cop, line_range, next_line_range)
if all_disabled?(comment)
# If there's a disable all comment followed by a comment
# specifically disabling `cop`, we don't report the `all`
# comment. If the disable all comment is truly redundant, we will
# detect that when examining the comments of another cop, and we
# get the full line range for the disable all.
if (next_line_range.nil? || line_range.last != next_line_range.first) &&
offenses.none? { |o| line_range.cover?(o.line) }
'all'
end
else
cop_offenses = offenses.select { |o| o.cop_name == cop }
cop if cop_offenses.none? { |o| line_range.cover?(o.line) }
end
def find_redundant_cop(cop, range)
cop_offenses = offenses_to_check.select { |offense| offense.cop_name == cop }
cop if range_with_offense?(range, cop_offenses)
end

def find_redundant_all(range, next_range)
# If there's a disable all comment followed by a comment
# specifically disabling `cop`, we don't report the `all`
# comment. If the disable all comment is truly redundant, we will
# detect that when examining the comments of another cop, and we
# get the full line range for the disable all.
has_no_next_range = next_range.nil? || !followed_ranges?(range, next_range)
'all' if has_no_next_range && range_with_offense?(range)
end

def find_redundant_department(cop, range)
department = cop.split('/').first
offenses = offenses_to_check.select { |offense| offense.cop_name.start_with?(department) }
add_department_marker(department) if range_with_offense?(range, offenses)
end

def followed_ranges?(range, next_range)
range.end == next_range.begin
end

def range_with_offense?(range, offenses = offenses_to_check)
offenses.none? { |offense| range.cover?(offense.line) }
end
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

def all_disabled?(comment)
/rubocop\s*:\s*(?:disable|todo)\s+all\b/.match?(comment.text)
DirectiveComment.new(comment).disabled_all?
end

def ignore_offense?(disabled_ranges, line_range)
def ignore_offense?(line_range)
disabled_ranges.any? do |range|
range.cover?(line_range.min) && range.cover?(line_range.max)
end
end

def department_disabled?(cop, comment)
directive = DirectiveComment.new(comment)
directive.in_directive_department?(cop) && !directive.overridden_by_department?(cop)
end

def directive_count(comment)
_, cops_string = DirectiveComment.new(comment).match_captures
cops_string.split(/,\s*/).size
DirectiveComment.new(comment).directive_count
end

def add_offenses(redundant_cops)
Expand All @@ -183,12 +205,9 @@ def add_offenses(redundant_cops)

def add_offense_for_entire_comment(comment, cops)
location = DirectiveComment.new(comment).range
cop_list = cops.sort.map { |c| describe(c) }
cop_names = cops.sort.map { |c| describe(c) }.join(', ')

add_offense(
location,
message: "Unnecessary disabling of #{cop_list.join(', ')}."
) do |corrector|
add_offense(location, message: message(cop_names)) do |corrector|
range = comment_range_with_surrounding_space(location, comment.loc.expression)
corrector.remove(range)
end
Expand All @@ -200,17 +219,16 @@ def add_offense_for_some_cops(comment, cops)
ranges = cop_ranges.map { |_, r| r }

cop_ranges.each do |cop, range|
add_offense(
range,
message: "Unnecessary disabling of #{describe(cop)}."
) do |corrector|
cop_name = describe(cop)
add_offense(range, message: message(cop_name)) do |corrector|
range = directive_range_in_list(range, ranges)
corrector.remove(range)
end
end
end

def cop_range(comment, cop)
cop = remove_department_marker(cop)
matching_range(comment.loc.expression, cop) ||
matching_range(comment.loc.expression, Badge.parse(cop).cop_name) ||
raise("Couldn't find #{cop} in comment: #{comment.text}")
Expand All @@ -233,18 +251,16 @@ def trailing_range?(ranges, range)
end

def describe(cop)
if cop == 'all'
'all cops'
elsif all_cop_names.include?(cop)
"`#{cop}`"
else
similar = NameSimilarity.find_similar_name(cop, all_cop_names)
if similar
"`#{cop}` (did you mean `#{similar}`?)"
else
"`#{cop}` (unknown cop)"
end
end
return 'all cops' if cop == 'all'
return "`#{remove_department_marker(cop)}` department" if department_marker?(cop)
return "`#{cop}`" if all_cop_names.include?(cop)

similar = NameSimilarity.find_similar_name(cop, all_cop_names)
similar ? "`#{cop}` (did you mean `#{similar}`?)" : "`#{cop}` (unknown cop)"
end

def message(cop_names)
"Unnecessary disabling of #{cop_names}."
end

def all_cop_names
Expand All @@ -255,6 +271,18 @@ def ends_its_line?(range)
line = range.source_buffer.source_line(range.last_line)
(line =~ /\s*\z/) == range.last_column
end

def department_marker?(department)
department.start_with?(DEPARTMENT_MARKER)
end

def remove_department_marker(department)
department.gsub(DEPARTMENT_MARKER, '')
end

def add_department_marker(department)
DEPARTMENT_MARKER + department
end
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/cop/lint/redundant_cop_enable_directive.rb
Expand Up @@ -54,6 +54,7 @@ def register_offense(comment, cop_names)
directive = DirectiveComment.new(comment)

cop_names.each do |name|
name = name.split('/').first if department?(directive, name)
add_offense(
range_of_offense(comment, name),
message: format(MSG, cop: all_or_name(name))
Expand Down Expand Up @@ -119,6 +120,10 @@ def range_with_comma_after(comment, start, begin_pos, end_pos)
def all_or_name(name)
name == 'all' ? 'all cops' : name
end

def department?(directive, name)
directive.in_directive_department?(name) && !directive.overridden_by_department?(name)
end
end
end
end
Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/cop/migration/department_name.rb
Expand Up @@ -61,7 +61,9 @@ def check_cop_name(name, comment, offset)
end

def valid_content_token?(content_token)
/\W+/.match?(content_token) || DISABLING_COPS_CONTENT_TOKEN.match?(content_token)
/\W+/.match?(content_token) ||
DISABLING_COPS_CONTENT_TOKEN.match?(content_token) ||
Registry.global.department?(content_token)
end

def contain_unexpected_character_for_department_name?(name)
Expand Down

0 comments on commit f313e6f

Please sign in to comment.