Skip to content

Commit

Permalink
markus/koltinBugfixes (#5200)
Browse files Browse the repository at this point in the history
* Fix handling of lambda functions wrapped by constructs like labels.

This resulted in a NoSuchElementException because the lambda expression
could not be associated with the correct call argument.

* Handle case where a type constructor has no declaration descriptor.

This case is unexpected. We now avoid the MatchErrorException return
None and log in order to not crash and burn and to understand the
situation better.

* Fix broken CPG in case variable declaration without initialization.
  • Loading branch information
ml86 authored Jan 3, 2025
1 parent cc0785b commit 56463fd
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -713,15 +713,19 @@ trait AstForDeclarationsCreator(implicit withSchemaValidation: ValidationMode) {
scope.addToScope(expr.getName, node)
val localAst = Ast(node)

val rhsAsts = astsForExpression(expr.getDelegateExpressionOrInitializer, Some(2))
val identifier = identifierNode(elem, elem.getText, elem.getText, typeFullName)
val identifierAst = astWithRefEdgeMaybe(identifier.name, identifier)
val assignmentNode =
NodeBuilders.newOperatorCallNode(Operators.assignment, expr.getText, None, line(expr), column(expr))
val call =
callAst(assignmentNode, List(identifierAst) ++ rhsAsts)
.withChildren(annotations.map(astForAnnotationEntry))
Seq(localAst, call)
if (expr.getDelegateExpressionOrInitializer != null) {
val rhsAsts = astsForExpression(expr.getDelegateExpressionOrInitializer, Some(2))
val identifier = identifierNode(elem, elem.getText, elem.getText, typeFullName)
val identifierAst = astWithRefEdgeMaybe(identifier.name, identifier)
val assignmentNode =
NodeBuilders.newOperatorCallNode(Operators.assignment, expr.getText, None, line(expr), column(expr))
val call =
callAst(assignmentNode, List(identifierAst) ++ rhsAsts)
.withChildren(annotations.map(astForAnnotationEntry))
Seq(localAst, call)
} else {
Seq(localAst)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,19 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) {
// Lambda is wrapped e.g. `SomeInterface { obj -> obj }`
samConstructorDesc.getBaseDescriptorForSynthetic
case _ =>
// Lambda/anan function is directly used as call argument e.g. `someCall(obj -> obj)`
// Lambda/anon function is directly used as call argument e.g. `someCall(obj -> obj)`
val directCallArgumentForLookup =
expr match {
case _: KtNamedFunction =>
// This is the anonymous function case.
// So far it does not seem like those could be wrapped so they are always the direct argument.
expr
case _ =>
// The lambda function case.
getDirectLambdaArgument(expr).get
}
callAtom.getArgumentMappingByOriginal.asScala.collectFirst {
case (paramDesc, resolvedArgument) if isExprIncluded(resolvedArgument, expr) =>
case (paramDesc, resolvedArgument) if isExprIncluded(resolvedArgument, directCallArgumentForLookup) =>
paramDesc.getType.getConstructor.getDeclarationDescriptor.asInstanceOf[ClassDescriptor]
}.get
}
Expand All @@ -571,6 +581,28 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) {
}
}

private def getDirectLambdaArgument(element: KtElement): Option[KtExpression] = {
var context: PsiElement = element
var parentContext: PsiElement = null

// KtCallExpression wrap their arguments in KtValueArgument which is why we look for those.
// KtBinaryExpressions do not do such a wrapping.
while ({
parentContext = context.getContext
parentContext != null &&
!parentContext.isInstanceOf[KtValueArgument] &&
!parentContext.isInstanceOf[KtBinaryExpression]
}) {
context = parentContext
}

if (parentContext != null) {
Some(context.asInstanceOf[KtExpression])
} else {
None
}
}

private def getSurroundingCallTarget(element: KtElement): Option[KtExpression] = {
var context: PsiElement = element.getContext
while (
Expand All @@ -591,6 +623,7 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) {
}

private def isExprIncluded(resolvedArgument: ResolvedCallArgument, expr: KtExpression): Boolean = {
// getArguments returns multiple arguments in case of varargs
resolvedArgument.getArguments.asScala.exists {
case psi: PSIFunctionKotlinCallArgument =>
psi.getExpression == expr
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.joern.kotlin2cpg.types

import io.joern.kotlin2cpg.types.NameRenderer.BuiltinTypeTranslationTable
import io.joern.kotlin2cpg.types.NameRenderer.{BuiltinTypeTranslationTable, logger}
import io.joern.x2cpg.Defines
import org.jetbrains.kotlin.builtins.jvm.JavaToKotlinClassMap
import org.jetbrains.kotlin.descriptors.impl.TypeAliasConstructorDescriptor
Expand All @@ -16,11 +16,14 @@ import org.jetbrains.kotlin.descriptors.{
import org.jetbrains.kotlin.name.FqNameUnsafe
import org.jetbrains.kotlin.types.KotlinType
import org.jetbrains.kotlin.types.error.ErrorClassDescriptor
import org.slf4j.LoggerFactory

import scala.collection.mutable
import scala.jdk.CollectionConverters.*

object NameRenderer {
private val logger = LoggerFactory.getLogger(getClass)

private val BuiltinTypeTranslationTable = mutable.HashMap(
"kotlin.Unit" -> "void",
"kotlin.Boolean" -> "boolean",
Expand Down Expand Up @@ -111,6 +114,13 @@ class NameRenderer {
} else {
Some(upperBoundTypeFns.flatten.mkString("&"))
}
case null =>
// We do not expect this because to my understanding a typ should always have a constructor
// descriptor.
logger.warn(
s"Found type without constructor descriptor. Typ: $typ Constructor class: ${typ.getConstructor.getClass}"
)
None
}

javaFullName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,4 +748,33 @@ class LambdaTests extends KotlinCode2CpgFixture(withOssDataflow = false, withDef
binding2.bindingTypeDecl shouldBe lambdaTypeDecl
}
}

"CPG for code with lambda wrapped in label" should {
val cpg = code("""
|package mypkg
|fun outer() {
| listOf(1).forEach someLabel@{x: Int -> x}
|}
|""".stripMargin)

"contain correct lambda, bindings and type decl nodes" in {
val List(lambdaMethod) = cpg.method.fullName(".*lambda.*").l
lambdaMethod.fullName shouldBe s"mypkg.outer.${Defines.ClosurePrefix}0:void(int)"
lambdaMethod.signature shouldBe "void(int)"

val List(lambdaTypeDecl) = lambdaMethod.bindingTypeDecl.dedup.l
lambdaTypeDecl.fullName shouldBe s"mypkg.outer.${Defines.ClosurePrefix}0"
lambdaTypeDecl.inheritsFromTypeFullName should contain theSameElementsAs (List("kotlin.Function1"))

val List(binding1, binding2) = lambdaMethod.referencingBinding.l
binding1.name shouldBe "invoke"
binding1.signature shouldBe "void(int)"
binding1.methodFullName shouldBe s"mypkg.outer.${Defines.ClosurePrefix}0:void(int)"
binding1.bindingTypeDecl shouldBe lambdaTypeDecl
binding2.name shouldBe "invoke"
binding2.signature shouldBe "java.lang.Object(java.lang.Object)"
binding2.methodFullName shouldBe s"mypkg.outer.${Defines.ClosurePrefix}0:void(int)"
binding2.bindingTypeDecl shouldBe lambdaTypeDecl
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,20 @@ class LocalTests extends KotlinCode2CpgFixture(withOssDataflow = false) {
l2.typeFullName shouldBe "int"
}
}

"CPG for local declaration without initialization" should {
val cpg = code("""
|fun main() {
| var x: Int
|}
|""".stripMargin)

"contain LOCAL node for `x`" in {
val List(l1) = cpg.local("x").l
l1.code shouldBe "x"
l1.name shouldBe "x"
l1.typeFullName shouldBe "int"
}
}

}

0 comments on commit 56463fd

Please sign in to comment.