Skip to content

Commit

Permalink
[prism/compiler] end_cursor should never be NULL
Browse files Browse the repository at this point in the history
This fixes a failed assertion reported to SimpleCov

simplecov-ruby/simplecov#1113

This can be repro'd as follows:

1. Create a file `test.rb` containing the following code

```
@foo&.(@bar)
```

2. require it with branch coverage enabled

```
ruby -rcoverage -e "Coverage.start(branches: true); require_relative 'test.rb'"
```

The assertion is failing because the Prism compiler is incorrectly
detecting the start and end cursor position of the call site for the
implicit call .()

This patch replicates the parse.y behaviour of setting the default
end_cursor to be the final closing location of the call node.

This behaviour can be verified against `parse.y` by modifying the test
command as follows:

```
ruby --parser=parse.y -rcoverage -e "Coverage.start(branches: true); require_relative 'test.rb'"
```

[Bug #20866]
  • Loading branch information
eightbitraptor committed Nov 20, 2024
1 parent 811dfa7 commit bb6b3b4
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
1 change: 1 addition & 0 deletions prism_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -3630,6 +3630,7 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c
const uint8_t *end_cursor = cursors[0];
end_cursor = (end_cursor == NULL || cursors[1] == NULL) ? cursors[1] : (end_cursor > cursors[1] ? end_cursor : cursors[1]);
end_cursor = (end_cursor == NULL || cursors[2] == NULL) ? cursors[2] : (end_cursor > cursors[2] ? end_cursor : cursors[2]);
if (!end_cursor) end_cursor = call_node->closing_loc.end;

const pm_line_column_t start_location = PM_NODE_START_LINE_COLUMN(scope_node->parser, call_node);
const pm_line_column_t end_location = pm_newline_list_line_column(&scope_node->parser->newline_list, end_cursor, scope_node->parser->start_line);
Expand Down
4 changes: 4 additions & 0 deletions test/coverage/test_coverage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ def test_branch_coverage_for_safe_method_invocation
[:"&.", 3, 7, 0, 7, 6] => {[:then, 4, 7, 0, 7, 6]=>0, [:else, 5, 7, 0, 7, 6]=>1},
[:"&.", 6, 8, 0, 8, 10] => {[:then, 7, 8, 0, 8, 10]=>1, [:else, 8, 8, 0, 8, 10]=>0},
[:"&.", 9, 9, 0, 9, 10] => {[:then, 10, 9, 0, 9, 10]=>0, [:else, 11, 9, 0, 9, 10]=>1},
[:"&.", 12, 10, 0, 10, 6] => {[:then, 13, 10, 0, 10, 6] => 0, [:else, 14, 10, 0, 10, 6] => 1},
[:"&.", 15, 11, 0, 11, 5] => {[:then, 16, 11, 0, 11, 5] => 0, [:else, 17, 11, 0, 11, 5] => 1},
}
}
assert_coverage(<<~"end;", { branches: true }, result)
Expand All @@ -475,6 +477,8 @@ class Dummy; def foo; end; def foo=(x); end; end
b&.foo
c&.foo = 1
d&.foo = 1
d&.(b)
d&.()
end;
end

Expand Down

0 comments on commit bb6b3b4

Please sign in to comment.