Refactor CommentConfig

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

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

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