Repository: joernio/joern (scala)

Issue:
php2cpg: model true/false as literals instead of field access
I don't believe it breaks anything, but `true/false` currently being field accesses seems weird to me.

Pull request:
Fixes https://github.com/joernio/joern/issues/2866
Fixes https://github.com/joernio/joern/issues/2822

Patch:
diff --git a/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreator.scala b/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreator.scala
index 369d0ece8e0c..155226cb676b 100644
--- a/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreator.scala
+++ b/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/astcreation/AstCreator.scala
@@ -1651,29 +1651,55 @@ class AstCreator(filename: String, phpAst: PhpFile)
     callAst(callNode, args :: Nil)
   }
 
-  private def astForClassConstFetchExpr(expr: PhpClassConstFetchExpr): Ast = {
-    val target              = astForExpr(expr.className)
-    val fieldIdentifierName = expr.constantName.map(_.name).getOrElse(NameConstants.Unknown)
+  private def astForMagicClassConstant(expr: PhpClassConstFetchExpr): Ast = {
+    val classAst = astForExpr(expr.className)
+    val typeFullName = expr.className match {
+      case nameExpr: PhpNameExpr =>
+        scope
+          .lookupVariable(nameExpr.name)
+          .flatMap(_.properties.get(PropertyNames.TYPE_FULL_NAME).map(_.toString))
+          .getOrElse(nameExpr.name)
 
-    val fieldIdentifier = newFieldIdentifierNode(fieldIdentifierName, line(expr))
+      case expr =>
+        classAst.rootType.orElse(classAst.rootName).getOrElse(UnresolvedNamespace)
+    }
 
-    val fieldAccessCode = s"${target.rootCodeOrEmpty}::${fieldIdentifier.code}"
+    Ast(typeRefNode(expr, classAst.rootCodeOrEmpty, typeFullName))
+  }
 
-    val fieldAccessCall = newOperatorCallNode(Operators.fieldAccess, fieldAccessCode, line = line(expr))
+  private def astForClassConstFetchExpr(expr: PhpClassConstFetchExpr): Ast = {
+    expr.constantName match {
+      // Foo::class should be a TypeRef and not a field access
+      case Some(constNameExpr) if constNameExpr.name == NameConstants.Class =>
+        astForMagicClassConstant(expr)
 
-    callAst(fieldAccessCall, List(target, Ast(fieldIdentifier)))
+      case _ =>
+        val targetAst           = astForExpr(expr.className)
+        val fieldIdentifierName = expr.constantName.map(_.name).getOrElse(NameConstants.Unknown)
+        val fieldIdentifier     = newFieldIdentifierNode(fieldIdentifierName, line(expr))
+        val fieldAccessCode     = s"${targetAst.rootCodeOrEmpty}::${fieldIdentifier.code}"
+        val fieldAccessCall     = newOperatorCallNode(Operators.fieldAccess, fieldAccessCode, line = line(expr))
+        callAst(fieldAccessCall, List(targetAst, Ast(fieldIdentifier)))
+    }
   }
 
   private def astForConstFetchExpr(expr: PhpConstFetchExpr): Ast = {
+    val constName = expr.name.name
 
-    val identifier =
-      identifierNode(expr, NamespaceTraversal.globalNamespaceName, NamespaceTraversal.globalNamespaceName, "ANY")
-    val fieldIdentifier = newFieldIdentifierNode(expr.name.name, line = line(expr))
+    if (NameConstants.isBoolean(constName)) {
+      Ast(literalNode(expr, constName, TypeConstants.Bool))
+    } else if (NameConstants.isNull(constName)) {
+      Ast(literalNode(expr, constName, TypeConstants.NullType))
+    } else {
+      val namespaceName   = NamespaceTraversal.globalNamespaceName
+      val identifier      = identifierNode(expr, namespaceName, namespaceName, "ANY")
+      val fieldIdentifier = newFieldIdentifierNode(constName, line = line(expr))
 
-    val fieldAccessNode = newOperatorCallNode(Operators.fieldAccess, code = expr.name.name, line = line(expr))
-    val args            = List(identifier, fieldIdentifier).map(Ast(_))
+      val fieldAccessNode = newOperatorCallNode(Operators.fieldAccess, code = constName, line = line(expr))
+      val args            = List(identifier, fieldIdentifier).map(Ast(_))
 
-    callAst(fieldAccessNode, args)
+      callAst(fieldAccessNode, args)
+    }
   }
 
   protected def line(phpNode: PhpNode): Option[Integer]      = phpNode.attributes.lineNumber
@@ -1691,6 +1717,7 @@ object AstCreator {
     val Void: String                = "void"
     val Any: String                 = "ANY"
     val Array: String               = "array"
+    val NullType: String            = "null"
     val VariadicPlaceholder: String = "PhpVariadicPlaceholder"
   }
 
@@ -1700,6 +1727,18 @@ object AstCreator {
     val This: String         = "this"
     val Unknown: String      = "UNKNOWN"
     val Closure: String      = "__closure"
+    val Class: String        = "class"
+    val True: String         = "true";
+    val False: String        = "false";
+    val NullName: String     = "null";
+
+    def isBoolean(name: String): Boolean = {
+      List(True, False).contains(name)
+    }
+
+    def isNull(name: String): Boolean = {
+      name.toLowerCase == NullName
+    }
   }
 
   val operatorSymbols: Map[String, String] = Map(
diff --git a/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/utils/AstPropertiesUtil.scala b/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/utils/AstPropertiesUtil.scala
index ce79d840ab71..2257502407fc 100644
--- a/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/utils/AstPropertiesUtil.scala
+++ b/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/utils/AstPropertiesUtil.scala
@@ -15,6 +15,8 @@ object AstPropertiesUtil {
 
     def rootCode: Option[String] = rootProperty(PropertyNames.CODE)
 
+    def rootName: Option[String] = rootProperty(PropertyNames.NAME)
+
     def rootCodeOrEmpty: String = rootCode.getOrElse("")
 
   }
@@ -25,6 +27,8 @@ object AstPropertiesUtil {
 
     def rootCode: Option[String] = asts.headOption.flatMap(_.rootCode)
 
+    def rootName: Option[String] = asts.headOption.flatMap(_.rootName)
+
     def rootCodeOrEmpty: String = asts.rootCode.getOrElse("")
   }
 }


Test patch:
diff --git a/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/ScalarTests.scala b/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/ScalarTests.scala
index 1a295b2b3dff..525cf45b6467 100644
--- a/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/ScalarTests.scala
+++ b/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/ScalarTests.scala
@@ -61,4 +61,27 @@ class ScalarTests extends PhpCode2CpgFixture {
       }
     }
   }
+
+  "booleans should be represented as literals" in {
+    val cpg = code("<?php\ntrue; false;")
+
+    inside(cpg.literal.l) { case List(trueBool, falseBool) =>
+      trueBool.code shouldBe "true"
+      trueBool.lineNumber shouldBe Some(2)
+      trueBool.typeFullName shouldBe "bool"
+
+      falseBool.code shouldBe "false"
+      falseBool.lineNumber shouldBe Some(2)
+      falseBool.typeFullName shouldBe "bool"
+    }
+  }
+  "null should be represented as a literal" in {
+    val cpg = code("<?php\nNULL;")
+
+    inside(cpg.literal.l) { case List(nullLiteral) =>
+      nullLiteral.code shouldBe "NULL"
+      nullLiteral.lineNumber shouldBe Some(2)
+      nullLiteral.typeFullName shouldBe "null"
+    }
+  }
 }
diff --git a/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/TypeDeclTests.scala b/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/TypeDeclTests.scala
index dc48cebec39c..24b0a47828c8 100644
--- a/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/TypeDeclTests.scala
+++ b/joern-cli/frontends/php2cpg/src/test/scala/io/joern/php2cpg/querying/TypeDeclTests.scala
@@ -6,6 +6,8 @@ import io.shiftleft.codepropertygraph.generated.{ModifierTypes, Operators}
 import io.shiftleft.codepropertygraph.generated.nodes.{Call, Identifier, Literal, Local, Member, Method}
 import io.shiftleft.semanticcpg.language._
 import io.shiftleft.codepropertygraph.generated.nodes.Block
+import io.shiftleft.codepropertygraph.generated.nodes.MethodRef
+import io.shiftleft.codepropertygraph.generated.nodes.TypeRef
 
 class TypeDeclTests extends PhpCode2CpgFixture {
 
@@ -246,4 +248,39 @@ class TypeDeclTests extends PhpCode2CpgFixture {
 
     cpg.typeDecl.nameExact("<global>").fullName.l shouldBe List("foo.php:<global>")
   }
+
+  "class magic constants" when {
+    "called on a class name should give the fully qualified class name" in {
+      val cpg = code("""<?php
+        |namespace foo;
+        |class Foo {}
+        |
+        |function test() {
+        |  Foo::class;
+        |}
+        |""")
+
+      inside(cpg.method.name("test").body.astChildren.l) { case List(fooRef: TypeRef) =>
+        fooRef.typeFullName shouldBe "foo\\Foo"
+        fooRef.lineNumber shouldBe Some(6)
+      }
+    }
+
+    "called on an object should give the fully qualified type name" in {
+      val cpg = code("""<?php
+        |namespace foo;
+        |class Foo {}
+        |
+        |function test(Foo $f) {
+        |  $f::class;
+        |}
+        |""")
+
+      inside(cpg.method.name("test").body.astChildren.l) { case List(fooRef: TypeRef) =>
+        // TODO The typeFullName here is missing, even though we should get it. Fix with types in general.
+        // fooRef.typeFullName shouldBe "foo\\Foo"
+        fooRef.lineNumber shouldBe Some(6)
+      }
+    }
+  }
 }