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) AST creator changes #3927 #3976

Merged

Conversation

badly-drawn-wizards
Copy link
Contributor

Continuation of the prior PR on the intermediate AST to make the corresponding changes for the AST creator.

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

// Helper for nil literals to put in empty clauses
protected def astForNilLiteral: Ast = Ast(NewLiteral().code("nil").typeFullName(getBuiltInType(Defines.NilClass)))
protected def astForNilBlock: Ast = blockAst(NewBlock(), List(astForNilLiteral))
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a blockNode(RubyNode) method that uses the node info for code/location information. Maybe this can accept an Option[RubyNode] then you can node.map(blockNode).getOrElse(NewBlock())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This takes inspiration from how the if expression is handled, as they insert a nil there. As long as dataflow can interpret an empty block as a nil value, then that logic is probably not necesary.

val rescueBodies = node.rescueClauses.map(rescue => astForStatementList(rescue.asStatementList))
val elseClause = node.elseClause.map(_.asStatementList).map(astForStatementList)
val ensureClause = node.ensureClause.map(_.asStatementList).map(astForStatementList)
controlStructureAst()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget to populate the params here when you start testing :)

@badly-drawn-wizards badly-drawn-wizards force-pushed the reuben/rubysrc2cpg/ruby-begin-rescue-ast-creator branch from 5597df7 to ec5dab7 Compare December 20, 2023 17:13
@@ -254,4 +252,37 @@ class ControlStructureTests extends RubyCode2CpgFixture {
putsHi.lineNumber shouldBe Some(2)
}

"`begin ... rescue ... end is represented by a `TRY` CONTROL_STRUCTURE node" in {
val cpg = code("""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think add multiple statements in at least one of these blocks to ensure they're correctly grouped for multiple lines and then I'd ship this and put the exception assignment handling in a new issue at lower priority.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yes, and a merge with master appears to be necessary

@badly-drawn-wizards badly-drawn-wizards marked this pull request as ready for review December 21, 2023 11:01
@DavidBakerEffendi DavidBakerEffendi merged commit bd76501 into master Dec 21, 2023
5 checks passed
@max-leuthaeuser max-leuthaeuser deleted the reuben/rubysrc2cpg/ruby-begin-rescue-ast-creator branch April 2, 2024 16:32
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