Merge pull request #605 from odlp/master

Fix relevant lines for unloaded files

features/config_tracked_files.feature

@@ -16,7 +16,7 @@ Feature:
     When I open the coverage report generated with `bundle exec rake test`
     Then I should see the groups:
       | name      | coverage | files |
-      | All Files | 76.81%   | 7     |
+      | All Files | 77.94%   | 7     |
 
     And I should see the source files:
       | name                                    | coverage |

features/config_tracked_files_relevant_lines.feature

@@ -0,0 +1,31 @@
+@rspec
+Feature:
+
+  Using the setting `tracked_files` should classify whether lines
+  are relevant or not (such as whitespace or comments).
+
+  Scenario:
+    Given SimpleCov for RSpec is configured with:
+      """
+      require 'simplecov'
+      SimpleCov.start do
+        track_files "lib/**/*.rb"
+      end
+      """
+    Given a file named "lib/not_loaded.rb" with:
+    """
+    # A comment line. Plus a whitespace line below:
+
+    # :nocov:
+    def ignore_me
+    end
+    # :nocov:
+
+    def this_is_relevant
+      puts "still relevant"
+    end
+    """
+
+    When I open the coverage report generated with `bundle exec rspec spec`
+    Then I follow "lib/not_loaded.rb"
+    Then I should see "3 relevant lines" within ".highlighted"

lib/simplecov.rb

@@ -55,15 +55,15 @@ def start(profile = nil, &block)
 
     #
     # Finds files that were to be tracked but were not loaded and initializes
-    # their coverage to zero.
+    # the line-by-line coverage to zero (if relevant) or nil (comments / whitespace etc).
     #
     def add_not_loaded_files(result)
       if tracked_files
         result = result.dup
         Dir[tracked_files].each do |file|
           absolute = File.expand_path(file)
 
-          result[absolute] ||= [0] * File.foreach(absolute).count
+          result[absolute] ||= LinesClassifier.new.classify(File.foreach(absolute))
         end
       end
 
@@ -177,6 +177,7 @@ def clear_result
 require "simplecov/filter"
 require "simplecov/formatter"
 require "simplecov/last_run"
+require "simplecov/lines_classifier"
 require "simplecov/raw_coverage"
 require "simplecov/result_merger"
 require "simplecov/command_guesser"

lib/simplecov/lines_classifier.rb

@@ -0,0 +1,32 @@
+module SimpleCov
+  # Classifies whether lines are relevant for code coverage analysis.
+  # Comments & whitespace lines, and :nocov: token blocks, are considered not relevant.
+
+  class LinesClassifier
+    RELEVANT = 0
+    NOT_RELEVANT = nil
+
+    WHITESPACE_LINE = /^\s*$/
+    COMMENT_LINE = /^\s*#/
+    WHITESPACE_OR_COMMENT_LINE = Regexp.union(WHITESPACE_LINE, COMMENT_LINE)
+
+    def self.no_cov_line
+      /^(\s*)#(\s*)(\:#{SimpleCov.nocov_token}\:)/
+    end
+
+    def classify(lines)
+      skipping = false
+
+      lines.map do |line|
+        if line =~ self.class.no_cov_line
+          skipping = !skipping
+          NOT_RELEVANT
+        elsif skipping || line =~ WHITESPACE_OR_COMMENT_LINE
+          NOT_RELEVANT
+        else
+          RELEVANT
+        end
+      end
+    end
+  end
+end

lib/simplecov/source_file.rb

@@ -180,7 +180,7 @@ def process_skipped_lines(lines)
       skipping = false
 
       lines.each do |line|
-        if line.src =~ /^([\s]*)#([\s]*)(\:#{SimpleCov.nocov_token}\:)/
+        if line.src =~ SimpleCov::LinesClassifier.no_cov_line
           skipping = !skipping
           line.skipped!
         elsif skipping

spec/lines_classifier_spec.rb

@@ -0,0 +1,103 @@
+require "helper"
+require "simplecov/lines_classifier"
+
+describe SimpleCov::LinesClassifier do
+  describe "#classify" do
+    describe "relevant lines" do
+      it "determines code as relevant" do
+        classified_lines = subject.classify [
+          "module Foo",
+          "  class Baz",
+          "    def Bar",
+          "      puts 'hi'",
+          "    end",
+          "  end",
+          "end",
+        ]
+
+        expect(classified_lines.length).to eq 7
+        expect(classified_lines).to all be_relevant
+      end
+    end
+
+    describe "not-relevant lines" do
+      it "determines whitespace is not-relevant" do
+        classified_lines = subject.classify [
+          "",
+          "  ",
+          "\t\t",
+        ]
+
+        expect(classified_lines.length).to eq 3
+        expect(classified_lines).to all be_irrelevant
+      end
+
+      describe "comments" do
+        it "determines comments are not-relevant" do
+          classified_lines = subject.classify [
+            "#Comment",
+            " # Leading space comment",
+            "\t# Leading tab comment",
+          ]
+
+          expect(classified_lines.length).to eq 3
+          expect(classified_lines).to all be_irrelevant
+        end
+
+        it "doesn't mistake interpolation as a comment" do
+          classified_lines = subject.classify [
+            'puts "#{var}"',
+          ]
+
+          expect(classified_lines.length).to eq 1
+          expect(classified_lines).to all be_relevant
+        end
+      end
+
+      describe ":nocov: blocks" do
+        it "determines :nocov: blocks are not-relevant" do
+          classified_lines = subject.classify [
+            "# :nocov:",
+            "def hi",
+            "end",
+            "# :nocov:",
+          ]
+
+          expect(classified_lines.length).to eq 4
+          expect(classified_lines).to all be_irrelevant
+        end
+
+        it "determines all lines after a non-closing :nocov: as not-relevant" do
+          classified_lines = subject.classify [
+            "# :nocov:",
+            "puts 'Not relevant'",
+            "# :nocov:",
+            "puts 'Relevant again'",
+            "puts 'Still relevant'",
+            "# :nocov:",
+            "puts 'Not relevant till the end'",
+            "puts 'Ditto'",
+          ]
+
+          expect(classified_lines.length).to eq 8
+
+          expect(classified_lines[0..2]).to all be_irrelevant
+          expect(classified_lines[3..4]).to all be_relevant
+          expect(classified_lines[5..7]).to all be_irrelevant
+        end
+      end
+    end
+  end
+
+  RSpec::Matchers.define :be_relevant do
+    match do |actual|
+      actual == SimpleCov::LinesClassifier::RELEVANT
+    end
+  end
+
+  RSpec::Matchers.define :be_irrelevant do
+    match do |actual|
+      actual == SimpleCov::LinesClassifier::NOT_RELEVANT
+    end
+  end
+end