-
Notifications
You must be signed in to change notification settings - Fork 296
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] do-end Blocks (with implicit &Proc and yield handling) #3928 #4359
[Ruby] do-end Blocks (with implicit &Proc and yield handling) #3928 #4359
Conversation
badly-drawn-wizards
commented
Mar 18, 2024
•
edited
Loading
edited
- Adds yield RubyNode
- Handles yield in RubyNodeCreator
- Generates a ProcParameter with a unique identifier if a yield is used in a method with no block parameter
- Generates a call to the block parameter for yields
bc8af93
to
f9e6a51
Compare
...cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/datastructures/RubyScope.scala
Outdated
Show resolved
Hide resolved
...ds/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ProcParameterAndYieldTests.scala
Show resolved
Hide resolved
scope.useProcParam match { | ||
case Some(param) => | ||
astForMemberCall( | ||
MemberCall(SimpleIdentifier()(node.span.spanStart(param)), ".", "call", node.arguments)(node.span) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we lower this differently? My concern is that this is closer to reflection in how this is executed, and not sure that our call graph could resolve this.
Could this instead be a SimpleCall
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, concerning flows, the yield needs to map to MethodReturn
, how is this done in pysrc2cpg
/should we add a return
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case node: ast.Yield => unhandled(node)
case node: ast.YieldFrom => unhandled(node)
it isn't handled there, but they would need to be handled vastly differently anyway.
...nds/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala
Outdated
Show resolved
Hide resolved
...nds/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala
Outdated
Show resolved
Hide resolved
...ds/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ProcParameterAndYieldTests.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, let's follow your suggestion (from our chat offline) to add some kind of custom conditional to handle the case where yields can return or generate a collection, etc.