Skip to content

Commit

Permalink
[ruby] Bind nested method members to method type (#4747)
Browse files Browse the repository at this point in the history
This PR fixes a bug where method members were not correctly linked to surrounding methods' bound type decls. Additionally, this handles `return` statements without any proceeding expression.

Resolves #4732
  • Loading branch information
DavidBakerEffendi authored Jul 8, 2024
1 parent 2bbece9 commit 0acd0a2
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
case node: SelfIdentifier => astForSelfIdentifier(node)
case node: BreakStatement => astForBreakStatement(node)
case node: StatementList => astForStatementList(node)
case node: ReturnExpression => astForReturnStatement(node)
case node: DummyNode => Ast(node.node)
case node: Unknown => astForUnknown(node)
case x =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
case Some(astParentTfn) => memberForMethod(method, Option(NodeTypes.TYPE_DECL), Option(astParentTfn))
case None => memberForMethod(method, scope.surroundingAstLabel, scope.surroundingScopeFullName)
}
Ast(memberForMethod(method, scope.surroundingAstLabel, scope.surroundingScopeFullName))
Ast(memberForMethod(method, Option(NodeTypes.TYPE_DECL), scope.surroundingScopeFullName))
}
// For closures, we also want the method/type refs for upstream use
val methodAst_ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ class RubyNodeCreator extends RubyParserBaseVisitor[RubyNode] {
ReturnExpression(expressions)(ctx.toTextSpan)
}

override def visitReturnWithoutArguments(ctx: RubyParser.ReturnWithoutArgumentsContext): RubyNode = {
ReturnExpression(Nil)(ctx.toTextSpan)
}

override def visitNumericLiteral(ctx: RubyParser.NumericLiteralContext): RubyNode = {
if (ctx.hasSign) {
UnaryExpression(ctx.sign.getText, visit(ctx.unsignedNumericLiteral()))(ctx.toTextSpan)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,4 +439,21 @@ class MethodReturnTests extends RubyCode2CpgFixture(withDataFlow = true) {
}
}

"a return in an expression position without arguments should generate a return node with no children" in {
val cpg = code("""
|def foo
| return unless baz()
| bar()
|end
|""".stripMargin)

inside(cpg.method.nameExact("foo").ast.isReturn.headOption) {
case Some(ret) =>
ret.code shouldBe "return"
ret.astChildren.size shouldBe 0
ret.astParent.astParent.code shouldBe "return unless baz()"
case None => fail(s"Expected at least one return node")
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import io.shiftleft.codepropertygraph.generated.nodes.*
import io.shiftleft.codepropertygraph.generated.{ControlStructureTypes, NodeTypes, Operators}
import io.shiftleft.semanticcpg.language.*

import scala.util.{Failure, Success, Try}

class MethodTests extends RubyCode2CpgFixture {

"`def f(x) = 1`" should {
Expand Down Expand Up @@ -717,4 +719,17 @@ class MethodTests extends RubyCode2CpgFixture {
case xs => fail(s"Expected one call to foo, got [${xs.code.mkString(",")}]")
}
}

"a nested method declaration inside of a do-block should connect the member node to the bound type decl" in {
val cpg = code("""
|foo do
| def bar
| end
|end
|""".stripMargin)

val parentType = cpg.member("bar").typeDecl.head
parentType.isLambda should not be empty
parentType.methodBinding.methodFullName.head should endWith("<lambda>0")
}
}

0 comments on commit 0acd0a2

Please sign in to comment.