Skip to content

Commit

Permalink
[ruby/prism] Fix incompatibility AST for regexp match in `Prism::Tran…
Browse files Browse the repository at this point in the history
…slation::Parser`

This PR fixes the following incompatibility AST for regexp match between Parser gem and Prism:

## Parser gem

Returns an `match_with_lvasgn` node:

```console
$ bundle exec ruby -rparser/ruby33 -ve 'p Parser::Ruby33.parse("/foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision ruby/prism@5124f9ac75) [x86_64-darwin22]
s(:match_with_lvasgn,
  s(:regexp,
    s(:str, "foo"),
    s(:regopt)),
  s(:send, nil, :bar))
```

## Prism (`Prism::Translation::Parser`)

### Before

Returns an `send` node:

```console
$ bundle exec ruby -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("/foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision ruby/prism@5124f9ac75) [x86_64-darwin22]
s(:send,
  s(:regexp,
    s(:str, "foo"),
    s(:regopt)), :=~,
  s(:send, nil, :bar))
```

### After

Returns an `match_with_lvasgn` node:

```console
$ bundle exec ruby -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("/foo/ =~ bar")'
ruby 3.3.0 (2023-12-25 revision ruby/prism@5124f9ac75) [x86_64-darwin22]
s(:match_with_lvasgn,
  s(:regexp,
    s(:str, "foo"),
    s(:regopt)),
  s(:send, nil, :bar))
```

## Background

Found due to incompatibility with RuboCop's `Performance/EndWith`, `Performance/StringInclude,
and `Performance/StartWith` cops.

## Note

This is the incompatibility when the receiver is a regular expression literal and `=~` is used.
Based on the node name `:match_with_lvasgn`, it appears that Prism's AST becomes more accurate
in cases like `visit_match_write_node` only.

However, as shown in the background, the current behavior of Parser gem is not like this.
Considering compatibility with the published AST of Parser gem, the AST incompatibility will be addressed.

This lvar-injecting feature appears to have not been supported by Parser gem for a long time:
whitequark/parser#69 (comment)

There seems to be no indication that it will be supported.

This PR prioritizes AST compatibility between the Parser gem and Prism.
However, it is unclear whether this is the best approach.

ruby/prism@dff4abb170
  • Loading branch information
koic authored and matzbot committed Mar 4, 2024
1 parent 26507b9 commit a03f929
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/prism/translation/parser/compiler.rb
Expand Up @@ -254,6 +254,10 @@ def visit_call_node(node)
end
when :!
return visit_block(builder.not_op(token(node.message_loc), token(node.opening_loc), visit(node.receiver), token(node.closing_loc)), block)
when :=~
if (receiver = node.receiver).type == :regular_expression_node
return builder.match_op(visit(receiver), token(node.message_loc), visit(node.arguments.arguments.first))
end
when :[]
return visit_block(builder.index(visit(node.receiver), token(node.opening_loc), visit_all(arguments), token(node.closing_loc)), block)
when :[]=
Expand Down

0 comments on commit a03f929

Please sign in to comment.