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

Refactor comment access #8405

Merged
merged 9 commits into from Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/rubocop/comment_config.rb
Expand Up @@ -128,9 +128,7 @@ def directive_on_comment_line?(comment)
end

def each_directive
return if processed_source.comments.nil?

processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
directive = directive_parts(comment)
next unless directive

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/correctors/line_break_corrector.rb
Expand Up @@ -16,7 +16,7 @@ def correct_trailing_body(configured_width:, corrector:, node:,
processed_source:)
@processed_source = processed_source
range = first_part_of(node.to_a.last)
eol_comment = end_of_line_comment(node.source_range.line)
eol_comment = processed_source.comment_at_line(node.source_range.line)

break_line_before(range: range, node: node, corrector: corrector,
configured_width: configured_width)
Expand Down
10 changes: 3 additions & 7 deletions lib/rubocop/cop/layout/class_structure.rb
Expand Up @@ -269,15 +269,11 @@ def end_position_for(node)
end

def begin_pos_with_comment(node)
annotation_line = node.first_line - 1
first_comment = nil
(node.first_line - 1).downto(1) do |annotation_line|
break unless (comment = processed_source.comment_at_line(annotation_line))

processed_source.comments_before_line(annotation_line)
.reverse_each do |comment|
if comment.location.line == annotation_line
first_comment = comment
annotation_line -= 1
end
first_comment = comment
end

start_line_position(first_comment || node)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/comment_indentation.rb
Expand Up @@ -39,7 +39,7 @@ class CommentIndentation < Cop
'instead of %<correct_comment_indentation>d).'

def investigate(processed_source)
processed_source.each_comment { |comment| check(comment) }
processed_source.comments.each { |comment| check(comment) }
end

def autocorrect(comment)
Expand Down
35 changes: 14 additions & 21 deletions lib/rubocop/cop/layout/extra_spacing.rb
Expand Up @@ -40,6 +40,7 @@ class ExtraSpacing < Base
def on_new_investigation
return if processed_source.blank?

@aligned_comments = aligned_locations(processed_source.comments.map(&:loc))
@corrected = Set.new if force_equal_sign_alignment?

processed_source.tokens.each_cons(2) do |token1, token2|
Expand All @@ -49,6 +50,18 @@ def on_new_investigation

private

def aligned_locations(locs)
return [] if locs.empty?

aligned = Set[locs.first.line, locs.last.line]
locs.each_cons(3) do |before, loc, after|
col = loc.column
aligned << loc.line if col == before.column || # rubocop:disable Style/MultipleComparison
col == after.column
end
aligned
end

def check_tokens(ast, token1, token2)
return if token2.type == :tNL

Expand Down Expand Up @@ -95,7 +108,7 @@ def extra_space_range(token1, token2)

def aligned_tok?(token)
if token.comment?
aligned_comments?(token)
@aligned_comments.include?(token.line)
else
aligned_with_something?(token.pos)
end
Expand All @@ -119,26 +132,6 @@ def ignored_ranges(ast)
end.compact
end

def aligned_comments?(comment_token)
ix = processed_source.comments.index do |comment|
comment.loc.expression.begin_pos == comment_token.begin_pos
end
aligned_with_previous_comment?(ix) || aligned_with_next_comment?(ix)
end

def aligned_with_previous_comment?(index)
index.positive? && comment_column(index - 1) == comment_column(index)
end

def aligned_with_next_comment?(index)
index < processed_source.comments.length - 1 &&
comment_column(index + 1) == comment_column(index)
end

def comment_column(index)
processed_source.comments[index].loc.column
end

def force_equal_sign_alignment?
cop_config['ForceEqualSignAlignment']
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/leading_comment_space.rb
Expand Up @@ -55,7 +55,7 @@ class LeadingCommentSpace < Cop
MSG = 'Missing space after `#`.'

def investigate(processed_source)
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
next unless /\A#+[^#\s=:+-]/.match?(comment.text)
next if comment.loc.line == 1 && allowed_on_first_line?(comment)
next if doxygen_comment_style?(comment)
Expand Down
29 changes: 13 additions & 16 deletions lib/rubocop/cop/lint/redundant_cop_disable_directive.rb
Expand Up @@ -41,13 +41,12 @@ def initialize(config = nil, options = nil, offenses = nil)
def on_new_investigation
return unless offenses_to_check

comments = processed_source.comments
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, comments) do |comment, redundant_cop|
offenses_to_check) do |comment, redundant_cop|
redundant_cops[comment].add(redundant_cop)
end

Expand Down Expand Up @@ -88,25 +87,25 @@ def directive_range_in_list(range, ranges)
newlines: false)
end

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

cop_disabled_line_ranges.each do |cop, line_ranges|
each_already_disabled(line_ranges,
disabled_ranges, comments) do |comment|
disabled_ranges) do |comment|
yield comment, cop
end

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

def each_line_range(line_ranges, disabled_ranges, offenses, comments,
def each_line_range(line_ranges, disabled_ranges, offenses,
cop)
line_ranges.each_with_index do |line_range, ix|
comment = comments.find { |c| c.loc.line == line_range.begin }
comment = processed_source.comment_at_line(line_range.begin)
next if ignore_offense?(disabled_ranges, line_range)

redundant_cop = find_redundant(comment, offenses, cop, line_range,
Expand All @@ -115,7 +114,7 @@ def each_line_range(line_ranges, disabled_ranges, offenses, comments,
end
end

def each_already_disabled(line_ranges, disabled_ranges, comments)
def each_already_disabled(line_ranges, disabled_ranges)
line_ranges.each_cons(2) do |previous_range, range|
next if ignore_offense?(disabled_ranges, range)
next if previous_range.end != range.begin
Expand All @@ -124,14 +123,12 @@ def each_already_disabled(line_ranges, disabled_ranges, comments)
# the end of the previous range, it means that the cop was
# already disabled by an earlier comment. So it's redundant
# whether there are offenses or not.
redundant_comment = comments.find do |c|
c.loc.line == range.begin &&
# 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.
!all_disabled?(c)
end
yield redundant_comment if redundant_comment
comment = processed_source.comment_at_line(range.begin)

# 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)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/migration/department_name.rb
Expand Up @@ -20,7 +20,7 @@ class DepartmentName < Base
DISABLING_COPS_CONTENT_TOKEN = %r{[A-z]+/[A-z]+|all}.freeze

def on_new_investigation
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
next if comment.text !~ DISABLE_COMMENT_FORMAT

offset = Regexp.last_match(1).length
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/mixin/alignment.rb
Expand Up @@ -64,8 +64,9 @@ def within?(inner, outer)
inner.begin_pos >= outer.begin_pos && inner.end_pos <= outer.end_pos
end

# @deprecated Use processed_source.comment_at_line(line)
def end_of_line_comment(line)
processed_source.find_comment { |c| c.loc.line == line }
processed_source.line_with_comment?(line)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/check_line_breakable.rb
Expand Up @@ -59,7 +59,7 @@ def extract_breakable_node_from_elements(node, elements, max)
return if safe_to_ignore?(node)

line = processed_source.lines[node.first_line - 1]
return if processed_source.commented?(node.loc.begin)
return if processed_source.line_with_comment?(node.loc.line)
return if line.length <= max

extract_first_element_over_column_limit(node, elements, max)
Expand Down
4 changes: 1 addition & 3 deletions lib/rubocop/cop/mixin/line_length_help.rb
Expand Up @@ -12,9 +12,7 @@ def ignore_cop_directives?

def directive_on_source_line?(line_index)
source_line_number = line_index + processed_source.buffer.first_line
comment =
processed_source.comments
.detect { |e| e.location.line == source_line_number }
comment = processed_source.comment_at_line(source_line_number)

return false unless comment

Expand Down
3 changes: 1 addition & 2 deletions lib/rubocop/cop/mixin/multiline_literal_brace_layout.rb
Expand Up @@ -27,8 +27,7 @@ def new_line_needed_before_closing_brace?(node)
last_element_line =
last_element_range_with_trailing_comma(node).last_line

last_element_commented =
processed_source.comments.any? { |c| c.loc.line == last_element_line }
last_element_commented = processed_source.comment_at_line(last_element_line)

last_element_commented && (node.chained? || node.argument?)
end
Expand Down
8 changes: 2 additions & 6 deletions lib/rubocop/cop/mixin/percent_array.rb
Expand Up @@ -28,12 +28,8 @@ def message(_node)
end

def comments_in_array?(node)
comments = processed_source.comments
array_range = node.source_range.to_a

comments.any? do |comment|
!(comment.loc.expression.to_a & array_range).empty?
end
line_span = node.source_range.first_line...node.source_range.last_line
processed_source.each_comment_in_lines(line_span).any?
end

def check_percent_array(node)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/mixin/statement_modifier.rb
Expand Up @@ -19,14 +19,14 @@ def single_line_as_modifier?(node)
def non_eligible_node?(node)
node.modifier_form? ||
node.nonempty_line_count > 3 ||
processed_source.commented?(node.loc.end)
processed_source.line_with_comment?(node.loc.last_line)
end

def non_eligible_body?(body)
body.nil? ||
body.empty_source? ||
body.begin_type? ||
processed_source.commented?(body.source_range)
processed_source.contains_comment?(body.source_range)
end

def non_eligible_condition?(condition)
Expand Down
6 changes: 2 additions & 4 deletions lib/rubocop/cop/mixin/trailing_comma.rb
Expand Up @@ -74,10 +74,8 @@ def should_have_comma?(style, node)
end

def inside_comment?(range, comma_offset)
processed_source.comments.any? do |comment|
comment_offset = comment.loc.expression.begin_pos - range.begin_pos
comment_offset >= 0 && comment_offset < comma_offset
end
comment = processed_source.comment_at_line(range.line)
comment && comment.loc.expression.begin_pos < range.begin_pos + comma_offset
end

# Returns true if the node has round/square/curly brackets.
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/ascii_comments.rb
Expand Up @@ -21,7 +21,7 @@ class AsciiComments < Base
MSG = 'Use only ascii symbols in comments.'

def on_new_investigation
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
next if comment.text.ascii_only?
next if only_allowed_non_ascii_chars?(comment.text)

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/block_comments.rb
Expand Up @@ -25,7 +25,7 @@ class BlockComments < Base
END_LENGTH = "\n=end".length

def on_new_investigation
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
next unless comment.document?

add_offense(comment) do |corrector|
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/commented_keyword.rb
Expand Up @@ -38,7 +38,7 @@ class CommentedKeyword < Cop
'`%<keyword>s` keyword.'

def investigate(processed_source)
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
add_offense(comment) if offensive?(comment)
end
end
Expand Down
17 changes: 8 additions & 9 deletions lib/rubocop/cop/style/empty_case_condition.rb
Expand Up @@ -73,7 +73,7 @@ def correct_case_when(corrector, case_node, when_nodes)

corrector.replace(case_range, 'if')

keep_first_when_comment(case_node, when_nodes.first, corrector)
keep_first_when_comment(case_range, corrector)

when_nodes[1..-1].each do |when_node|
corrector.replace(when_node.loc.keyword, 'elsif')
Expand All @@ -93,15 +93,14 @@ def correct_when_conditions(corrector, when_nodes)
end
end

def keep_first_when_comment(case_node, first_when_node, corrector)
comment = processed_source.comments_before_line(
first_when_node.loc.keyword.line
).map(&:text).join("\n")
def keep_first_when_comment(case_range, corrector)
indent = ' ' * case_range.column
comments = processed_source.each_comment_in_lines(
case_range.first_line...case_range.last_line
).map { |comment| "#{indent}#{comment.text}\n" }.join

line = range_by_whole_lines(case_node.source_range)

corrector.insert_before(line, "#{comment}\n") if !comment.empty? &&
!case_node.parent
line_beginning = case_range.adjust(begin_pos: -case_range.column)
corrector.insert_before(line_beginning, comments)
end
end
end
Expand Down
13 changes: 4 additions & 9 deletions lib/rubocop/cop/style/empty_else.rb
Expand Up @@ -138,21 +138,16 @@ def nil_check(node)

def autocorrect(corrector, node)
return false if autocorrect_forbidden?(node.type.to_s)
return false if comment_in_else?(node)
return false if comment_in_else?(node.loc)

end_pos = base_node(node).loc.end.begin_pos
corrector.remove(range_between(node.loc.else.begin_pos, end_pos))
end

def comment_in_else?(node)
range = else_line_range(node.loc)
processed_source.find_comment { |c| range.include?(c.loc.line) }
end

def else_line_range(loc)
return 0..0 if loc.else.nil? || loc.end.nil?
def comment_in_else?(loc)
return false if loc.else.nil? || loc.end.nil?

loc.else.first_line..loc.end.first_line
processed_source.contains_comment?(loc.else.join(loc.end))
end

def base_node(node)
Expand Down
5 changes: 1 addition & 4 deletions lib/rubocop/cop/style/if_unless_modifier.rb
Expand Up @@ -159,10 +159,7 @@ def to_normal_form(node)
end

def first_line_comment(node)
comment =
processed_source.find_comment { |c| c.loc.line == node.loc.line }

comment ? comment.loc.expression.source : nil
processed_source.comment_at_line(node.loc.line)&.text
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/inline_comment.rb
Expand Up @@ -21,7 +21,7 @@ class InlineComment < Base
MSG = 'Avoid trailing inline comments.'

def on_new_investigation
processed_source.each_comment do |comment|
processed_source.comments.each do |comment|
next if comment_line?(processed_source[comment.loc.line - 1]) ||
comment.text.match?(/\A# rubocop:(enable|disable)/)

Expand Down