[javasrc2cpg] Generate TYPE_REF for implicit base of static field acc…

…esses. (#5036) * [javasrc2cpg] Generate TYPE_REF for implicit base of static field accesses. For static field access like `String x = staticField1;` we used to generate a IDENTIFIER with the name of the class declaring the static field as base for the field access operator. Now we generate a TYPE_REF referencing the declaring type. This better represents the actual situation where there is no local variable for a class/type which in practice also resulted in a CPG format error because the IDENTIFIFER node was at least not in all cases linked to a LOCAL. While implementing this i reorganized the code a bit to avoid duplication. Also changes the `equals` semantic for TrackedTypeRef. Before it was based on the actual TYPE_REF node which does not make sense. Instead we need to compare the referenced TYPE itself to check if two different type references in code point to the same TYPE. * Handle statically imported field scenario.
joernio · Oct 31, 2024 · a792e0b · a792e0b
1 parent 784b2fd
commit a792e0b
Show file tree

Hide file tree

Showing 8 changed files with 159 additions and 103 deletions.
diff --git a/...ain/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForNameExpressionsCreator.scala b/...ain/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForNameExpressionsCreator.scala
@@ -1,12 +1,20 @@
 package io.joern.javasrc2cpg.astcreation.expressions
 
+import com.github.javaparser.ast.Node
 import com.github.javaparser.ast.expr.NameExpr
 import com.github.javaparser.resolution.declarations.ResolvedFieldDeclaration
 import io.joern.javasrc2cpg.astcreation.{AstCreator, ExpectedType}
+import io.joern.javasrc2cpg.scope.JavaScopeElement.TypeDeclScope
 import io.joern.javasrc2cpg.typesolvers.TypeInfoCalculator.TypeConstants
 import io.joern.javasrc2cpg.util.NameConstants
 import io.joern.x2cpg.{Ast, Defines}
-import io.shiftleft.codepropertygraph.generated.nodes.{NewLocal, NewMethodParameterIn}
+import io.shiftleft.codepropertygraph.generated.nodes.{
+  NewLocal,
+  NewMethodParameterIn,
+  NewTypeDecl,
+  NewTypeRef,
+  NewUnknown
+}
 
 import scala.util.Success
 import io.joern.javasrc2cpg.scope.Scope.{
@@ -17,12 +25,10 @@ import io.joern.javasrc2cpg.scope.Scope.{
   ScopeVariable,
   SimpleVariable
 }
-import io.shiftleft.codepropertygraph.generated.nodes.NewTypeDecl
 import org.slf4j.LoggerFactory
-import io.shiftleft.codepropertygraph.generated.nodes.NewUnknown
 import io.joern.x2cpg.utils.AstPropertiesUtil.*
 import io.shiftleft.codepropertygraph.generated.Operators
-import io.joern.x2cpg.utils.NodeBuilders.newOperatorCallNode
+import io.joern.x2cpg.utils.NodeBuilders.{newIdentifierNode, newOperatorCallNode}
 
 trait AstForNameExpressionsCreator { this: AstCreator =>
 
@@ -39,15 +45,13 @@ trait AstForNameExpressionsCreator { this: AstCreator =>
         astForStaticImportOrUnknown(nameExpr, name, typeFullName)
 
       case SimpleVariable(variable: ScopeMember) =>
-        val targetName =
-          Option.when(variable.isStatic)(scope.enclosingTypeDecl.fullName).flatten.getOrElse(NameConstants.This)
-        fieldAccessAst(
-          targetName,
-          scope.enclosingTypeDecl.fullName,
+        createImplicitBaseFieldAccess(
+          variable.isStatic,
+          scope.enclosingTypeDecl.name.get,
+          scope.enclosingTypeDecl.fullName.get,
+          nameExpr,
           variable.name,
-          Some(variable.typeFullName),
-          line(nameExpr),
-          column(nameExpr)
+          variable.typeFullName
         )
 
       case SimpleVariable(variable) =>
@@ -65,28 +69,48 @@ trait AstForNameExpressionsCreator { this: AstCreator =>
     }
   }
 
+  private[expressions] def createImplicitBaseFieldAccess(
+    isStatic: Boolean,
+    baseTypeDeclName: String,
+    baseTypeDeclFullName: String,
+    node: Node,
+    fieldName: String,
+    fieldTypeFullName: String
+  ): Ast = {
+    val base =
+      if (isStatic) {
+        NewTypeRef()
+          .code(baseTypeDeclName)
+          .typeFullName(baseTypeDeclFullName)
+          .lineNumber(line(node))
+          .columnNumber(column(node))
+      } else {
+        newIdentifierNode(NameConstants.This, baseTypeDeclFullName)
+      }
+    createFieldAccessAst(
+      Ast(base),
+      s"${base.code}.${fieldName}",
+      line(node),
+      column(node),
+      fieldName,
+      fieldTypeFullName,
+      line(node),
+      column(node)
+    )
+  }
+
   private def astForStaticImportOrUnknown(nameExpr: NameExpr, name: String, typeFullName: Option[String]): Ast = {
     tryWithSafeStackOverflow(nameExpr.resolve()) match {
-      case Success(value) if value.isField =>
-        val identifierName = if (value.asField.isStatic) {
-          // TODO: v is wrong. Statically imported expressions can also be represented by just the name.
-          // A static field represented by a NameExpr must belong to the class in which it's used. Static fields
-          // from other classes are represented by a FieldAccessExpr instead.
-          scope.enclosingTypeDecl.map(_.typeDecl.name).getOrElse(s"${Defines.UnresolvedNamespace}.$name")
-        } else {
-          NameConstants.This
-        }
-
-        val identifierTypeFullName =
-          value match {
-            case fieldDecl: ResolvedFieldDeclaration =>
-              // TODO It is not quite correct to use the declaring classes type.
-              // Instead we should take the using classes type which is either the same or a
-              // sub class of the declaring class.
-              typeInfoCalc.fullName(fieldDecl.declaringType())
-          }
-
-        fieldAccessAst(identifierName, identifierTypeFullName, name, typeFullName, line(nameExpr), column(nameExpr))
+      case Success(value: ResolvedFieldDeclaration) =>
+        // TODO using the enclosingTypeDecl is wrong if the field was imported via a static import.
+        createImplicitBaseFieldAccess(
+          value.asField().isStatic,
+          typeInfoCalc.name(value.declaringType()).getOrElse(TypeConstants.Any),
+          typeInfoCalc.fullName(value.declaringType()).getOrElse(TypeConstants.Any),
+          nameExpr,
+          name,
+          typeFullName.getOrElse(TypeConstants.Any)
+        )
 
       case _ =>
         Ast(identifierNode(nameExpr, name, name, typeFullName.getOrElse(TypeConstants.Any)))

diff --git a/...n/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForSimpleExpressionsCreator.scala b/...n/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForSimpleExpressionsCreator.scala
@@ -250,26 +250,54 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
     astsForExpression(expr.getInner, expectedType)
   }
 
+  private[expressions] def createFieldAccessAst(
+    base: Ast,
+    fieldAccessCode: String,
+    fieldAccessLineNo: Option[Int],
+    fieldAccessColumnNo: Option[Int],
+    fieldName: String,
+    fieldTypeFullName: String,
+    fieldLineNo: Option[Int],
+    fieldColumnNo: Option[Int]
+  ): Ast = {
+    val callNode =
+      newOperatorCallNode(
+        Operators.fieldAccess,
+        fieldAccessCode,
+        Some(fieldTypeFullName),
+        fieldAccessLineNo,
+        fieldAccessColumnNo
+      )
+
+    val fieldIdentifierNode = NewFieldIdentifier()
+      .code(fieldName)
+      .canonicalName(fieldName)
+      .lineNumber(fieldLineNo)
+      .columnNumber(fieldColumnNo)
+
+    callAst(callNode, Seq(base, Ast(fieldIdentifierNode)))
+  }
+
   private[expressions] def astForFieldAccessExpr(expr: FieldAccessExpr, expectedType: ExpectedType): Ast = {
     val typeFullName =
       expressionReturnTypeFullName(expr)
         .orElse(expectedType.fullName)
         .map(typeInfoCalc.registerType)
         .getOrElse(TypeConstants.Any)
 
-    val callNode =
-      newOperatorCallNode(Operators.fieldAccess, expr.toString, Some(typeFullName), line(expr), column(expr))
-
     val fieldIdentifier = expr.getName
     val identifierAsts  = astsForExpression(expr.getScope, ExpectedType.empty)
-    val fieldIdentifierNode = NewFieldIdentifier()
-      .canonicalName(fieldIdentifier.toString)
-      .lineNumber(line(fieldIdentifier))
-      .columnNumber(column(fieldIdentifier))
-      .code(fieldIdentifier.toString)
-    val fieldIdAst = Ast(fieldIdentifierNode)
 
-    callAst(callNode, identifierAsts ++ Seq(fieldIdAst))
+    createFieldAccessAst(
+      identifierAsts.head,
+      expr.toString,
+      line(expr),
+      column(expr),
+      fieldIdentifier.toString,
+      typeFullName,
+      line(fieldIdentifier),
+      column(fieldIdentifier)
+    )
   }
 
   private[expressions] def astForInstanceOfExpr(expr: InstanceOfExpr): Ast = {
@@ -291,41 +319,6 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
     callAst(callNode, exprAst ++ Seq(typeAst))
   }
 
-  private[expressions] def fieldAccessAst(
-    identifierName: String,
-    identifierType: Option[String],
-    fieldIdentifierName: String,
-    returnType: Option[String],
-    lineNo: Option[Int],
-    columnNo: Option[Int]
-  ): Ast = {
-    val typeFullName     = identifierType.orElse(Some(TypeConstants.Any)).map(typeInfoCalc.registerType)
-    val identifier       = newIdentifierNode(identifierName, typeFullName.getOrElse("ANY"))
-    val maybeCorrespNode = scope.lookupVariable(identifierName).variableNode
-
-    val fieldIdentifier = NewFieldIdentifier()
-      .code(fieldIdentifierName)
-      .canonicalName(fieldIdentifierName)
-      .lineNumber(lineNo)
-      .columnNumber(columnNo)
-
-    val fieldAccessCode = s"$identifierName.$fieldIdentifierName"
-    val fieldAccess =
-      newOperatorCallNode(
-        Operators.fieldAccess,
-        fieldAccessCode,
-        returnType.orElse(Some(TypeConstants.Any)),
-        lineNo,
-        columnNo
-      )
-
-    val identifierAst = Ast(identifier)
-    val fieldIdentAst = Ast(fieldIdentifier)
-
-    callAst(fieldAccess, Seq(identifierAst, fieldIdentAst))
-      .withRefEdges(identifier, maybeCorrespNode.toList)
-  }
-
   private[expressions] def astForLiteralExpr(expr: LiteralExpr): Ast = {
     val typeFullName = expressionReturnTypeFullName(expr).map(typeInfoCalc.registerType).getOrElse(TypeConstants.Any)
     val literalNode =

diff --git a/...n/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForVarDeclAndAssignsCreator.scala b/...n/scala/io/joern/javasrc2cpg/astcreation/expressions/AstForVarDeclAndAssignsCreator.scala
@@ -19,6 +19,7 @@ import io.shiftleft.codepropertygraph.generated.nodes.{
   NewIdentifier,
   NewLocal,
   NewMember,
+  NewTypeRef,
   NewUnknown
 }
 import io.shiftleft.codepropertygraph.generated.{DispatchTypes, Operators}
@@ -28,6 +29,7 @@ import scala.jdk.CollectionConverters.*
 import scala.jdk.OptionConverters.RichOptional
 import scala.util.{Failure, Success, Try}
 import io.joern.javasrc2cpg.scope.JavaScopeElement.PartialInit
+import io.joern.x2cpg.utils.NodeBuilders.newIdentifierNode
 
 trait AstForVarDeclAndAssignsCreator { this: AstCreator =>
   private val logger = LoggerFactory.getLogger(this.getClass())
@@ -151,17 +153,13 @@ trait AstForVarDeclAndAssignsCreator { this: AstCreator =>
       case Some(declarationNode) =>
         val assignmentTarget = declarationNode match {
           case member: NewMember =>
-            val name =
-              if (scope.isEnclosingScopeStatic)
-                scope.enclosingTypeDecl.map(_.typeDecl.name).getOrElse(NameConstants.Unknown)
-              else NameConstants.This
-            fieldAccessAst(
-              name,
-              scope.enclosingTypeDecl.fullName,
-              declarationNode.name,
-              Option(declarationNode.typeFullName),
-              line(originNode),
-              column(originNode)
+            createImplicitBaseFieldAccess(
+              scope.isEnclosingScopeStatic,
+              scope.enclosingTypeDecl.name.get,
+              scope.enclosingTypeDecl.fullName.get,
+              originNode,
+              variableName,
+              declarationNode.typeFullName
             )
 
           case variable =>

diff --git a/...ntends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/AnonymousClassTests.scala b/...ntends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/AnonymousClassTests.scala
@@ -3,7 +3,15 @@ package io.joern.javasrc2cpg.querying
 import io.joern.javasrc2cpg.JavaSrc2Cpg
 import io.shiftleft.semanticcpg.language.*
 import io.joern.javasrc2cpg.testfixtures.JavaSrcCode2CpgFixture
-import io.shiftleft.codepropertygraph.generated.nodes.{Binding, Block, Call, FieldIdentifier, Identifier, TypeDecl}
+import io.shiftleft.codepropertygraph.generated.nodes.{
+  Binding,
+  Block,
+  Call,
+  FieldIdentifier,
+  Identifier,
+  TypeDecl,
+  TypeRef
+}
 import io.shiftleft.codepropertygraph.generated.Operators
 
 class AnonymousClassTests extends JavaSrcCode2CpgFixture {
@@ -178,10 +186,8 @@ class AnonymousClassTests extends JavaSrcCode2CpgFixture {
           fieldAccess.name shouldBe Operators.fieldAccess
           fieldAccess.typeFullName shouldBe "foo.Bar"
 
-          inside(fieldAccess.argument.l) { case List(fooIdentifier: Identifier, bField: FieldIdentifier) =>
-            fooIdentifier.name shouldBe "Foo"
-            fooIdentifier.typeFullName shouldBe "foo.Foo"
-
+          inside(fieldAccess.argument.l) { case List(typeRef: TypeRef, bField: FieldIdentifier) =>
+            typeRef.typeFullName shouldBe "foo.Foo"
             bField.canonicalName shouldBe "b"
           }
         }

diff --git a/...ends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/ControlStructureTests.scala b/...ends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/ControlStructureTests.scala
@@ -11,7 +11,8 @@ import io.shiftleft.codepropertygraph.generated.nodes.{
   Identifier,
   Literal,
   Local,
-  Return
+  Return,
+  TypeRef
 }
 import io.shiftleft.semanticcpg.language.*
 
@@ -155,10 +156,8 @@ class NewControlStructureTests extends JavaSrcCode2CpgFixture {
           fieldAccess.name shouldBe Operators.fieldAccess
           fieldAccess.typeFullName shouldBe "java.lang.String[]"
 
-          inside(fieldAccess.argument.l) { case List(barIdentifier: Identifier, staticArr: FieldIdentifier) =>
-            barIdentifier.name shouldBe "Bar"
-            barIdentifier.typeFullName shouldBe "Bar"
-
+          inside(fieldAccess.argument.l) { case List(barTypeRef: TypeRef, staticArr: FieldIdentifier) =>
+            barTypeRef.typeFullName shouldBe "Bar"
             staticArr.canonicalName shouldBe "STATIC_ARR"
           }
         }

diff --git a/...frontends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/FieldAccessTests.scala b/...frontends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/FieldAccessTests.scala
@@ -1,7 +1,7 @@
 package io.joern.javasrc2cpg.querying
 
 import io.joern.javasrc2cpg.testfixtures.JavaSrcCode2CpgFixture
-import io.shiftleft.codepropertygraph.generated.nodes.{Call, FieldIdentifier, Identifier}
+import io.shiftleft.codepropertygraph.generated.nodes.{Call, FieldIdentifier, Identifier, TypeRef}
 import io.shiftleft.semanticcpg.language.*
 
 class FieldAccessTests extends JavaSrcCode2CpgFixture {
@@ -82,4 +82,28 @@ class FieldAccessTests extends JavaSrcCode2CpgFixture {
     access.referencedMember.name.head shouldBe "value"
   }
 
+  "correctly handle access to statically imported field" in {
+    val cpg = code("""
+        |import static Bar.STATIC_INT;
+        |public class Foo {
+        |  public void foo() {
+        |    int x = STATIC_INT;
+        |  }
+        |}
+        |""".stripMargin)
+      .moreCode(
+        """
+          |public class Bar {
+          |  public static int STATIC_INT = 111;
+          |}
+          |""".stripMargin,
+        fileName = "Bar.java"
+      )
+
+    val List(assignment) = cpg.call.code("int x = STATIC_INT").l
+    val fieldAccess      = assignment.argument(2).asInstanceOf[Call]
+    val typeRef          = fieldAccess.argument(1).asInstanceOf[TypeRef]
+    typeRef.typeFullName shouldBe "Bar"
+  }
+
 }
diff --git a/...n-cli/frontends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/ScopeTests.scala b/...n-cli/frontends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/ScopeTests.scala
@@ -2,7 +2,7 @@ package io.joern.javasrc2cpg.querying
 
 import io.joern.javasrc2cpg.testfixtures.JavaSrcCode2CpgFixture
 import io.shiftleft.codepropertygraph.generated.Operators
-import io.shiftleft.codepropertygraph.generated.nodes.{Call, FieldIdentifier, Identifier}
+import io.shiftleft.codepropertygraph.generated.nodes.{Call, FieldIdentifier, Identifier, TypeRef}
 import io.shiftleft.semanticcpg.language.*
 
 class ScopeTests extends JavaSrcCode2CpgFixture {
@@ -151,9 +151,8 @@ class ScopeTests extends JavaSrcCode2CpgFixture {
         fieldAccess.methodFullName shouldBe Operators.fieldAccess
         fieldAccess.typeFullName shouldBe "java.lang.Object"
         fieldAccess.argument.l match {
-          case List(identifier: Identifier, fieldIdentifier: FieldIdentifier) =>
-            identifier.name shouldBe "Test"
-            identifier.typeFullName shouldBe "Test"
+          case List(typeRef: TypeRef, fieldIdentifier: FieldIdentifier) =>
+            typeRef.typeFullName shouldBe "Test"
             fieldIdentifier.canonicalName shouldBe "staticO"
           case res => fail(s"Expected field access args but got $res")
         }