Skip to content

Commit

Permalink
[ruby] Fixed Grouped Stmt Flows & Splat Ambiguity (#4526)
Browse files Browse the repository at this point in the history
* Fixed issue where lists of `StatementList` were not being unpacked in grouped assignments
* Fixed ambiguity of assignment to a splat without spacing being detected as a call multiplied by RHS, e.g. `y=*x`

Resolves #4506
  • Loading branch information
DavidBakerEffendi authored May 2, 2024
1 parent d17611f commit bdfe025
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
case node: ProcOrLambdaExpr => astForProcOrLambdaExpr(node)
case node: RubyCallWithBlock[_] => astsForCallWithBlockInExpr(node)
case node: SelfIdentifier => astForSelfIdentifier(node)
case node: StatementList => astForStatementList(node)
case node: DummyNode => Ast(node.node)
case _ => astForUnknown(node)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,27 @@ class RubyNodeCreator extends RubyParserBaseVisitor[RubyNode] {
HereDocNode(ctx.hereDoc().getText)(ctx.toTextSpan)
}

override def visitPrimaryOperatorExpression(ctx: RubyParser.PrimaryOperatorExpressionContext): RubyNode = {
super.visitPrimaryOperatorExpression(ctx) match {
case x: BinaryExpression if x.lhs.text.endsWith("=") && x.op == "*" =>
// fixme: This workaround handles a parser ambiguity with method identifiers having `=` and assignments with
// splatting on the RHS. The Ruby parser gives precedence to assignments over methods called with this suffix
// however
val newLhs = x.lhs match {
case call: SimpleCall => SimpleIdentifier(None)(call.span.spanStart(call.span.text.stripSuffix("=")))
case y =>
logger.warn(s"Unhandled class in repacking of primary operator expression ${y.getClass}")
y
}
val newRhs = {
val oldRhsSpan = x.rhs.span
SplattingRubyNode(x.rhs)(oldRhsSpan.spanStart(s"*${oldRhsSpan.text}"))
}
SingleAssignment(newLhs, "=", newRhs)(x.span)
case x => x
}
}

override def visitPowerExpression(ctx: RubyParser.PowerExpressionContext): RubyNode = {
BinaryExpression(visit(ctx.primaryValue(0)), ctx.powerOperator.getText, visit(ctx.primaryValue(1)))(ctx.toTextSpan)
}
Expand Down Expand Up @@ -349,6 +370,14 @@ class RubyNodeCreator extends RubyParserBaseVisitor[RubyNode] {
SingleAssignment(lhs, op, rhs)(ctx.toTextSpan)
}

private def flattenStatementLists(x: List[RubyNode]): List[RubyNode] = {
x match {
case (head: StatementList) :: xs => head.statements ++ flattenStatementLists(xs)
case head :: tail => head +: flattenStatementLists(tail)
case Nil => Nil
}
}

override def visitMultipleAssignmentStatement(ctx: RubyParser.MultipleAssignmentStatementContext): RubyNode = {

/** Recursively expand and duplicate splatting nodes so that they line up with what they consume.
Expand All @@ -372,11 +401,13 @@ class RubyNodeCreator extends RubyParserBaseVisitor[RubyNode] {
.map(node => SplattingRubyNode(node)(node.span.spanStart(s"*${node.span.text}")))
)
.getOrElse(defaultResult()) match {
case x: StatementList => x.statements
case x: StatementList => flattenStatementLists(x.statements)
case x => List(x)
}
val rhsNodes = Option(ctx.multipleRightHandSide()).map(visit).getOrElse(defaultResult()) match {
case x: StatementList => x.statements
val rhsNodes = Option(ctx.multipleRightHandSide())
.map(visit)
.getOrElse(defaultResult()) match {
case x: StatementList => flattenStatementLists(x.statements)
case x => List(x)
}
val op = ctx.EQ().toString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ class MultipleAssignmentTests extends RubyCode2CpgFixture(withPostProcessing = t
sink.reachableByFlows(src).l.size shouldBe 2
}

// Works in deprecated - fails to parse in new frontend
"flow through multiple assignments with grouping" ignore {
"flow through multiple assignments with grouping" in {
val cpg = code("""
|x = 1
|y = 2
Expand All @@ -29,11 +28,10 @@ class MultipleAssignmentTests extends RubyCode2CpgFixture(withPostProcessing = t

val src = cpg.identifier.name("x").l
val sink = cpg.call.name("puts").l
sink.reachableByFlows(src).l.size shouldBe 2
sink.reachableByFlows(src).size shouldBe 2
}

// Works in deprecated - fails to parse in new frontend
"Data flow through multiple assignments with grouping and method in RHS" ignore {
"Data flow through multiple assignments with grouping and method in RHS" in {
val cpg = code("""
|def foo()
|x = 1
Expand All @@ -48,11 +46,10 @@ class MultipleAssignmentTests extends RubyCode2CpgFixture(withPostProcessing = t

val src = cpg.identifier.name("x").l
val sink = cpg.call.name("puts").l
sink.reachableByFlows(src).l.size shouldBe 2
sink.reachableByFlows(src).size shouldBe 2
}

// Works in deprecated
"Data flow through single LHS and splatting RHS" ignore {
"Data flow through single LHS and splatting RHS" in {
val cpg = code("""
|x=1
|y=*x
Expand All @@ -61,7 +58,7 @@ class MultipleAssignmentTests extends RubyCode2CpgFixture(withPostProcessing = t

val src = cpg.identifier.name("x").l
val sink = cpg.call.name("puts").l
sink.reachableByFlows(src).l.size shouldBe 2
sink.reachableByFlows(src).size shouldBe 2
}

}

0 comments on commit bdfe025

Please sign in to comment.