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

[ruby] Begin, Rescue, Ensure (Try-Catch Statements) for intermediate ast #3927 #3970

Merged

Conversation

badly-drawn-wizards
Copy link
Contributor

@badly-drawn-wizards badly-drawn-wizards commented Dec 18, 2023

The related issue assumes the RubyNode intermediate AST is complete for Begin, Rescue, Ensure, but this is not the case. The scope of this issue has crept to include handling the ANTLR-to-RubyNode AST feature for these expressions, as well as the AstCreator CPG building. This appears to be the case for a few of the other Ruby issues, which will influence their ETA accordingly.

Additionally, Rescue clauses in Ruby can contain splats which isn't handled yet in parsing. For now the exception list is will be an unknown block and posted as a medium-priority issue. I am unsure if adding a RubyNode for multipleRightHandSide affects anything else.

The scope of this PR is reduced to just the intermediate AST and the AST creator changes will be in a follow up.

@badly-drawn-wizards badly-drawn-wizards linked an issue Dec 18, 2023 that may be closed by this pull request
@badly-drawn-wizards badly-drawn-wizards self-assigned this Dec 18, 2023
@badly-drawn-wizards badly-drawn-wizards added the ruby Relates to rubysrc2cpg label Dec 18, 2023
@badly-drawn-wizards
Copy link
Contributor Author

badly-drawn-wizards commented Dec 18, 2023

An open question for myself is that it that currently the same RubyNode can result in different AST generation depending on whether it is processed by astForExpression or astForStatement. For example

if true then end

results in a ControlFlowNode
and

x = if true then end

result in a ternary operation

This leads to the question of why this is the case and whether rescue blocks should follow the same logic or if all control flow can be treated uniformly as expressions.

@badly-drawn-wizards badly-drawn-wizards force-pushed the 3927-ruby-begin-rescue-ensure-try-catch-statements branch from f44297d to 0c8f76f Compare December 19, 2023 11:53
@badly-drawn-wizards badly-drawn-wizards force-pushed the 3927-ruby-begin-rescue-ensure-try-catch-statements branch from 0c8f76f to 360b98e Compare December 19, 2023 11:54
@badly-drawn-wizards badly-drawn-wizards changed the title [ruby] Begin, Rescue, Ensure (Try-Catch Statements) #3927 [ruby] Begin, Rescue, Ensure (Try-Catch Statements) for intermediate ast #3927 Dec 19, 2023
@badly-drawn-wizards badly-drawn-wizards marked this pull request as ready for review December 19, 2023 11:59
@DavidBakerEffendi DavidBakerEffendi merged commit 77b6eaf into master Dec 19, 2023
10 checks passed
@DavidBakerEffendi DavidBakerEffendi deleted the 3927-ruby-begin-rescue-ensure-try-catch-statements branch December 19, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ruby Relates to rubysrc2cpg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ruby] Begin, Rescue, Ensure (Try-Catch Statements)
2 participants