Skip to content

Commit 7a79301

Browse files
AndreiDreyerTebogo Selahle
andauthored
[php] Capture variables used in arrow closure expressions (#5571)
* [php] Rework PhpScope to use classes and traits similar to Ruby * [php] Revert scalafmt changes * [php] Code cleanup, revert scalafmt version bump * revert changes from scalafmt version bump * [php] auto capture variables used in arrow functions * fmt * merged stash changes * [php] continuing with automatic variable capturing * [php] implemented closure capturing for recursive arrow functions * [php] fix bug on global statements in global scope * chore: fix mess from conflicts * fix: use lambda counter per scope --------- Co-authored-by: Tebogo Selahle <[email protected]>
1 parent f76444d commit 7a79301

File tree

9 files changed

+456
-91
lines changed

9 files changed

+456
-91
lines changed

joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreator.scala

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class AstCreator(
3131
with AstForTypesCreator {
3232

3333
protected val logger: Logger = LoggerFactory.getLogger(AstCreator.getClass)
34-
protected val scope = new Scope(summary, () => nextClosureName())
34+
protected val scope = new Scope(summary)
3535
protected var fileContent = Option.empty[String]
3636

3737
override def createAst(): DiffGraphBuilder = {
@@ -178,50 +178,14 @@ class AstCreator(
178178
}
179179

180180
private def astForGlobalStmt(stmt: PhpGlobalStmt): List[Ast] = {
181-
val surroundingIter = scope.getSurroundingMethods.drop(1).iterator // drop first to ignore global method
182-
val surroundingMethods = scope.getSurroundingMethods.dropRight(1) // drop last to ignore innermost method
183-
184-
surroundingMethods.foreach { currentMethod =>
185-
val innerMethodScope = surroundingIter.next()
186-
val innerMethodNode = innerMethodScope.methodNode
187-
val innerMethodRef = innerMethodScope.methodRefNode
188-
innerMethodRef match {
189-
case Some(methodRef) =>
190-
scope.getMethodRef(innerMethodNode.fullName) match {
191-
case None =>
192-
diffGraph.addNode(methodRef)
193-
diffGraph.addEdge(currentMethod.bodyNode, methodRef, EdgeTypes.AST)
194-
scope.addMethodRef(innerMethodNode.fullName, methodRef)
195-
case _ =>
196-
}
197-
198-
stmt.vars.foreach {
199-
case PhpVariable(name: PhpNameExpr, _) =>
200-
val closureBindingId = s"$relativeFileName:${innerMethodNode.fullName}:${name.name}"
201-
val closureLocal = localNode(stmt, name.name, name.name, Defines.Any, Option(closureBindingId))
202-
203-
val closureBindingNode = NewClosureBinding()
204-
.closureBindingId(closureBindingId)
205-
.evaluationStrategy(EvaluationStrategies.BY_SHARING)
206-
207-
scope.lookupVariable(name.name) match {
208-
case Some(refLocal) => diffGraph.addEdge(closureBindingNode, refLocal, EdgeTypes.REF)
209-
case _ => // do nothing
210-
}
211-
212-
scope.addVariableToMethodScope(closureLocal.name, closureLocal, innerMethodNode.fullName) match {
213-
case Some(ms) => diffGraph.addEdge(ms.bodyNode, closureLocal, EdgeTypes.AST)
214-
case _ => // do nothing
215-
}
216-
217-
diffGraph.addNode(closureBindingNode)
218-
diffGraph.addEdge(methodRef, closureBindingNode, EdgeTypes.CAPTURE)
219-
case x =>
220-
logger.warn(s"Unexpected variable type ${x.getClass} found")
221-
}
222-
case None =>
223-
logger.warn(s"No methodRef found for capturing global variable in method ${innerMethodNode.fullName}")
224-
}
181+
stmt.vars.foreach {
182+
case PhpVariable(name: PhpNameExpr, _) =>
183+
val surroundingIter = scope.getSurroundingMethods.drop(1).iterator // drop first to ignore global method
184+
val surroundingMethods = scope.getSurroundingMethods.dropRight(1) // drop last to ignore innermost method
185+
186+
createClosureCaptureForNode(stmt, name.name, surroundingIter, surroundingMethods)
187+
case x =>
188+
logger.warn(s"Unexpected variable type ${x.getClass} found")
225189
}
226190

227191
stmt.vars.map(astForExpr)

joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreatorHelper.scala

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,24 @@ import io.joern.php2cpg.passes.SymbolSummaryPass.PhpFunction
88
import io.joern.x2cpg.Defines.UnresolvedNamespace
99
import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
1010
import io.joern.x2cpg.{Ast, Defines, ValidationMode}
11-
import io.shiftleft.codepropertygraph.generated.{EdgeTypes, ModifierTypes}
11+
import io.shiftleft.codepropertygraph.generated.{EdgeTypes, EvaluationStrategies, ModifierTypes}
1212
import io.shiftleft.codepropertygraph.generated.nodes.{
13+
MethodParameterIn,
1314
NewBlock,
15+
NewClosureBinding,
1416
NewIdentifier,
1517
NewLiteral,
1618
NewLocal,
1719
NewMethod,
20+
NewMethodParameterIn,
21+
NewMethodRef,
1822
NewModifier,
1923
NewNamespaceBlock,
2024
NewNode
2125
}
2226
import io.shiftleft.semanticcpg.language.types.structure.NamespaceTraversal
2327

28+
import scala.collection.mutable
2429
import java.nio.charset.StandardCharsets
2530

2631
trait AstCreatorHelper(disableFileContent: Boolean)(implicit withSchemaValidation: ValidationMode) { this: AstCreator =>
@@ -147,16 +152,84 @@ trait AstCreatorHelper(disableFileContent: Boolean)(implicit withSchemaValidatio
147152
}
148153

149154
scope.addToScope(name, local) match {
150-
case BlockScope(block, _) => diffGraph.addEdge(block, local, EdgeTypes.AST)
151-
case MethodScope(_, block, _, _, _) => diffGraph.addEdge(block, local, EdgeTypes.AST)
152-
case _ => // do nothing
155+
case BlockScope(block, _) => diffGraph.addEdge(block, local, EdgeTypes.AST)
156+
case MethodScope(_, block, _, _, _, _) => diffGraph.addEdge(block, local, EdgeTypes.AST)
157+
case _ => // do nothing
153158
}
154159

155160
local
161+
case Some(local: NewLocal)
162+
if scope.isSurroundedByArrowClosure && local.closureBindingId.exists(_.contains("<lambda>")) =>
163+
local // the contains check ensures that we can capture global variables into an arrow closure
164+
case Some(param: NewMethodParameterIn)
165+
if scope.isSurroundedByArrowClosure && !scope.surroundingMethodParams.contains(param.name) =>
166+
createClosureBindingsForArrowClosure(expr, name)
167+
case Some(_: NewLocal) if scope.isSurroundedByArrowClosure =>
168+
createClosureBindingsForArrowClosure(expr, name)
156169
case Some(local) => local
157170
}
158171
}
159172

173+
def createClosureCaptureForNode(
174+
expr: PhpNode,
175+
name: String,
176+
innerMethodsIterator: Iterator[MethodScope],
177+
surroundingMethods: List[MethodScope]
178+
): Unit = {
179+
surroundingMethods.foreach { currentMethod =>
180+
val innerMethodScope = innerMethodsIterator.next()
181+
val innerMethodNode = innerMethodScope.methodNode
182+
val innerMethodRef = innerMethodScope.methodRefNode
183+
innerMethodRef match {
184+
case Some(methodRef) =>
185+
scope.getMethodRef(innerMethodNode.fullName) match {
186+
case None =>
187+
diffGraph.addNode(methodRef)
188+
diffGraph.addEdge(currentMethod.bodyNode, methodRef, EdgeTypes.AST)
189+
scope.addMethodRef(innerMethodNode.fullName, methodRef)
190+
case _ =>
191+
}
192+
193+
val closureBindingId = if (innerMethodNode.fullName.contains(NamespaceTraversal.globalNamespaceName)) {
194+
s"${innerMethodNode.fullName}:${name}"
195+
} else {
196+
s"$relativeFileName:${innerMethodNode.fullName}:${name}"
197+
}
198+
199+
val closureLocal = localNode(expr, name, name, Defines.Any, Option(closureBindingId))
200+
201+
val closureBindingNode = createClosureBinding(closureBindingId)
202+
203+
scope.lookupVariable(name) match {
204+
case Some(refLocal) =>
205+
diffGraph.addEdge(closureBindingNode, refLocal, EdgeTypes.REF)
206+
case _ => // do nothing
207+
}
208+
209+
scope.addVariableToMethodScope(closureLocal.name, closureLocal, innerMethodNode.fullName) match {
210+
case Some(ms) => diffGraph.addEdge(ms.bodyNode, closureLocal, EdgeTypes.AST)
211+
case _ => // do nothing
212+
}
213+
214+
diffGraph.addNode(closureBindingNode)
215+
diffGraph.addEdge(methodRef, closureBindingNode, EdgeTypes.CAPTURE)
216+
case None =>
217+
logger.warn(s"No methodRef found for capturing global variable in method ${innerMethodNode.fullName}")
218+
}
219+
}
220+
}
221+
222+
private def createClosureBindingsForArrowClosure(expr: PhpNode, name: String): NewNode = {
223+
val surroundingIter = scope.getSurroundingMethodsForArrowClosure.drop(1).iterator
224+
val surroundingMethods = scope.getSurroundingMethodsForArrowClosure.dropRight(1)
225+
val localNodes = mutable.ArrayBuffer[NewLocal]()
226+
createClosureCaptureForNode(expr, name, surroundingIter, surroundingMethods)
227+
scope.lookupVariable(name).get
228+
}
229+
230+
protected def createClosureBinding(closureBindingId: String): NewClosureBinding =
231+
NewClosureBinding().closureBindingId(closureBindingId).evaluationStrategy(EvaluationStrategies.BY_SHARING)
232+
160233
protected def staticInitMethodAst(node: PhpNode, methodNode: NewMethod, body: Ast, returnType: String): Ast = {
161234
val staticModifier = NewModifier().modifierType(ModifierTypes.STATIC)
162235
val methodReturn = methodReturnNode(node, returnType)

joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstForControlStructuresCreator.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,10 @@ trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMo
127127
val local = localNode(variable, name.name, name.name, Defines.Any)
128128

129129
val node = scope.addToScope(name.name, local) match {
130-
case NamespaceScope(namespaceNode, _) => namespaceNode
131-
case TypeScope(typeDeclNode, _) => typeDeclNode
132-
case MethodScope(methodNode, _, _, _, _) => methodNode
133-
case BlockScope(blockNode, _) => blockNode
130+
case NamespaceScope(namespaceNode, _) => namespaceNode
131+
case TypeScope(typeDeclNode, _) => typeDeclNode
132+
case MethodScope(methodNode, _, _, _, _, _) => methodNode
133+
case BlockScope(blockNode, _) => blockNode
134134
}
135135

136136
diffGraph.addEdge(node, local, EdgeTypes.AST)

joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstForExpressionsCreator.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import io.joern.php2cpg.astcreation.AstCreator.{NameConstants, TypeConstants, op
44
import io.joern.php2cpg.datastructures.ArrayIndexTracker
55
import io.joern.php2cpg.parser.Domain
66
import io.joern.php2cpg.parser.Domain.*
7+
import io.joern.x2cpg.Defines.{UnresolvedNamespace, UnresolvedSignature}
8+
import io.joern.x2cpg.Defines.UnresolvedSignature
79
import io.joern.php2cpg.utils.BlockScope
810
import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
911
import io.joern.x2cpg.{Ast, Defines, ValidationMode}

joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstForFunctionsCreator.scala

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import io.joern.php2cpg.astcreation.AstCreator.{NameConstants, TypeConstants}
44
import io.joern.php2cpg.parser.Domain
55
import io.joern.php2cpg.parser.Domain.*
66
import io.joern.php2cpg.parser.Domain.PhpModifiers.containsAccessModifier
7-
import io.joern.php2cpg.utils.MethodScope
7+
import io.joern.php2cpg.utils.{BlockScope, MethodScope}
8+
import io.joern.x2cpg.Defines.UnresolvedSignature
89
import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
910
import io.joern.x2cpg.{Ast, Defines, ValidationMode}
1011
import io.shiftleft.codepropertygraph.generated.nodes.*
@@ -47,9 +48,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
4748

4849
local.closureBindingId(closureBindingId)
4950

50-
val closureBindingNode = NewClosureBinding()
51-
.closureBindingId(closureBindingId)
52-
.evaluationStrategy(EvaluationStrategies.BY_SHARING)
51+
val closureBindingNode = createClosureBinding(closureBindingId)
5352

5453
scope.lookupVariable(local.name) match {
5554
case Some(refLocal) =>
@@ -94,7 +93,9 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
9493
localsForUses.map(Ast(_)),
9594
Option(methodName),
9695
usesCode = Option(usesCode),
97-
isAnonymousMethod = true
96+
isArrowClosure = closureExpr.isArrowFunc,
97+
isAnonymousMethod = true,
98+
closureMethodRef = Option(methodRef)
9899
)
99100

100101
// Add method to scope to be attached to typeDecl later
@@ -109,7 +110,9 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
109110
fullNameOverride: Option[String] = None,
110111
isConstructor: Boolean = false,
111112
usesCode: Option[String] = None,
112-
isAnonymousMethod: Boolean = false
113+
isArrowClosure: Boolean = false,
114+
isAnonymousMethod: Boolean = false,
115+
closureMethodRef: Option[NewMethodRef] = None
113116
): Ast = {
114117
val isStatic = decl.modifiers.contains(ModifierTypes.STATIC)
115118
val thisParam = if (!isAnonymousMethod && decl.isClassMethod && !isStatic) {
@@ -150,7 +153,9 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
150153

151154
val methodBodyNode = blockNode(decl)
152155

153-
scope.pushNewScope(MethodScope(method, methodBodyNode, method.fullName, methodRef))
156+
scope.pushNewScope(
157+
MethodScope(method, methodBodyNode, method.fullName, decl.params.map(_.name), methodRef, isArrowClosure)
158+
)
154159
scope.useFunctionDecl(methodName, fullName)
155160

156161
val returnType = decl.returnType.map(_.name).getOrElse(Defines.Any)
@@ -165,6 +170,28 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
165170
val attributeAsts = decl.attributeGroups.flatMap(astForAttributeGroup)
166171
val methodBody = blockAst(methodBodyNode, methodBodyStmts)
167172

173+
if (isArrowClosure) {
174+
scope.getAndClearClosureBindings.foreach { (variableLocal, variableClosureBinding) =>
175+
val closureBindingId = s"$methodName:${variableLocal.name}"
176+
val closureBinding = createClosureBinding(closureBindingId)
177+
val localNode_ =
178+
localNode(decl, variableLocal.name, variableLocal.name, variableLocal.typeFullName, Option(closureBindingId))
179+
180+
scope.addClosureBinding(closureBinding, localNode_)
181+
182+
diffGraph.addNode(closureBinding)
183+
diffGraph.addEdge(variableClosureBinding, localNode_, EdgeTypes.REF)
184+
185+
scope.addToScope(localNode_.name, localNode_) match {
186+
case BlockScope(block, _) => diffGraph.addEdge(block, localNode_, EdgeTypes.AST)
187+
case MethodScope(_, block, _, _, _, _) => diffGraph.addEdge(block, localNode_, EdgeTypes.AST)
188+
case _ => // do nothing
189+
}
190+
191+
diffGraph.addEdge(closureMethodRef.get, closureBinding, EdgeTypes.CAPTURE)
192+
}
193+
}
194+
168195
scope.popScope()
169196
val methodAst = methodAstWithAnnotations(method, parameters, methodBody, methodReturn, modifiers, attributeAsts)
170197

@@ -302,7 +329,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
302329

303330
val methodBlock = NewBlock()
304331

305-
scope.pushNewScope(MethodScope(methodNode_, methodBlock, fullName, Option(methodRef)))
332+
scope.pushNewScope(MethodScope(methodNode_, methodBlock, fullName, methodRefNode = Option(methodRef)))
306333

307334
val assignmentAsts = inits.map { init =>
308335
astForMemberAssignment(init.originNode, init.memberNode, init.value, isField = false)

0 commit comments

Comments
 (0)