Let the HTML report show the branch type instead of +/-

I believe this is much more user friendly as opposed to the
thinking what is positive/negative (like in an unless ... else,
or case when else). This way people see immediately that the else
branch was/wasn't hit or the then branch or the body of a loop.

Specifically I think it also helps with us reporting else branches
that aren't even defined (for if/case) - when people see 0 hits
for an else branch I think that's easier to figure out than
0 hits for the "-" branch. There's less of a "what does -/+ mean?"
going on.

As a side effect, also removes some code for checking what's
positive that's also rather intricately tied to how branch
coverage hands out its IDs which I don't believe we should take
for granted forever.

Gemfile

@@ -3,10 +3,10 @@
 source "https://rubygems.org"
 
 # Uncomment this to use local copy of simplecov-html in development when checked out
-# gem "simplecov-html", :path => ::File.dirname(__FILE__) + "/../simplecov-html"
+# gem "simplecov-html", path: File.dirname(__FILE__) + "/../simplecov-html"
 
 # Uncomment this to use development version of html formatter from github
-gem "simplecov-html", github: "colszowka/simplecov-html"
+gem "simplecov-html", github: "colszowka/simplecov-html", branch: "branch-report-mentioning-branch-types"
 
 group :development do
   gem "aruba", github: "cucumber/aruba"

Gemfile.lock

@@ -1,6 +1,7 @@
 GIT
   remote: https://github.com/colszowka/simplecov-html.git
-  revision: 402bcacc3a9a7c1d94f343d5ee40aca98d10b5d9
+  revision: 8958bce3e9af51d406bfd5a7f95a74c180c14cf1
+  branch: branch-report-mentioning-branch-types
   specs:
     simplecov-html (0.11.0.beta2)
 

features/branch_coverage.feature

@@ -30,5 +30,5 @@ Feature:
     When I open the detailed view for "lib/faked_project/some_class.rb"
     Then I should see a line coverage summary of 12/15 for the file
     And I should see a branch coverage summary of 1/2 for the file
-    And I should see coverage branch data like: 1, +
-    And I should see coverage branch data like: 0, -
+    And I should see coverage branch data like "then: 1"
+    And I should see coverage branch data like "else: 0"

features/step_definitions/html_steps.rb

@@ -64,6 +64,6 @@
   expect(header_text).to eq file_path
 end
 
-Then /^I should see coverage branch data like: (\d+), ([+|-])$/ do |hit_count, pos_or_neg|
-  expect(find(".hits", visible: true, text: "#{hit_count}, #{pos_or_neg}")).to be_truthy
+Then /^I should see coverage branch data like "(.+)"$/ do |text|
+  expect(find(".hits", visible: true, text: text)).to be_truthy
 end

lib/simplecov/source_file.rb

@@ -210,41 +210,26 @@ def build_branches_from(condition, branches)
       #     [:then, 4, 6, 6, 6, 10]
       #
       # which is [type, id, start_line, start_col, end_line, end_col]
-      _condition_type, condition_id, condition_start_line, * = restore_ruby_data_structure(condition)
+      _condition_type, _condition_id, condition_start_line, * = restore_ruby_data_structure(condition)
 
       branches.map do |branch_data, hit_count|
         branch_data = restore_ruby_data_structure(branch_data)
-        build_branch(branch_data, hit_count, condition_start_line, condition_id)
+        build_branch(branch_data, hit_count, condition_start_line)
       end
     end
 
-    def build_branch(branch_data, hit_count, condition_start_line, condition_id)
-      type, id, start_line, _start_col, end_line, _end_col = branch_data
+    def build_branch(branch_data, hit_count, condition_start_line)
+      type, _id, start_line, _start_col, end_line, _end_col = branch_data
 
       SourceFile::Branch.new(
         start_line: start_line,
         end_line:   end_line,
         coverage:   hit_count,
         inline:     start_line == condition_start_line,
-        positive:   positive_branch?(condition_id, id, type)
+        type:       type
       )
     end
 
-    #
-    # Branch is positive or negative.
-    # For `case` conditions, `when` always supposed as positive branch.
-    # For `if, else` conditions:
-    # coverage returns matrices ex: [:if, 0,..] => {[:then, 1,..], [:else, 2,..]},
-    # positive branch always has id equals to condition id incremented by 1.
-    #
-    # @return [Boolean]
-    #
-    def positive_branch?(condition_id, branch_id, branch_type)
-      return true if branch_type == :when
-
-      branch_id == (1 + condition_id)
-    end
-
     #
     # Select the covered branches
     # Here we user tree schema because some conditions like case may have additional
@@ -277,7 +262,7 @@ def branches_for_line(line_number)
     # @return [Boolean]
     #
     def line_with_missed_branch?(line_number)
-      branches_for_line(line_number).select { |count, _sign| count.zero? }.any?
+      branches_for_line(line_number).select { |_type, count| count.zero? }.any?
     end
 
     #

lib/simplecov/source_file/branch.rb

@@ -6,15 +6,15 @@ class SourceFile
     # Representing single branch that has been detected in coverage report.
     # Give us support methods that handle needed calculations.
     class Branch
-      attr_reader :start_line, :end_line, :coverage
+      attr_reader :start_line, :end_line, :coverage, :type
 
       # rubocop:disable Metrics/ParameterLists
-      def initialize(start_line:, end_line:, coverage:, inline:, positive:)
+      def initialize(start_line:, end_line:, coverage:, inline:, type:)
         @start_line = start_line
         @end_line   = end_line
         @coverage   = coverage
         @inline     = inline
-        @positive   = positive
+        @type       = type
         @skipped    = false
       end
       # rubocop:enable Metrics/ParameterLists
@@ -23,19 +23,6 @@ def inline?
         @inline
       end
 
-      def positive?
-        @positive
-      end
-
-      #
-      # Branch is negative
-      #
-      # @return [Boolean]
-      #
-      def negative?
-        !positive?
-      end
-
       #
       # Return true if there is relevant count defined > 0
       #
@@ -54,15 +41,6 @@ def missed?
         !skipped? && coverage.zero?
       end
 
-      #
-      # Return the sign depends on branch is positive or negative
-      #
-      # @return [String]
-      #
-      def badge
-        positive? ? "+" : "-"
-      end
-
       # The line on which we want to report the coverage
       #
       # Usually we choose the line above the start of the branch (so that it shows up
@@ -99,7 +77,7 @@ def overlaps_with?(line_range)
       # @return [Array]
       #
       def report
-        [coverage, badge]
+        [type, coverage]
       end
     end
   end

spec/source_file/branch_spec.rb

@@ -3,32 +3,24 @@
 require "helper"
 
 describe SimpleCov::SourceFile::Branch do
-  let(:positive_branch) do
-    described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, positive: true)
+  let(:if_branch) do
+    described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, type: :then)
   end
 
-  let(:negative_branch) do
-    described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, positive: false)
+  let(:else_branch) do
+    described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, type: :else)
   end
 
   context "a source branch if..else" do
-    it "has positive badge of positive branch" do
-      expect(positive_branch.badge).to eq "+"
-    end
-
-    it "has negative badge of negative branch" do
-      expect(negative_branch.badge).to eq "-"
-    end
-
-    it "corrects report branch report" do
-      expect(positive_branch.report).to eq([0, "+"])
-      expect(negative_branch.report).to eq([0, "-"])
+    it "correct branch report" do
+      expect(if_branch.report).to eq([:then, 0])
+      expect(else_branch.report).to eq([:else, 0])
     end
   end
 
   context "A source branch with coverage" do
     let(:covered_branch) do
-      described_class.new(start_line: 1, end_line: 3, coverage: 1, inline: false, positive: true)
+      described_class.new(start_line: 1, end_line: 3, coverage: 1, inline: false, type: :then)
     end
 
     it "is covered" do
@@ -48,7 +40,7 @@
 
   context "a source branch without coverage" do
     let(:uncovered_branch) do
-      described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, positive: true)
+      described_class.new(start_line: 1, end_line: 3, coverage: 0, inline: false, type: :then)
     end
 
     it "isn't covered" do
@@ -67,7 +59,7 @@
   end
 
   describe "skipping lines" do
-    subject { described_class.new(start_line: 5, end_line: 7, coverage: 0, inline: false, positive: true) }
+    subject { described_class.new(start_line: 5, end_line: 7, coverage: 0, inline: false, type: :then) }
 
     it "isn't skipped by default" do
       expect(subject).not_to be_skipped
@@ -80,7 +72,7 @@
   end
 
   describe "#overlaps_with?(range)" do
-    subject { described_class.new(start_line: 5, end_line: 7, coverage: 0, inline: false, positive: true) }
+    subject { described_class.new(start_line: 5, end_line: 7, coverage: 0, inline: false, type: :then) }
 
     it "doesn't overlap with a range beyond its lines" do
       expect(subject.overlaps_with?(8..10)).to eq false

spec/source_file_spec.rb

@@ -128,10 +128,10 @@
 
       it "has coverage report" do
         expect(subject.branches_report).to eq(
-          3 => [[0, "+"], [1, "-"]],
-          5 => [[1, "+"], [0, "-"]],
-          7 => [[0, "+"]],
-          9 => [[1, "-"]]
+          3 => [[:then, 0], [:else, 1]],
+          5 => [[:then, 1], [:else, 0]],
+          7 => [[:then, 0]],
+          9 => [[:else, 1]]
         )
       end
 
@@ -207,7 +207,7 @@
     end
 
     it "has dual element in condition at line 3 report" do
-      expect(subject.branches_report[3]).to eq([[1, "+"], [0, "-"]])
+      expect(subject.branches_report[3]).to eq([[:then, 1], [:else, 0]])
     end
 
     it "has branches coverage precent 50.00" do
@@ -370,9 +370,9 @@
     describe "branch coverage" do
       it "has an empty branch report" do
         expect(subject.branches_report).to eq(
-          9 => [[1, "-"]],
-          13 => [[1, "+"], [0, "-"]],
-          22 => [[1, "+"]]
+          9 => [[:else, 1]],
+          13 => [[:then, 1], [:else, 0]],
+          22 => [[:when, 1]]
         )
       end
 
@@ -415,7 +415,7 @@
       end
 
       it "registered 2 hits for the while branch" do
-        expect(subject.branches_report[7]).to eq [[2, "+"]]
+        expect(subject.branches_report[7]).to eq [[:body, 2]]
       end
     end
   end
@@ -454,10 +454,10 @@
 
       it "covers all the things right" do
         expect(subject.branches_report).to eq(
-          4 => [[0, "+"]],
-          6 => [[1, "+"]],
-          8 => [[0, "+"]],
-          10 => [[0, "-"]]
+          4 => [[:when, 0]],
+          6 => [[:when, 1]],
+          8 => [[:when, 0]],
+          10 => [[:else, 0]]
         )
       end
     end
@@ -496,15 +496,15 @@
       end
 
       it "marks the non declared else branch as missing at the point of the case" do
-        expect(subject.branches_for_line(3)).to eq [[0, "-"]]
+        expect(subject.branches_for_line(3)).to eq [[:else, 0]]
       end
 
       it "covers the branch that includes 42" do
         expect(subject.branches_report).to eq(
-          3 => [[0, "-"]],
-          4 => [[0, "+"]],
-          6 => [[1, "+"]],
-          8 => [[0, "+"]]
+          3 => [[:else, 0]],
+          4 => [[:when, 0]],
+          6 => [[:when, 1]],
+          8 => [[:when, 0]]
         )
       end
     end
@@ -543,7 +543,7 @@
       end
 
       it "covers the branch that includes 42" do
-        expect(subject.branches_report[7]).to eq [[1, "+"]]
+        expect(subject.branches_report[7]).to eq [[:then, 1]]
       end
     end
   end
@@ -597,7 +597,7 @@
       end
 
       it "notifies us of the missing else branch on line 27 that's hit" do
-        expect(subject.branches_report[27]).to eq [[0, "+"], [1, "-"]]
+        expect(subject.branches_report[27]).to eq [[:then, 0], [:else, 1]]
       end
     end
   end