Skip to content

Chained safe-navigation followed by a block emits a phantom branch tuple that doesn't match Coverage, creating an unhittable missed branch on merge #1233

Description

@alexdeng-mp

Summary

This is the same class of bug as #1226 (fixed in 1.0.1 for elsif), but for a different construct that isn't covered by that fix: a safe-navigation chain (a&.b&.c) where the final call takes a block.

SimulateCoverage backfills branch data for tracked-but-unloaded files using StaticCoverageExtractor (Prism). For the second (or later) &. in a chain when it's immediately followed by a block, the synthesized tuple's end position extends through the entire block, whereas Ruby's Coverage ends the tuple at the method call itself. Since BranchesCombiner keys arms on [type, start_line, start_col, end_line, end_col], merging a simulated resultset for a file with a real one for the same file does not combine that arm — it keeps both, and the synthesized one can never be hit.

Reproduction

require "coverage"
require "simplecov"
require "simplecov/static_coverage_extractor"

path = "/tmp/chained_safe_nav_example.rb"
File.write(path, <<~RUBY)
  class Example
    def call(x)
      x&.foo&.bar { |y| y.baz }
    end
  end
RUBY

Coverage.start(lines: true, branches: true)
load path
Example.new.call(Struct.new(:foo).new(Struct.new(:bar).new))
real = Coverage.result[path][:branches]

synth = SimpleCov::StaticCoverageExtractor.call(File.read(path))["branches"]

zeroed = synth.transform_values { |a| a.transform_values { 0 } }
merged = SimpleCov::Combine::BranchesCombiner.combine(zeroed, real)

puts "REAL:"
real.each   { |k, arms| arms.each { |a, c| puts "  #{k.inspect} #{a.inspect} => #{c}" } }
puts "SYNTH:"
synth.each  { |k, arms| arms.each { |a, c| puts "  #{k.inspect} #{a.inspect} => #{c}" } }
puts "MERGED:"
merged.each { |k, arms| arms.each { |a, c| puts "  #{k.inspect} #{a.inspect} => #{c}" } }

Output (simplecov 1.0.1, ruby 3.4.9, prism 1.9.0)

REAL:
  [:"&.", 0, 3, 4, 3, 10] [:then, 1, 3, 4, 3, 10] => 1
  [:"&.", 0, 3, 4, 3, 10] [:else, 2, 3, 4, 3, 10] => 0
  [:"&.", 3, 3, 4, 3, 15] [:then, 4, 3, 4, 3, 15] => 1
  [:"&.", 3, 3, 4, 3, 15] [:else, 5, 3, 4, 3, 15] => 0
SYNTH:
  [:"&.", 0, 3, 4, 3, 29] [:then, 1, 3, 4, 3, 29] => 0
  [:"&.", 0, 3, 4, 3, 29] [:else, 2, 3, 4, 3, 29] => 0
  [:"&.", 3, 3, 4, 3, 10] [:then, 4, 3, 4, 3, 10] => 0
  [:"&.", 3, 3, 4, 3, 10] [:else, 5, 3, 4, 3, 10] => 0
MERGED:
  [:"&.", 0, 3, 4, 3, 29] [:then, 1, 3, 4, 3, 29] => 0   <== phantom, synthesized only
"/private/tmp/claude-501/-Users-alexdeng-projects-marketplacer/582258cc-8ec1-4c3a-b974-4cad8e284450/scratchpad/simplecov_issue.md" 138L, 8410B
### Suggested fix

The root cause is `Visitor#emit_safe_navigation` in `lib/simplecov/static_coverage_extractor/visitor.rb`, which uses the `CallNode`'s whole `location` for the branch tuple:

```ruby
def emit_safe_navigation(node)
  loc = node.location
  @branches[build_tuple(:"&.", loc)] = {
    build_tuple(:then, loc) => 0,
    build_tuple(:else, loc) => 0
  }
end

CallNode#location spans through an attached block when the call has one, but Coverage never includes the block in the branch's range — it stops at the end of the call's arguments (or the message, if there are none). I checked what Coverage actually uses across every combination of "has args" x "has parens" x "has block" and it's consistently: closing_loc (closing paren) if present, else arguments.location if present (paren-less args), else message_loc — the block is never part of it, regardless of whether one is attached:

def emit_safe_navigation(node)
  loc = safe_navigation_location(node)
  @branches[build_tuple(:"&.", loc)] = {
    build_tuple(:then, loc) => 0,
    build_tuple(:else, loc) => 0
  }
end

# Coverage's safe-navigation branch spans the receiver through the end of
# the call's arguments (or just the message when there are none), but
# never includes a trailing block: `x&.foo { ... }` and `x&.foo(1) { ... }`
# both end exactly where `x&.foo` / `x&.foo(1)` would without the block.
# `node.location` includes an attached block, so build the end position
# from `closing_loc` / `arguments` / `message_loc` instead.
def safe_navigation_location(node)
  end_loc = node.closing_loc || node.arguments&.location || node.message_loc
  node.location.join(end_loc)
end

I verified this produces byte-for-byte identical tuples to Coverage.result across every combination I could think of: bare (x&.foo), with parenthesized args (x&.foo(1)), with a block and no args (x&.foo { 1 }), with both args and a block (x&.foo(1) { 1 }), paren-less args (x&.foo 1), and chained with/without args ending in a block (x&.foo&.bar { 1 }, x&.foo(1)&.bar(2) { 1 }) — plus the real production line that originally surfaced this for us. Happy to open a PR with this plus regression specs if that's useful, but wanted to raise the issue with a proposed fix in hand either way.

Given elsif (#1226) and this both slipped through independently, it's probably worth an exhaustive audit of StaticCoverageExtractor against Coverage across every branch-producing node type Prism can emit (ideally property-tested — fuzzing small ASTs and diffing Coverage.result against StaticCoverageExtractor.call) rather than continuing to fix constructs one report at a time.

As a defensive second layer, it might also be worth having the merge reconcile a simulated entry against a real one for the same file rather than unioning arm tuples on the full [type, start_line, start_col, end_line, end_col] key — e.g. drop synthesized arms entirely for any file some process actually executed. That would contain the blast radius of the next undiscovered mismatch to "denominator inflation for files no process loaded" rather than "phantom missed branch on files that were fully covered."

Environment

  • simplecov 1.0.1
  • ruby 3.4.9
  • prism 1.9.0
  • Rails app, RSpec, ~120 parallel CI shards each merged via SimpleCov.collate, enable_coverage(:branch), track_files (now removed as a workaround)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions