Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
headius merged 13 commits into from
Oct 2, 2026
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
61e7053
Fix line coverage of statements inside string interpolation and heredocs
sferik Sep 14, 2026
23e7904
Only uncover a line the lone interpolated statement marked itself
sferik Sep 16, 2026
efcfd21
Count a statement whose value holds statements of its own
sferik Sep 16, 2026
b8097c3
Keep a statement's coverage count on the statement's line
sferik Sep 16, 2026
3c5d9d4
Give multi-line literals the line they start on
sferik Sep 16, 2026
50150b4
Count line coverage on the line of a statement's first instruction
sferik Sep 16, 2026
f7cb395
Size line coverage past a heredoc that ends the file
sferik Sep 16, 2026
16e0223
Give an undef statement the line of its undef keyword
sferik Sep 16, 2026
84a62b7
Make each arm of a ternary a line event
sferik Sep 16, 2026
bb9bcfe
Run the MRI oneshot_lines coverage test
sferik Sep 14, 2026
281bcb2
Give Coverage.line_stub an entry for every line of the file
sferik Oct 1, 2026
9ad177e
Keep IRBuilder's line coverage state in a LineNumberInfo
sferik Oct 2, 2026
b22bd97
Use CRuby's helpers for node locations in the grammar
sferik Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Use CRuby's helpers for node locations in the grammar 
The grammar set where undef statements, list and hash literals and
strings start, and unmarked the lone statement of an interpolation,
inline. Those rules now read as CRuby's do, calling nd_set_first_loc,
make_list, nd_set_loc and nd_unset_fl_newline, which RubyParserBase
implements for our nodes. nd_set_first_loc was an empty stub until now,
so the find and hash patterns that call it (Const(...) and Const[...])
now start at their constant, as in CRuby.

Both parsers were regenerated with jay.
  • Loading branch information
@sferik
sferik committed Oct 2, 2026
commit b22bd97d34460b25faeba1337d7351cd22d24ca7
46 changes: 11 additions & 35 deletions core/src/main/java/org/jruby/parser/RubyParser.java
View file
Open in desktop
Comment thread
headius marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -2207,10 +2207,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
};
states[30] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
/* The statement starts at undef, not at the first name it undefines*/
if (((Node)yyVals[0+yyTop].value) instanceof BlockNode block) block.get(0).setLine(yyVals[yyTop - count + 1].start());
((Node)yyVals[0+yyTop].value).setLine(yyVals[yyTop - count + 1].start());
yyVal = ((Node)yyVals[0+yyTop].value);
yyVal = p.nd_set_first_loc(((Node)yyVals[0+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: undef!($2) %*/
return yyVal;
Expand Down Expand Up @@ -4162,22 +4159,15 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
};
states[359] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
Integer position = yyVals[yyTop - count + 1].start();
if (((Node)yyVals[-1+yyTop].value) == null) {
yyVal = new ZArrayNode(position); /* zero length array */
} else {
yyVal = ((Node)yyVals[-1+yyTop].value);
((Node)yyVal).setLine(position); /* where it starts, not where its first element is */
}
yyVal = p.make_list(((Node)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: array!(escape_Qundef($2)) %*/
return yyVal;
};
states[360] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
yyVal = ((HashNode)yyVals[-1+yyTop].value);
yyVal = p.nd_set_loc(((HashNode)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
((HashNode)yyVal).setIsLiteral();
((HashNode)yyVal).setLine(yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: hash!(escape_Qundef($2)) %*/
return yyVal;
Expand Down Expand Up @@ -5763,7 +5753,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
states[633] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
p.heredoc_dedent(((Node)yyVals[-1+yyTop].value));
if (((Node)yyVals[-1+yyTop].value) != null) ((Node)yyVals[-1+yyTop].value).setLine(yyVals[yyTop - count + 1].start()); /* where it starts, not where its contents end */
if (((Node)yyVals[-1+yyTop].value) != null) p.nd_set_loc(((Node)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
yyVal = ((Node)yyVals[-1+yyTop].value);
/*% %*/
/*% ripper: string_literal!(heredoc_dedent(p, $2)) %*/
Expand Down Expand Up @@ -5799,8 +5789,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
};
states[638] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
yyVal = ((ListNode)yyVals[-1+yyTop].value);
((Node)yyVal).setLine(yyVals[yyTop - count + 1].start());
yyVal = p.make_list(((ListNode)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: array!($3) %*/
return yyVal;
Expand Down Expand Up @@ -5833,8 +5822,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
};
states[643] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
yyVal = ((ListNode)yyVals[-1+yyTop].value);
((Node)yyVal).setLine(yyVals[yyTop - count + 1].start());
yyVal = p.make_list(((ListNode)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: array!($3) %*/
return yyVal;
Expand All @@ -5855,16 +5843,14 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
};
states[646] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
yyVal = ((ListNode)yyVals[-1+yyTop].value);
((Node)yyVal).setLine(yyVals[yyTop - count + 1].start());
yyVal = p.make_list(((ListNode)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: array!($3) %*/
return yyVal;
};
states[647] = (RubyParser p, Object yyVal, ProductionState[] yyVals, int yyTop, int count, int yychar) -> {
/*%%%*/
yyVal = ((ListNode)yyVals[-1+yyTop].value);
((Node)yyVal).setLine(yyVals[yyTop - count + 1].start());
yyVal = p.make_list(((ListNode)yyVals[-1+yyTop].value), yyVals[yyTop - count + 1].start());
/*% %*/
/*% ripper: array!($3) %*/
return yyVal;
Expand Down Expand Up @@ -5996,17 +5982,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
p.setHeredocLineIndent(-1);

/*%%%*/
if (((Node)yyVals[-1+yyTop].value) != null) {
/* 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, and so*/
/* does a lone conditional, as MRI counts its branches.*/
/* MRI's compiler marks coverable lines from the newline flag, but newline_node marked*/
/* this one already, so undo that too (it was the last line newline_node marked).*/
if (!(((Node)yyVals[-1+yyTop].value) instanceof IfNode)) {
if (((Node)yyVals[-1+yyTop].value).isNewline()) p.uncoverLastLine();
((Node)yyVals[-1+yyTop].value).unsetNewline();
}
}
if (((Node)yyVals[-1+yyTop].value) != null) p.nd_unset_fl_newline(((Node)yyVals[-1+yyTop].value));
yyVal = p.newEvStrNode(yyVals[yyTop - count + 6].start(), ((Node)yyVals[-1+yyTop].value));
/*% %*/
/*% ripper: string_embexpr!($6) %*/
Expand Down Expand Up @@ -6929,7 +6905,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
return yyVal;
};
}
// line 4925 "parse.y"
// line 4901 "parse.y"

}
// line 15267 "-"
// line 15243 "-"
42 changes: 9 additions & 33 deletions core/src/main/java/org/jruby/parser/RubyParser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,10 +620,7 @@ stmt : keyword_alias fitem {
}
| keyword_undef undef_list {
/*%%%*/
// The statement starts at undef, not at the first name it undefines
if ($2 instanceof BlockNode block) block.get(0).setLine(@1.start());
$2.setLine(@1.start());
$$ = $2;
$$ = p.nd_set_first_loc($2, @1.start());
/*% %*/
/*% ripper: undef!($2) %*/
}
Expand Down Expand Up @@ -2361,21 +2358,14 @@ primary : literal
}
| tLBRACK aref_args ']' {
/*%%%*/
Integer position = @1.start();
if ($2 == null) {
$$ = new ZArrayNode(position); /* zero length array */
} else {
$$ = $2;
$<Node>$.setLine(position); /* where it starts, not where its first element is */
}
$$ = p.make_list($2, @1.start());
/*% %*/
/*% ripper: array!(escape_Qundef($2)) %*/
}
| tLBRACE assoc_list '}' {
/*%%%*/
$$ = $2;
$$ = p.nd_set_loc($2, @1.start());
$<HashNode>$.setIsLiteral();
$<HashNode>$.setLine(@1.start());
/*% %*/
/*% ripper: hash!(escape_Qundef($2)) %*/
}
Expand Down Expand Up @@ -3864,7 +3854,7 @@ string : tCHAR {
string1 : tSTRING_BEG string_contents tSTRING_END {
/*%%%*/
p.heredoc_dedent($2);
if ($2 != null) $2.setLine(@1.start()); /* where it starts, not where its contents end */
if ($2 != null) p.nd_set_loc($2, @1.start());
$$ = $2;
/*% %*/
/*% ripper: string_literal!(heredoc_dedent(p, $2)) %*/
Expand Down Expand Up @@ -3903,8 +3893,7 @@ words_sep : ' ' {
// [!null] - ListNode
words : tWORDS_BEG words_sep word_list tSTRING_END {
/*%%%*/
$$ = $3;
$<Node>$.setLine(@1.start());
$$ = p.make_list($3, @1.start());
/*% %*/
/*% ripper: array!($3) %*/
};
Expand Down Expand Up @@ -3938,8 +3927,7 @@ word : string_content {

symbols : tSYMBOLS_BEG words_sep symbol_list tSTRING_END {
/*%%%*/
$$ = $3;
$<Node>$.setLine(@1.start());
$$ = p.make_list($3, @1.start());
/*% %*/
/*% ripper: array!($3) %*/
};
Expand All @@ -3960,17 +3948,15 @@ symbol_list : /* none */ {
// [!null] - ListNode
qwords : tQWORDS_BEG words_sep qword_list tSTRING_END {
/*%%%*/
$$ = $3;
$<Node>$.setLine(@1.start());
$$ = p.make_list($3, @1.start());
/*% %*/
/*% ripper: array!($3) %*/
};

// [!null] - ListNode
qsymbols : tQSYMBOLS_BEG words_sep qsym_list tSTRING_END {
/*%%%*/
$$ = $3;
$<Node>$.setLine(@1.start());
$$ = p.make_list($3, @1.start());
/*% %*/
/*% ripper: array!($3) %*/
};
Expand Down Expand Up @@ -4091,17 +4077,7 @@ string_content : tSTRING_CONTENT {
p.setHeredocLineIndent(-1);

/*%%%*/
if ($6 != null) {
// 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, and so
// does a lone conditional, as MRI counts its branches.
// MRI's compiler marks coverable lines from the newline flag, but newline_node marked
// this one already, so undo that too (it was the last line newline_node marked).
if (!($6 instanceof IfNode)) {
if ($6.isNewline()) p.uncoverLastLine();
$6.unsetNewline();
}
}
if ($6 != null) p.nd_unset_fl_newline($6);
$$ = p.newEvStrNode(@6.start(), $6);
/*% %*/
/*% ripper: string_embexpr!($6) %*/
Expand Down
51 changes: 47 additions & 4 deletions core/src/main/java/org/jruby/parser/RubyParserBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,53 @@ public Node newline_node(Node node, int line) {
return node;
}

/**
* CRuby's nd_set_first_loc: the node starts at line. Our nodes only have a line, so that is what moves. An
* undef_list is a BlockNode of UndefNodes here (one NODE_UNDEF in CRuby), so its first statement starts there
* too.
*/
public Node nd_set_first_loc(Node node, int line) {
if (node instanceof BlockNode block) block.get(0).setLine(line);
node.setLine(line);

return node;
}

/**
* CRuby's nd_set_loc, for the line: the node starts at line.
*/
public Node nd_set_loc(Node node, int line) {
node.setLine(line);

return node;
}

/**
* CRuby's make_list, for the line: a list literal starts at line, not at its first element.
*/
public Node make_list(Node list, int line) {
if (list == null) return new ZArrayNode(line);

list.setLine(line);

return list;
}

/**
* CRuby's nd_unset_fl_newline, for the statements of a string interpolation: a lone statement there is not a
* line event of its own; the string it is part of is. Several statements each remain one, and so does a lone
* conditional, as MRI counts its branches.
*
* CRuby's compiler finds coverable lines from the newline flag, but JRuby marks them as it parses, and
* newline_node marked this statement's line already (the last line it marked), so that is undone too.
*/
public void nd_unset_fl_newline(Node node) {
if (node instanceof IfNode) return;

if (node.isNewline()) uncoverLastLine();
node.unsetNewline();
}

// This is the last node made in the AST unintuitively so so post-processing can occur here.
public Node addRootNode(Node topOfAST) {
int line;
Expand Down Expand Up @@ -2475,10 +2522,6 @@ public Node remove_begin(Node node) {
return node;
}

public void nd_set_first_loc(Node node, int line) {
// FIXME: IMPL
}

public RubyParserResult parse() throws IOException {
yyparse(lexer, runtime.getInstanceConfig().isDebug() ? new YYDebug() : null);

Expand Down
Loading