Skip to content
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.

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

Implement translation for PM_BEGIN_NODE #214

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Implement translation for PM_BEGIN_NODE #214

merged 1 commit into from
Aug 23, 2024

Conversation

egiurleo
Copy link

@egiurleo egiurleo commented Aug 23, 2024

Closes #47

Motivation

Translates PM_BEGIN_NODEs from Prism to Sorbet.

These represent begin blocks, e.g.

begin
  # ...
rescue
  # ...
end

Note that I haven't implemented support for rescue (#160), ensure (#86) or else because they're not needed for parsing the RBI gem.

Test plan

See included automated tests.

@egiurleo egiurleo requested review from amomchilov, a team and vinistock August 23, 2024 16:54
Copy link

@amomchilov amomchilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you open an issue to track the translation of else_clause?

The rescue and ensure are their own node types, so we'll implement those in #160 and #86.

test/prism_regression/begin.rb Outdated Show resolved Hide resolved
test/prism_regression/begin.parse-tree.exp Outdated Show resolved Hide resolved
parser/prism/Translator.cc Outdated Show resolved Hide resolved
test/prism_regression/begin.rb Show resolved Hide resolved
@egiurleo
Copy link
Author

@amomchilov Good point re: else. I actually added a note to our rescue ticket because the else clause works together with rescue.

@egiurleo egiurleo merged commit 12e07c9 into prism Aug 23, 2024
1 check passed
@egiurleo egiurleo deleted the emily/begin branch August 23, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants