Fix an incompatible behavior for `Prism::Translation::Parser` #2552

koic · 2024-03-04T17:05:33Z

This PR fixes the following incompatible behavior for Prism::Translation::Parser when using case/when/end with ; expression.

Expected

It returns the range of the semicolon even if there is a space before the semicolon:

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 15...16>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 16...17>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

Actual

It returns nil if there is a space before the semicolon:

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 15...16>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
nil
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \
'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-e:1:in `<main>': undefined method `source' for nil (NoMethodError)

p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source
                                                                                        ^^^^^^^

kddnewton · 2024-03-04T17:08:17Z

@koic I just added that to the WhenNode to fix this. Have you recompiled?

koic · 2024-03-04T17:23:28Z

Oops, I forgot to recompile. But, after compiling, the following issues occur.

Expected

It returns the range of the semicolon even if there is a space before the semicolon:

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 15...16>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 16...17>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

Actual

It returns nil if there is a space before the semicolon:

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
#<Parser::Source::Range (string) 15...16>
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
";"

$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
nil
$ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source'
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-e:1:in `<main>': undefined method `source' for nil (NoMethodError)

p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source
                                                                                        ^^^^^^^

This PR also fixes the above issues. I've updated the PR description as well.

koic · 2024-03-09T07:49:32Z

lib/prism/translation/parser/compiler.rb

-            else
-              srange_find(node.conditions.last.location.end_offset, node.statements&.location&.start_offset || (node.conditions.last.location.end_offset + 1), [";"])
-            end,
+            srange_find(node.conditions.last.location.end_offset, node.statements&.location&.start_offset || node.statements&.then_keyword_loc, [";", "then"]),


Note: This is the same solution as the following.
https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/compiler.rb#L828

This PR fixes the following incompatible behavior for `Prism::Translation::Parser` when using `case`/`when`/`end` with `;` expression. ## Expected It returns the range of the semicolon even if there is a space before the semicolon: ```console $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] #<Parser::Source::Range (string) 15...16> $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] ";" $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] #<Parser::Source::Range (string) 16...17> $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] ";" ``` ## Actual It returns `nil` if there is a space before the semicolon: ```console $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] #<Parser::Source::Range (string) 15...16> $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x; end").children.to_a[1].loc.begin.source' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] ";" $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] nil $ bundle exec ruby -Ilib -rprism -rprism/translation/parser33 -ve \ 'p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source' ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] -e:1:in `<main>': undefined method `source' for nil (NoMethodError) p Prism::Translation::Parser33.parse("case foo when x ; end").children.to_a[1].loc.begin.source ^^^^^^^ ```

kddnewton · 2024-04-23T19:20:30Z

Thanks @koic, this problem ended up being handled by #2552

koic force-pushed the fix_parser_translator_for_case_when_then branch from 1f7e2eb to 1aa9c8c Compare March 4, 2024 17:23

koic changed the title ~~Fix an error for Prism::Translation::Parser~~ Fix an unexpected behavior for Prism::Translation::Parser Mar 4, 2024

koic force-pushed the fix_parser_translator_for_case_when_then branch from 1aa9c8c to 098b9f5 Compare March 4, 2024 17:28

koic commented Mar 9, 2024

View reviewed changes

koic force-pushed the fix_parser_translator_for_case_when_then branch from 098b9f5 to a181fa9 Compare April 2, 2024 14:17

koic changed the title ~~Fix an unexpected behavior for Prism::Translation::Parser~~ Fix an incompatible behavior for Prism::Translation::Parser Apr 2, 2024

koic force-pushed the fix_parser_translator_for_case_when_then branch from a181fa9 to 9625805 Compare April 2, 2024 14:18

kddnewton mentioned this pull request Apr 23, 2024

srange_find should only look on current line #2729

Merged

kddnewton closed this in #2729 Apr 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix an incompatible behavior for `Prism::Translation::Parser` #2552

Fix an incompatible behavior for `Prism::Translation::Parser` #2552

koic commented Mar 4, 2024 •

edited

Loading

kddnewton commented Mar 4, 2024

koic commented Mar 4, 2024 •

edited

Loading

koic Mar 9, 2024

kddnewton commented Apr 23, 2024

Fix an incompatible behavior for Prism::Translation::Parser #2552

Fix an incompatible behavior for Prism::Translation::Parser #2552

Conversation

koic commented Mar 4, 2024 • edited Loading

Expected

Actual

kddnewton commented Mar 4, 2024

koic commented Mar 4, 2024 • edited Loading

Expected

Actual

koic Mar 9, 2024

Choose a reason for hiding this comment

kddnewton commented Apr 23, 2024

Fix an incompatible behavior for `Prism::Translation::Parser` #2552

Fix an incompatible behavior for `Prism::Translation::Parser` #2552

koic commented Mar 4, 2024 •

edited

Loading

koic commented Mar 4, 2024 •

edited

Loading