Skip to content

Fix line coverage of statements inside string interpolation and heredocs - #9682

Open
sferik wants to merge 9 commits into
jruby:masterfrom
sferik:eval-coverage
Open

sferik wants to merge 9 commits into
jruby:masterfrom
sferik:eval-coverage

Conversation

@sferik

@sferik sferik commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Coverage counted a statement holding a heredoc twice per execution when the heredoc's interpolation sat on a later line, and marked the interpolation's line as code although nothing ever counted there. A statement's coverage event is now emitted once, when the statement starts. The call site only refreshes the line for backtraces (determineIfWeNeedLineNumberForCall). Also, a lone statement inside #{} is not a line event. The string it is part of is, so the parser no longer leaves that line marked as code (uncoverLine). Several statements in one interpolation each remain a line event.

Coverage counted the statement holding a heredoc twice per execution when
the heredoc's interpolation sat on a later line, and marked that
interpolation's line as code although nothing ever counted there. MRI
reports the statement once and leaves the line nil. Two fixes:

- A statement that is a call re-emitted its coverage line event when it
  built the call itself, if building its receiver or arguments had moved
  the current line elsewhere (an interpolation on another line, statements
  inside parentheses, ...). A statement's coverage event is now emitted
  once, when the statement starts; the call site only refreshes the line
  for backtraces.

- A lone statement inside #{} is not a line event of its own (the string it
  is part of is), so the parser no longer leaves its line marked as code.
  Several statements in one interpolation each remain a line event, as in
  MRI.

Un-excludes test_eval from the MRI coverage tests.

@headius headius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling all @enebo's for a secondary review. This might be fine but it diverges from parse.y and I want to understand why.

// A lone statement in an interpolation is not a line event of its own (MRI); the
// string it is part of is. Several statements in one interpolation each remain one.
$6.unsetNewline();
p.uncoverLine(@6.start());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find equivalent logic in CRuby for this; the lines related only unset newline and have no additional logic:

https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/parse.y#L6088-L6089

I'd like to understand why we need this and CRuby does not appear to do the same.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRuby gets the same result from nd_unset_fl_newline alone because it decides which lines are coverable at compile time, from the newline flag. iseq_compile_each adds RUBY_EVENT_COVERAGE_LINE only to nodes that still have the flag:

https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/compile.c#L11045-L11052

and iseq_set_sequence sets a line's entry from nil to 0 only when that event lands on an instruction:

https://github.com/ruby/ruby/blob/b2ec82d4ea84f5526b3759923159a039bc5c1d3b/compile.c#L2713-L2720

So once the parser clears the flag, the line was never coverable.

JRuby marks coverable lines earlier, in the parser. newline_node sets the flag and marks the line at the same time. It has to happen there because method bodies are built lazily, so the IR can't mark lines for methods that never run. By the time this action clears the flag, the line is already marked 0, and it would show up as code that never ran where CRuby shows nil. The extra call undoes what newline_node did.

Your question also led me to a bug in that call. It set the line back to nil without checking anything, which wiped out marks left by other statements on the same line. It now only undoes the mark the lone statement added itself, and only if nothing else had marked that line (db35bd1).

CRuby also counts a statement on the line of the first instruction after its line event, not on the line where the statement starts. For example, x =\n y.size counts on line 2, and x = <<~EOS counts on the line of its first interpolation. JRuby treated this differently in several ways, so I pushed a few more commits:

  • dbf912c: a statement nested in another (such as one of several statements in #{}) no longer replaces the outer statement's pending line event.
  • 30039a8: a call built before a statement's first instruction no longer moves the statement's count to an unmarked line. Most multi-line statements used to read [1, 0, 0]. Backtrace lines don't change.
  • 814037b: multi-line array, hash, string, regexp and symbol literals get the line they start on.
  • 92a5b70: a new LineEvents helper finds each statement's first instruction. The parser, Coverage.line_stub and the IR builder all use it, so marked lines and counted lines agree.
  • a76bc29: the coverage array is no longer one line short when a heredoc ends the file.

@headius headius added this to the JRuby 10.1.3.0 milestone Sep 16, 2026
@headius

headius commented Sep 16, 2026

Copy link
Copy Markdown
Member

Let's get this in with the other coverage work in 10.1.3.0.

Setting the interpolation's line back to nil also wiped out a mark left by
another statement on that line, such as a statement in a block earlier on
the same line or the first of several statements in one interpolation.
Undo only the mark the lone statement's newline_node added, and only when
no earlier statement had already marked the line.
A statement's line event waits for the statement's first instruction. When
building its value reached a statement of its own first, such as one of
several statements in an interpolation, that statement's event replaced
the outer one, so coverage never counted the outer statement's line:
x = "#{\n  a = 1\n  a\n}" read [0, 1, 1, nil] where MRI reads [1, 1, 1, nil].
When coverage is on, emit the outer statement's pending event before the
inner one replaces it.

A begin statement now asks for its body's line, the line the parser marks
for it, so emitting its event does not count the begin keyword's line.
A statement's line event waits for its first instruction and then took the
line of whatever call had been built since, so o.b =\n  [].size counted on
the [].size line, a line the parser never marked: the results read
[1, 0, 0] instead of counting the statement. Emit the event on the line it
was requested for, then restore the call's line for backtraces, so
backtraces do not change.

As in MRI, a statement is a line event unless the last line event was on
the same line; calls built in between no longer hide one (the 1 in
if\n  [].size then 1 end was not counted). A pending coverage event is no
longer replaced by a call's backtrace line either.
An array literal took the line of its first element, a hash literal the
line of its first key, a string the line its contents end on (or, for
"" "..." concatenated, the line of the second string), a regexp without
interpolation the line its contents end on, and a dynamic symbol, regexp,
or backtick string the start of its contents, which for an empty
production is not a real position (nor is it for the elements of %W, %I
and %i lists, or a "string": label). MRI reports each on the line of its
opening token; use that line for all of them.
MRI counts a statement on the line of its first instruction, which is a
later line whenever evaluating the statement starts with a part on one:
x =\n  [].size counts on line 2, and so does x = <<~EOS whose body starts
with an interpolation (x = <<~EOS\n  #{1.to_s}\nEOS reads [nil, 1, nil]).
JRuby marked and counted the line the statement starts on.

LineEvents finds the node whose instructions come first, following
assignment values, call receivers, conditions, the first element of an
array or key of a hash that MRI does not build as one literal, the first
interpolation of a string that does not start with a literal part, and
so on. The parser marks its line, Coverage.line_stub reports it, and the
IR builder counts the statement on it. As in MRI, a statement inside
another that starts with the same instruction shares its line event,
and a statement is no line event when the last one started on its line.

Along the way:

- The newline flags the lexer puts on heredoc lines for dedenting are
  cleared after dedenting, so the IR builder no longer takes those lines
  for statements.
- An ensure body is built before the body it protects; its line events
  no longer count as the last ones when building the protected body.
- A lone conditional in an interpolation stays a line event, as MRI
  counts its branches.

Against MRI 3.4, Coverage.line_stub now matches for 749 of the 840
files in lib/ruby/stdlib (510 before), with no line that matched before
differing now.
After reading a heredoc the lexer goes back to the line the heredoc
started on, and only skips past the heredoc when it reads the next line.
When the heredoc ends the file there is no next line, so the coverage
array stopped short: x = <<~EOS\n  #{1.to_s}\nEOS read [nil, 1] instead of
[nil, 1, nil]. Size it by the last line read instead.
@sferik

sferik commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Let's get this in with the other coverage work in 10.1.3.0.

Sounds good! I also have a branch for branch-coverage that is based on the method-coverage branch, as well as some fixes to oneshot and multiline line coverage. Once all these are merged, we should be able to completely delete test/mri/excludes/TestCoverage.rb.

If we can ship all of that in 10.1.3.0, I think it will be the biggest change to JRuby’s Coverage support since it was introduced!

An undef took the line of the first name it undefines, so with that name
on the next line, line coverage counted undef\n  a on the second line
instead of the first, where MRI counts it.
MRI parses the arms of a ternary as statements, so each is a line event
of its own: in x = y.size ?\n  1 : 2, the arm on the second line is counted
by line coverage and reported to line tracing. JRuby parsed them as plain
expressions, so that line read nil.
@sferik

sferik commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

I pushed two more commits:

  • 741d5e4: an undef statement now takes the line of its undef keyword, not the line of the first name it undefines. Before, undef\n a was counted on the second line. In x = 1; undef\n a it was also counted as a separate line event, where CRuby merges it with the other statement on the first line.
  • 6f7ba4d: each arm of a ternary is now a line event, as in CRuby. In x = y.size ?\n 1 : 2, the second line used to read nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants