[php2cpg] Add method bindings to class methods to fix dataflow (#5592)

* [php2cpg] Add method bindings to fix dataflow

* Use the empty string for php2cpg signatures

* Sort test output for stability

Author: johannescoetzee

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

@@ -1,13 +1,18 @@
 package io.joern.php2cpg.astcreation
 
-import io.joern.php2cpg.utils.{BlockScope, NamespaceScope, TypeScope, MethodScope}
+import io.joern.php2cpg.utils.{BlockScope, MethodScope, NamespaceScope, TypeScope}
 import io.joern.php2cpg.astcreation.AstCreator.TypeConstants
 import io.joern.php2cpg.parser.Domain.*
-import io.joern.x2cpg.Defines.UnresolvedSignature
 import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
 import io.joern.x2cpg.{Ast, Defines, ValidationMode}
 import io.shiftleft.codepropertygraph.generated.nodes.*
-import io.shiftleft.codepropertygraph.generated.{ControlStructureTypes, DispatchTypes, EdgeTypes, Operators}
+import io.shiftleft.codepropertygraph.generated.{
+  ControlStructureTypes,
+  DispatchTypes,
+  EdgeTypes,
+  Operators,
+  PropertyDefaults
+}
 import io.shiftleft.proto.cpg.Cpg.CpgStruct.Edge.EdgeType
 
 trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMode) { this: AstCreator =>
@@ -200,17 +205,9 @@ trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMo
 
     // Update asts
     val nextIterIdent = astForIdentifierWithLocalRef(iterIdentifier.copy, localN)
-    val nextSignature = "void()"
     val nextCallCode  = s"${nextIterIdent.rootCodeOrEmpty}${InstanceMethodDelimiter}next()"
-    val nextCallNode = callNode(
-      stmt,
-      nextCallCode,
-      "next",
-      "Iterator.next",
-      DispatchTypes.DYNAMIC_DISPATCH,
-      Some(nextSignature),
-      Some(Defines.Any)
-    )
+    val nextCallNode =
+      callNode(stmt, nextCallCode, "next", "Iterator.next", DispatchTypes.DYNAMIC_DISPATCH, None, Some(Defines.Any))
     val nextCallAst = callAst(nextCallNode, base = Option(nextIterIdent))
     val itemUpdateAst = itemInitAst.root match {
       case Some(initRoot: AstNodeNew) => itemInitAst.subTreeCopy(initRoot)
@@ -240,7 +237,6 @@ trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMo
     // create assignment for value-part
     val valueAssign = {
       val iteratorIdentifierAst = astForIdentifierWithLocalRef(iteratorIdentifier, localN)
-      val currentCallSignature  = s"$UnresolvedSignature(0)"
       val currentCallCode       = s"${iteratorIdentifierAst.rootCodeOrEmpty}${InstanceMethodDelimiter}current()"
       // `current` function is used to get the current element of given array
       // see https://www.php.net/manual/en/function.current.php & https://www.php.net/manual/en/iterator.current.php
@@ -250,7 +246,7 @@ trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMo
         "current",
         "Iterator.current",
         DispatchTypes.DYNAMIC_DISPATCH,
-        Some(currentCallSignature),
+        None,
         Some(Defines.Any)
       )
       val currentCallAst = callAst(currentCallNode, base = Option(iteratorIdentifierAst))
@@ -268,19 +264,11 @@ trait AstForControlStructuresCreator(implicit withSchemaValidation: ValidationMo
     // try to create assignment for key-part
     val keyAssignOption = stmt.keyVar.map(keyVar =>
       val iteratorIdentifierAst = astForIdentifierWithLocalRef(iteratorIdentifier.copy, localN)
-      val keyCallSignature      = s"$UnresolvedSignature(0)"
       val keyCallCode           = s"${iteratorIdentifierAst.rootCodeOrEmpty}${InstanceMethodDelimiter}key()"
       // `key` function is used to get the key of the current element
       // see https://www.php.net/manual/en/function.key.php & https://www.php.net/manual/en/iterator.key.php
-      val keyCallNode = callNode(
-        stmt,
-        keyCallCode,
-        "key",
-        "Iterator.key",
-        DispatchTypes.DYNAMIC_DISPATCH,
-        Some(keyCallSignature),
-        Some(Defines.Any)
-      )
+      val keyCallNode =
+        callNode(stmt, keyCallCode, "key", "Iterator.key", DispatchTypes.DYNAMIC_DISPATCH, None, Some(Defines.Any))
       val keyCallAst = callAst(keyCallNode, base = Option(iteratorIdentifierAst))
       simpleAssignAst(stmt, astForExpr(keyVar), keyCallAst)
     )

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

@@ -4,18 +4,10 @@ import io.joern.php2cpg.astcreation.AstCreator.{NameConstants, TypeConstants, op
 import io.joern.php2cpg.datastructures.ArrayIndexTracker
 import io.joern.php2cpg.parser.Domain.*
 import io.joern.php2cpg.utils.BlockScope
-import io.joern.x2cpg.Defines.{UnresolvedNamespace, UnresolvedSignature}
-import io.joern.x2cpg.Defines.UnresolvedSignature
 import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
 import io.joern.x2cpg.{Ast, Defines, ValidationMode}
 import io.shiftleft.codepropertygraph.generated.nodes.*
-import io.shiftleft.codepropertygraph.generated.{
-  ControlStructureTypes,
-  DispatchTypes,
-  EdgeTypes,
-  Operators,
-  PropertyNames
-}
+import io.shiftleft.codepropertygraph.generated.{ControlStructureTypes, DispatchTypes, Operators, PropertyNames}
 import io.shiftleft.semanticcpg.language.types.structure.NamespaceTraversal
 
 import scala.collection.mutable
@@ -110,9 +102,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
 
     val fullName = getMfn(call, name)
 
-    // Use method signature for methods that can be linked to avoid varargs issue.
-    val signature = s"$UnresolvedSignature(${call.args.size})"
-    val callRoot  = callNode(call, code, name, fullName, dispatchType, Some(signature), Some(Defines.Any))
+    val callRoot = callNode(call, code, name, fullName, dispatchType, None, Some(Defines.Any))
 
     val receiverAst = targetAst match {
       case None if !scope.isTopLevel =>
@@ -140,9 +130,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
       case _                                             => getMfn(call, name)
     }
 
-    // Use method signature for methods that can be linked to avoid varargs issue.
-    val signature = s"$UnresolvedSignature(${call.args.size})"
-    val callRoot  = callNode(call, code, name, fullName, dispatchType, Some(signature), Some(Defines.Any))
+    val callRoot = callNode(call, code, name, fullName, dispatchType, None, Some(Defines.Any))
 
     callAst(callRoot, arguments)
   }
@@ -886,7 +874,6 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
 
     // Init node
     val initArgs      = expr.args.map(astForCallArg)
-    val initSignature = s"$UnresolvedSignature(${initArgs.size})"
     val initFullName  = s"$className$MethodDelimiter$ConstructorMethodName"
     val initCode      = s"new $className(${initArgs.map(_.rootCodeOrEmpty).mkString(",")})"
     val maybeTypeHint = scope.resolveIdentifier(className).map(_.name) // consider imported or defined types
@@ -896,7 +883,7 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
       ConstructorMethodName,
       initFullName,
       DispatchTypes.STATIC_DISPATCH,
-      Some(initSignature),
+      None,
       maybeTypeHint.orElse(Some(Defines.Any)) // TODO Review Note: Should the hint be under dynamicTypeHintFullName?
     )
     val initReceiver = astForIdentifierWithLocalRef(tmpIdentifier.copy, local)

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

@@ -4,7 +4,6 @@ import io.joern.php2cpg.astcreation.AstCreator.{NameConstants, TypeConstants}
 import io.joern.php2cpg.parser.Domain.*
 import io.joern.php2cpg.parser.Domain.PhpModifiers.containsAccessModifier
 import io.joern.php2cpg.utils.MethodScope
-import io.joern.x2cpg.Defines.UnresolvedSignature
 import io.joern.x2cpg.utils.AstPropertiesUtil.RootProperties
 import io.joern.x2cpg.{Ast, Defines, ValidationMode}
 import io.shiftleft.codepropertygraph.generated.nodes.*
@@ -110,8 +109,6 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
     val methodName = decl.name.name
     val fullName   = fullNameOverride.getOrElse(composeMethodFullName(methodName))
 
-    val signature = s"$UnresolvedSignature(${decl.params.size})"
-
     val parameters = thisParam.toList ++ decl.params.zipWithIndex.map { case (param, idx) =>
       astForParam(param, idx + 1)
     }
@@ -132,7 +129,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
     val methodRef =
       if methodName == NamespaceTraversal.globalNamespaceName then None
       else Option(methodRefNode(decl, s"$methodCode", fullName, Defines.Any))
-    val method = methodNode(decl, methodName, methodCode, fullName, Some(signature), relativeFileName)
+    val method = methodNode(decl, methodName, methodCode, fullName, None, relativeFileName)
 
     scope.surroundingScopeFullName.map(method.astParentFullName(_))
     scope.surroundingAstLabel.map(method.astParentType(_))
@@ -157,11 +154,9 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
     scope.popScope()
     val methodAst = methodAstWithAnnotations(method, parameters, methodBody, methodReturn, modifiers, attributeAsts)
 
-    Ast.storeInDiffGraph(methodAst, diffGraph)
     val methodRefAst = methodRef.map(Ast(_)).getOrElse(Ast())
 
-    // method gets added via the AST_PARENT_FULLNAME property
-    Ast()
+    methodAst
   }
 
   private def thisParamAstForMethod(originNode: PhpNode): Ast = {
@@ -195,8 +190,6 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
   protected def defaultConstructorAst(originNode: PhpNode): Ast = {
     val fullName = composeMethodFullName(ConstructorMethodName)
 
-    val signature = s"$UnresolvedSignature(0)"
-
     val modifiers =
       List(ModifierTypes.VIRTUAL, ModifierTypes.PUBLIC, ModifierTypes.CONSTRUCTOR).map(modifierNode(originNode, _))
 
@@ -206,7 +199,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
       astForMemberAssignment(fieldInit.originNode, fieldInit.memberNode, fieldInit.value, isField = true)
     }
 
-    val method    = methodNode(originNode, ConstructorMethodName, fullName, fullName, Some(signature), relativeFileName)
+    val method    = methodNode(originNode, ConstructorMethodName, fullName, fullName, None, relativeFileName)
     val methodRef = methodRefNode(originNode, fullName, fullName, Defines.Any)
 
     val methodBodyBlock = blockNode(originNode)
@@ -221,12 +214,10 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
     val methodReturn = methodReturnNode(originNode, Defines.Any)
 
     val methodAst = methodAstWithAnnotations(method, thisParam :: Nil, methodBody, methodReturn, modifiers)
-    Ast.storeInDiffGraph(methodAst, diffGraph)
 
     scope.popScope()
 
-    // AST gets added via AST_PARENT_FULLNAME property
-    Ast()
+    methodAst
   }
 
   protected def astForAttributeGroup(attrGrp: PhpAttributeGroup): Seq[Ast] = {
@@ -266,13 +257,12 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
       case Nil => None
 
       case inits =>
-        val signature = s"${TypeConstants.Void}()"
-        val fullName  = composeMethodFullName(Defines.StaticInitMethodName)
+        val fullName = composeMethodFullName(Defines.StaticInitMethodName)
         val methodNode_ = methodNode(
           node,
           Defines.StaticInitMethodName,
           fullName,
-          signature,
+          PropertyDefaults.Signature,
           Option(relativeFileName).getOrElse(PropertyDefaults.Filename)
         )
 

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

@@ -5,14 +5,23 @@ import io.joern.php2cpg.parser.Domain.*
 import io.joern.php2cpg.utils.TypeScope
 import io.joern.x2cpg.{Ast, Defines, ValidationMode}
 import io.shiftleft.codepropertygraph.generated.nodes.{
+  NewBinding,
   NewCall,
   NewFieldIdentifier,
   NewIdentifier,
   NewMember,
+  NewMethod,
   NewTypeDecl,
   NewTypeRef
 }
-import io.shiftleft.codepropertygraph.generated.{DispatchTypes, ModifierTypes, NodeTypes, Operators}
+import io.shiftleft.codepropertygraph.generated.{
+  DispatchTypes,
+  EdgeTypes,
+  ModifierTypes,
+  NodeTypes,
+  Operators,
+  PropertyDefaults
+}
 import io.shiftleft.semanticcpg.language.types.structure.NamespaceTraversal
 
 trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this: AstCreator =>
@@ -52,6 +61,14 @@ trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this:
     Ast(typeDecl).withChildren(modifiers).withChildren(bodyStmts :+ clinitAst).withChildren(annotationAsts)
   }
 
+  private def addBindingsForTypeDecl(typeDecl: NewTypeDecl, bodyAsts: List[Ast]): Unit = {
+    bodyAsts.flatMap(_.root).collect { case method: NewMethod =>
+      val binding = NewBinding().name(method.name).methodFullName(method.fullName)
+      diffGraph.addEdge(typeDecl, binding, EdgeTypes.BINDS)
+      diffGraph.addEdge(binding, method, EdgeTypes.REF)
+    }
+  }
+
   private def astForAnonymousClass(
     stmt: PhpClassLikeStmt,
     dynamicStmts: List[PhpStmt],
@@ -140,6 +157,8 @@ trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this:
       diffGraph.addNode(metaTypeDeclMember)
     }
 
+    addBindingsForTypeDecl(typeDeclTemp, bodyStmts)
+
     val prefixAst = createTypeRefPointer(typeDeclTemp)
     val typeDeclAst = Ast(typeDeclTemp)
       .withChildren(modifiers)
@@ -245,6 +264,7 @@ trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this:
     scope.popScope()
 
     val classTypeDeclAst = Ast(typeDecl).withChildren(modifiers).withChildren(bodyStmts).withChildren(annotationAsts)
+    addBindingsForTypeDecl(typeDecl, bodyStmts)
 
     scope.pushNewScope(TypeScope(metaTypeDeclNode, metaTypeDeclFullName))
 

joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/CallTests.scala

@@ -73,7 +73,7 @@ class CallTests extends PhpCode2CpgFixture {
     inside(cpg.call.l) { case List(fooCall) =>
       fooCall.name shouldBe "foo"
       fooCall.methodFullName shouldBe s"foo"
-      fooCall.signature shouldBe s"${Defines.UnresolvedSignature}(1)"
+      fooCall.signature shouldBe ""
       fooCall.receiver.isEmpty shouldBe true
       fooCall.dispatchType shouldBe DispatchTypes.STATIC_DISPATCH
       fooCall.lineNumber shouldBe Some(2)

joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/ClosureTests.scala

@@ -81,7 +81,7 @@ class ClosureTests extends PhpCode2CpgFixture {
       val expectedName = s"foo.php:<global>.<lambda>0"
       closureMethod.name shouldBe expectedName
       closureMethod.fullName shouldBe expectedName
-      closureMethod.signature shouldBe s"${Defines.UnresolvedSignature}(1)"
+      closureMethod.signature shouldBe ""
       closureMethod.code shouldBe s"function $expectedName($$value) use($$use1, &$$use2)"
       closureMethod.parameter.size shouldBe 1
 
@@ -153,7 +153,7 @@ class ClosureTests extends PhpCode2CpgFixture {
       val expectedName = "foo.php:<global>.<lambda>0"
       closureMethod.name shouldBe expectedName
       closureMethod.fullName shouldBe expectedName
-      closureMethod.signature shouldBe s"${Defines.UnresolvedSignature}(1)"
+      closureMethod.signature shouldBe ""
       closureMethod.code shouldBe s"function $expectedName($$value)"
       closureMethod.parameter.size shouldBe 1
 

joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/MethodTests.scala

@@ -21,7 +21,7 @@ class MethodTests extends PhpCode2CpgFixture {
 
     inside(cpg.method.name("foo").l) { case List(fooMethod) =>
       fooMethod.fullName shouldBe "foo"
-      fooMethod.signature shouldBe s"${Defines.UnresolvedSignature}(0)"
+      fooMethod.signature shouldBe ""
       fooMethod.lineNumber shouldBe Some(2)
       fooMethod.lineNumberEnd shouldBe Some(2)
       fooMethod.code shouldBe "function foo()"

joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/OperatorTests.scala

@@ -710,7 +710,7 @@ class OperatorTests extends PhpCode2CpgFixture {
       absCall.name shouldBe "abs"
       absCall.methodFullName shouldBe "abs"
       absCall.code shouldBe "abs($a)"
-      absCall.signature shouldBe s"${Defines.UnresolvedSignature}(1)"
+      absCall.signature shouldBe ""
     }
   }