From dff4abb1708d87674c0aa212c2fbb932e2b27937 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 4 Mar 2024 21:10:47 +0900 Subject: [PATCH 1/2] Fix incompatibility AST for regexp match in `Prism::Translation::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 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 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 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: https://github.com/whitequark/parser/issues/69#issuecomment-19506391 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. --- lib/prism/translation/parser/compiler.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index 0025e4697d7..9dd20baeb4d 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -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 :[]= From dccfd83bc4dcafb5c3bb8f734808e3c53202f9cc Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Mon, 4 Mar 2024 11:20:55 -0500 Subject: [PATCH 2/2] Update lib/prism/translation/parser/compiler.rb --- lib/prism/translation/parser/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index 9dd20baeb4d..3ce62bb8f1e 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -255,7 +255,7 @@ def visit_call_node(node) 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 + if (receiver = node.receiver).is_a?(RegularExpressionNode) return builder.match_op(visit(receiver), token(node.message_loc), visit(node.arguments.arguments.first)) end when :[]