From 98ff6844284ddeac017a5fc77733b80d97c3c097 Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Wed, 17 Mar 2021 15:27:37 +0300 Subject: [PATCH] Refactor CommentConfig --- lib/rubocop/comment_config.rb | 102 ++++++++++--------------- lib/rubocop/directive_comment.rb | 10 +++ spec/rubocop/directive_comment_spec.rb | 27 +++++++ 3 files changed, 79 insertions(+), 60 deletions(-) diff --git a/lib/rubocop/comment_config.rb b/lib/rubocop/comment_config.rb index c1bd6db86df..c29e62914af 100644 --- a/lib/rubocop/comment_config.rb +++ b/lib/rubocop/comment_config.rb @@ -40,13 +40,13 @@ def comment_only_line?(line_number) private def extra_enabled_comments_with_names(extras:, names:) - each_directive do |comment, cop_names, disabled| - next unless comment_only_line?(comment.loc.expression.line) + each_directive do |directive| + next unless comment_only_line?(directive.line_number) - if !disabled && enable_all?(comment) - handle_enable_all(names, extras, comment) + if directive.enabled_all? + handle_enable_all(directive, names, extras) else - handle_switch(cop_names, names, disabled, extras, comment) + handle_switch(directive, names, extras) end end @@ -56,9 +56,11 @@ def extra_enabled_comments_with_names(extras:, names:) def analyze # rubocop:todo Metrics/AbcSize analyses = Hash.new { |hash, key| hash[key] = CopAnalysis.new([], nil) } - each_mentioned_cop do |cop_name, disabled, line, single_line| - analyses[cop_name] = - analyze_cop(analyses[cop_name], disabled, line, single_line) + each_directive do |directive| + directive.cop_names.each do |cop_name| + cop_name = qualified_cop_name(cop_name) + analyses[cop_name] = analyze_cop(analyses[cop_name], directive) + end end analyses.each_with_object({}) do |element, hash| @@ -67,37 +69,42 @@ def analyze # rubocop:todo Metrics/AbcSize end end - def analyze_cop(analysis, disabled, line, single_line) - if single_line - analyze_single_line(analysis, line, disabled) - elsif disabled - analyze_disabled(analysis, line) + def analyze_cop(analysis, directive) + # Disabling cops after comments like `#=SomeDslDirective` does not related to single line + if !comment_only_line?(directive.line_number) || directive.single_line? + analyze_single_line(analysis, directive) + elsif directive.disabled? + analyze_disabled(analysis, directive) else - analyze_rest(analysis, line) + analyze_rest(analysis, directive) end end - def analyze_single_line(analysis, line, disabled) - return analysis unless disabled + def analyze_single_line(analysis, directive) + return analysis unless directive.disabled? + + line = directive.line_number + start_line = analysis.start_line_number - CopAnalysis.new(analysis.line_ranges + [(line..line)], - analysis.start_line_number) + CopAnalysis.new(analysis.line_ranges + [(line..line)], start_line) end - def analyze_disabled(analysis, line) - if (start_line = analysis.start_line_number) - # Cop already disabled on this line, so we end the current disabled - # range before we start a new range. - return CopAnalysis.new(analysis.line_ranges + [start_line..line], line) - end + def analyze_disabled(analysis, directive) + line = directive.line_number + start_line = analysis.start_line_number + + # Cop already disabled on this line, so we end the current disabled + # range before we start a new range. + return CopAnalysis.new(analysis.line_ranges + [start_line..line], line) if start_line CopAnalysis.new(analysis.line_ranges, line) end - def analyze_rest(analysis, line) - if (start_line = analysis.start_line_number) - return CopAnalysis.new(analysis.line_ranges + [start_line..line], nil) - end + def analyze_rest(analysis, directive) + line = directive.line_number + start_line = analysis.start_line_number + + return CopAnalysis.new(analysis.line_ranges + [start_line..line], nil) if start_line CopAnalysis.new(analysis.line_ranges, nil) end @@ -108,30 +115,10 @@ def cop_line_ranges(analysis) analysis.line_ranges + [(analysis.start_line_number..Float::INFINITY)] end - def each_mentioned_cop - each_directive do |comment, cop_names, disabled| - comment_line_number = comment.loc.expression.line - single_line = !comment_only_line?(comment_line_number) || - directive_on_comment_line?(comment) - - cop_names.each do |cop_name| - yield qualified_cop_name(cop_name), disabled, comment_line_number, - single_line - end - end - end - - def directive_on_comment_line?(comment) - DirectiveComment.new(comment).single_line? - end - def each_directive processed_source.comments.each do |comment| directive = DirectiveComment.new(comment) - next unless directive.cop_names - - # TODO: pass only directive - yield directive.comment, directive.cop_names, directive.disabled? + yield directive if directive.cop_names end end @@ -146,12 +133,7 @@ def non_comment_token_line_numbers end end - def enable_all?(comment) - _, cops = DirectiveComment.new(comment).match_captures - cops == 'all' - end - - def handle_enable_all(names, extras, comment) + def handle_enable_all(directive, names, extras) enabled_cops = 0 names.each do |name, counter| next unless counter.positive? @@ -160,19 +142,19 @@ def handle_enable_all(names, extras, comment) enabled_cops += 1 end - extras[comment] << 'all' if enabled_cops.zero? + extras[directive.comment] << 'all' if enabled_cops.zero? end # Collect cops that have been disabled or enabled by name in a directive comment # so that `Lint/RedundantCopEnableDirective` can register offenses correctly. - def handle_switch(cop_names, names, disabled, extras, comment) - cop_names.each do |name| - if disabled + def handle_switch(directive, names, extras) + directive.cop_names.each do |name| + if directive.disabled? names[name] += 1 elsif (names[name]).positive? names[name] -= 1 else - extras[comment] << name + extras[directive.comment] << name end end end diff --git a/lib/rubocop/directive_comment.rb b/lib/rubocop/directive_comment.rb index 486b485aedc..6b55d7f08f1 100644 --- a/lib/rubocop/directive_comment.rb +++ b/lib/rubocop/directive_comment.rb @@ -54,6 +54,11 @@ def disabled? %w[disable todo].include?(mode) end + # Checks if this directive enables all cops + def enabled_all? + !disabled? && all_cops? + end + # Checks if all cops specified in this directive def all_cops? cops == 'all' @@ -64,6 +69,11 @@ def cop_names @cop_names ||= all_cops? ? all_cop_names : parsed_cop_names end + # Returns line number for directive + def line_number + comment.loc.expression.line + end + private def parsed_cop_names diff --git a/spec/rubocop/directive_comment_spec.rb b/spec/rubocop/directive_comment_spec.rb index 7ca994be332..7e16870c729 100644 --- a/spec/rubocop/directive_comment_spec.rb +++ b/spec/rubocop/directive_comment_spec.rb @@ -171,4 +171,31 @@ end end end + + describe '#line_number' do + let(:comment) { instance_double(Parser::Source::Comment, text: text, loc: loc) } + let(:loc) { instance_double(Parser::Source::Map, expression: expression) } + let(:expression) { instance_double(Parser::Source::Range, line: 1) } + + it 'returns line number for directive' do + expect(directive_comment.line_number).to be 1 + end + end + + describe '#enabled_all?' do + subject { directive_comment.enabled_all? } + + [ + ['when enabled all cops', 'def foo # rubocop:enable all', true], + ['when enabled specific cops', '# rubocop:enable Foo/Bar', false], + ['when disabled all cops', '# rubocop:disable all', false], + ['when disabled specific cops', '# rubocop:disable Foo/Bar', false] + ].each do |example| + context example[0] do + let(:text) { example[1] } + + it { is_expected.to eq example[2] } + end + end + end end