Drop a simulated file's branches when a real run covered it

SimulateCoverage backfills branches and methods for tracked-but-unloaded
files by static analysis, and the merge unions those tuples into the real
ones by position. If a synthesized tuple's location drifts from what
Coverage emits (the class of bug behind the elsif, safe-navigation, and
Ruby 3.3 fixes), the drifted tuple has no real counterpart to combine
with and survives the merge as a phantom, permanently-missed branch on a
file that was actually fully covered.

Reconcile at the point two file coverages combine: when exactly one side
was actually executed (has a covered line, which a simulated file never
does), treat its branch and method tuples as authoritative and drop the
other side's. This contains any future extractor drift to denominator
inflation for files no process loaded, rather than a false miss on a
covered file. Lines still combine from both sides, so a simulated file
keeps contributing the unloaded-file line denominator.

When both sides ran, or neither did, nothing changes: the tuples combine
by union as before.

Author: sferik

lib/simplecov/combine/files_combiner.rb

@@ -9,22 +9,57 @@ module Combine
     module FilesCombiner
     module_function
 
+      # Branch/method tuples drawn from a simulated (never-loaded) file
+      # when the other side of the merge was actually executed.
+      NO_SYNTHESIZED = {"branches" => {}.freeze, "methods" => {}.freeze}.freeze
+
       #
       # Combines the results for 2 coverages of a file.
       #
       # @return [Hash]
       #
       def combine(coverage_a, coverage_b)
+        source_a, source_b = reconcile_synthesized(coverage_a, coverage_b)
+
         combination = {"lines" => Combine.combine(LinesCombiner, coverage_a["lines"], coverage_b["lines"])}
         if SimpleCov.branch_coverage?
-          combined_branches = Combine.combine(BranchesCombiner, coverage_a["branches"], coverage_b["branches"])
+          combined_branches = Combine.combine(BranchesCombiner, source_a["branches"], source_b["branches"])
           combination["branches"] = combined_branches || {}
         end
         if SimpleCov.method_coverage?
-          combination["methods"] = Combine.combine(MethodsCombiner, coverage_a["methods"], coverage_b["methods"])
+          combination["methods"] = Combine.combine(MethodsCombiner, source_a["methods"], source_b["methods"])
         end
         combination
       end
+
+      # When exactly one side of the merge was actually executed, its branch
+      # and method tuples are authoritative and the other side's are dropped.
+      # A simulated entry (SimulateCoverage backfills tracked-but-unloaded
+      # files) synthesizes those tuples statically, so a location that drifts
+      # from what Coverage emits would otherwise be unioned in by position
+      # and survive as a phantom, permanently-missed branch (see #1233). This
+      # contains any such drift to denominator inflation for files no process
+      # loaded, rather than a false miss on a covered file. Lines are never
+      # dropped: a simulated file's line shape is correct and carries the
+      # unloaded-file denominator (#1059).
+      #
+      # Returns the two coverages to draw branch/method tuples from, blanking
+      # the non-executed side only when the other side was executed. When
+      # both sides were executed (two real runs) or neither was (all
+      # simulated), both are returned unchanged and combine normally.
+      def reconcile_synthesized(coverage_a, coverage_b)
+        executed_a = executed?(coverage_a)
+        executed_b = executed?(coverage_b)
+        return [coverage_a, coverage_b] if executed_a == executed_b
+
+        executed_a ? [coverage_a, NO_SYNTHESIZED] : [NO_SYNTHESIZED, coverage_b]
+      end
+
+      # A file some process actually loaded has at least one executed line;
+      # a simulated (never-loaded) file's lines are all `nil` or `0`.
+      def executed?(coverage)
+        Array(coverage["lines"]).any? { |count| count&.positive? }
+      end
     end
   end
 end

spec/combine/files_combiner_spec.rb

@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+require "helper"
+
+RSpec.describe SimpleCov::Combine::FilesCombiner do
+  # A real, executed file: some line ran, and its branch tuples come from
+  # Coverage, so they sit at the exact positions Coverage reports.
+  let(:executed) do
+    {
+      "lines" => [nil, 1, 1, 0, nil],
+      "branches" => {
+        [:if, 0, 2, 2, 4, 10] => {[:then, 1, 3, 4, 3, 10] => 1, [:else, 2, 4, 4, 4, 10] => 0}
+      }
+    }
+  end
+
+  # A simulated (tracked-but-never-loaded) file: every line is nil / 0, and
+  # its branch tuples are synthesized. Here the `if` condition's end column
+  # has drifted (…4, 12] vs …4, 10]), the exact failure mode of #1233.
+  let(:simulated_drifted) do
+    {
+      "lines" => [nil, 0, 0, 0, nil],
+      "branches" => {
+        [:if, 0, 2, 2, 4, 12] => {[:then, 1, 3, 4, 3, 10] => 0, [:else, 2, 4, 4, 4, 12] => 0}
+      }
+    }
+  end
+
+  around do |example|
+    SimpleCov.enable_coverage(:branch)
+    example.run
+    SimpleCov.clear_coverage_criteria
+  end
+
+  describe ".combine", if: SimpleCov.branch_coverage_supported? do
+    it "drops a simulated file's branches when the other side was executed" do
+      combined = described_class.combine(executed, simulated_drifted)
+
+      # Only the executed side's real tuple survives — the drifted one would
+      # otherwise be a phantom, permanently-missed branch after merge.
+      expect(combined["branches"].keys).to eq([[:if, 0, 2, 2, 4, 10]])
+    end
+
+    it "is order-independent (simulated first)" do
+      combined = described_class.combine(simulated_drifted, executed)
+
+      expect(combined["branches"].keys).to eq([[:if, 0, 2, 2, 4, 10]])
+    end
+
+    it "still merges the lines from the simulated side" do
+      combined = described_class.combine(executed, simulated_drifted)
+
+      # Line shape is authoritative on both sides, so lines combine as usual
+      # (the simulated side contributes its zeros / relevance).
+      expect(combined["lines"]).to eq([nil, 1, 1, 0, nil])
+    end
+
+    it "keeps both branch sets when neither side was executed" do
+      # Two simulated copies of a never-loaded file: no real data exists, so
+      # its branches still count toward the denominator (#1059). If their
+      # tuples happen to disagree, both survive — denominator inflation, the
+      # acceptable fallback, rather than a false miss on a covered file.
+      other = {
+        "lines" => [nil, 0, 0, 0, nil],
+        "branches" => {[:if, 0, 2, 2, 4, 20] => {[:then, 1, 3, 4, 3, 10] => 0, [:else, 2, 4, 4, 4, 20] => 0}}
+      }
+
+      combined = described_class.combine(simulated_drifted, other)
+
+      expect(combined["branches"].keys).to contain_exactly([:if, 0, 2, 2, 4, 12], [:if, 0, 2, 2, 4, 20])
+    end
+
+    it "unions two executed runs of the same file normally" do
+      other_run = {
+        "lines" => [nil, 1, 1, 1, nil],
+        "branches" => {
+          [:if, 0, 2, 2, 4, 10] => {[:then, 1, 3, 4, 3, 10] => 4, [:else, 2, 4, 4, 4, 10] => 5}
+        }
+      }
+
+      combined = described_class.combine(executed, other_run)
+      arms = combined["branches"][[:if, 0, 2, 2, 4, 10]]
+
+      expect(arms[[:then, 1, 3, 4, 3, 10]]).to eq(5)
+      expect(arms[[:else, 2, 4, 4, 4, 10]]).to eq(5)
+    end
+  end
+
+  describe ".combine method coverage", if: SimpleCov.method_coverage_supported? do
+    around do |example|
+      SimpleCov.enable_coverage(:method)
+      example.run
+      SimpleCov.clear_coverage_criteria
+    end
+
+    it "drops a simulated file's methods when the other side was executed" do
+      executed_methods = {
+        "lines" => [nil, 1, 1],
+        "methods" => {["Foo", :bar, 2, 2, 3, 5] => 1}
+      }
+      simulated_methods = {
+        "lines" => [nil, 0, 0],
+        "methods" => {["Foo", :bar, 2, 2, 3, 7] => 0} # drifted end column
+      }
+
+      combined = described_class.combine(executed_methods, simulated_methods)
+
+      expect(combined["methods"].keys).to eq([["Foo", :bar, 2, 2, 3, 5]])
+    end
+  end
+end

spec/combine/results_combiner_spec.rb

@@ -158,4 +158,36 @@
     expect(merged_result[source_fixture("sample.rb")]["lines"]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil])
     expect(merged_result[source_fixture("app/models/user.rb")]["lines"]).to eq([nil, 1, 1, 1, nil, nil, 1, 0, nil, nil])
   end
+
+  # End-to-end analogue of the #1233 collate scenario: one worker executed
+  # the file (real branches), another only tracked it (simulated branches
+  # whose tuple has drifted). The merge must keep the executed worker's
+  # tuple and drop the simulated one rather than carry a phantom miss.
+  describe "reconciling a simulated file against an executed one",
+           if: SimpleCov.branch_coverage_supported? do
+    around do |example|
+      SimpleCov.enable_coverage(:branch)
+      example.run
+      SimpleCov.clear_coverage_criteria
+    end
+
+    it "keeps only the executed worker's branch tuple" do
+      executed_worker = {
+        source_fixture("sample.rb") => {
+          "lines" => [nil, 1, 1, 0, nil],
+          "branches" => {[:if, 0, 2, 2, 4, 10] => {[:then, 1, 3, 4, 3, 10] => 1, [:else, 2, 4, 4, 4, 10] => 0}}
+        }
+      }
+      tracking_worker = {
+        source_fixture("sample.rb") => {
+          "lines" => [nil, 0, 0, 0, nil],
+          "branches" => {[:if, 0, 2, 2, 4, 12] => {[:then, 1, 3, 4, 3, 10] => 0, [:else, 2, 4, 4, 4, 12] => 0}}
+        }
+      }
+
+      combined = described_class.combine(executed_worker, tracking_worker)
+
+      expect(combined[source_fixture("sample.rb")]["branches"].keys).to eq([[:if, 0, 2, 2, 4, 10]])
+    end
+  end
 end