Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaSrc2Cpg: Infer type by Namespace and arg/parameter size #4434

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

JavaSrc2Cpg: Infer type by Namespace and arg/parameter size #4434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb70dda
add - support to infer type if parameter / argument size matches
khemrajrathore Apr 8, 2024
8486748
minor refactor
khemrajrathore Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions .../frontends/javasrc2cpg/src/main/scala/io/joern/javasrc2cpg/passes/TypeInferencePass.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class TypeInferencePass(cpg: Cpg) extends ConcurrentWriterCpgPass[Call](cpg) {
.filterNot(_.fullName.startsWith(Defines.UnresolvedNamespace))
.filterNot(_.signature.startsWith(Defines.UnresolvedSignature))
.groupBy(_.name)

private case class NameParts(typeDecl: Option[String], signature: String)

override def generateParts(): Array[Call] = {
Expand All @@ -32,7 +31,12 @@ class TypeInferencePass(cpg: Cpg) extends ConcurrentWriterCpgPass[Call](cpg) {
.toArray
}

private def isMatchingMethod(method: Method, call: Call, callNameParts: NameParts): Boolean = {
private def isMatchingMethod(
method: Method,
call: Call,
callNameParts: NameParts,
ignoreArgTypes: Boolean = false
): Boolean = {
// An erroneous `this` argument is added for unresolved calls to static methods.
val argSizeMod = if (method.modifier.modifierType.iterator.contains(ModifierTypes.STATIC)) 1 else 0
lazy val methodNameParts = getNameParts(method.name, method.fullName)
Expand All @@ -44,7 +48,8 @@ class TypeInferencePass(cpg: Cpg) extends ConcurrentWriterCpgPass[Call](cpg) {

lazy val typeDeclMatches = (callNameParts.typeDecl == methodNameParts.typeDecl)

parameterSizesMatch && argTypesMatch && typeDeclMatches
if ignoreArgTypes then parameterSizesMatch && typeDeclMatches
else parameterSizesMatch && argTypesMatch && typeDeclMatches
}

/** Check if argument types match by comparing exact full names. An argument type of `ANY` always matches.
Expand Down Expand Up @@ -87,19 +92,41 @@ class TypeInferencePass(cpg: Cpg) extends ConcurrentWriterCpgPass[Call](cpg) {
cache.get(callKey).toScala.getOrElse {
val callNameParts = getNameParts(call.name, call.methodFullName)
resolvedMethodIndex.get(call.name).flatMap { candidateMethods =>
val candidateMethodsIter = candidateMethods.iterator
val uniqueMatchingMethod =
candidateMethodsIter.find(isMatchingMethod(_, call, callNameParts)).flatMap { method =>
val otherMatchingMethod = candidateMethodsIter.find(isMatchingMethod(_, call, callNameParts))
// Only return a resulting method if exactly one matching method is found.
Option.when(otherMatchingMethod.isEmpty)(method)
}
val uniqueMatchingMethod = retreiveMatchingMethod(candidateMethods, call, callNameParts) match {
case Some(method) => Some(method)
case None => retreiveMatchingMethod(candidateMethods, call, callNameParts, ignoreArgTypes = true)
}
cache.put(callKey, uniqueMatchingMethod)
uniqueMatchingMethod
}
}
}

/** Return a method only if there exists a one to one mapping of call to method node
* @param candidateMethods
* @param call
* @param callNameParts
* @param ignoreArgTypes
* @return
*/
private def retreiveMatchingMethod(
candidateMethods: List[Method],
call: Call,
callNameParts: NameParts,
ignoreArgTypes: Boolean = false
): Option[Method] = {
val candidateMethodsIter = candidateMethods.iterator
val uniqueMatchingMethod =
candidateMethodsIter.find(isMatchingMethod(_, call, callNameParts, ignoreArgTypes = ignoreArgTypes)).flatMap {
method =>
val otherMatchingMethod =
candidateMethodsIter.find(isMatchingMethod(_, call, callNameParts, ignoreArgTypes = ignoreArgTypes))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An iterator can only be used once, and I see it used twice. Rather get rid of candidateMethodsIter and just call candidateMethods.start or candidateMethods.iterator when you need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for calling it twice is

  • The first iterator call gives us the first match and the iterator stops there
  • we use the same iterator to continue the search to see if we get another matching method

This is an optimization than calling the iterator from the start again

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, iterators cannot be used more than once. I've verified this in a shell for you e.g.

scala> val x = Iterator(1, 2, 3)
val x: Iterator[Int] = <iterator>
                                                                                                                                                                                                                                                                                                                                                          
scala> x.toList
val res2: List[Int] = List(1, 2, 3)
                                                                                                                                                                                                                                                                                                                                                          
scala> x.toList
val res3: List[Int] = List()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional (maybe a comment is required in code to explain this) and is an optimisation to avoid having to traverse the entire iterator twice.

The first call to candidateMethodsIter.find consumes iterator elements until the first match, but stops at this point. The second find call then continues the search until a second match is found.

To illustrate with an example considering only the first find call:

scala> val x = Iterator(1, 2, 3, 2, 4)
val x: Iterator[Int] = <iterator>
                                                                                               
scala> x.find(_ == 2)
val res3: Option[Int] = Some(2)
                                                                                               
scala> x.toList
val res4: List[Int] = List(3, 2, 4)

And for both calls:

scala> val x = Iterator(1, 2, 3, 2, 4)
val x: Iterator[Int] = <iterator>
                                                                                               
scala> x.find(_ == 2)
val res5: Option[Int] = Some(2)
                                                                                               
scala> x.find(_ == 2)
val res6: Option[Int] = Some(2)
                                                                                               
scala> x.toList
val res7: List[Int] = List(4)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, find won't consume the iterator fully. Instead for condition = true, find will return the matched item to the flatMap for further processing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah now I understand, TDIL! Thanks

// Only return a resulting method if exactly one matching method is found.
Option.when(otherMatchingMethod.isEmpty)(method)
}
uniqueMatchingMethod
}

override def runOnPart(diffGraph: DiffGraphBuilder, call: Call): Unit = {
getReplacementMethod(call).foreach { replacementMethod =>
diffGraph.setNodeProperty(call, PropertyNames.MethodFullName, replacementMethod.fullName)
Expand Down
39 changes: 39 additions & 0 deletions ...ontends/javasrc2cpg/src/test/scala/io/joern/javasrc2cpg/querying/TypeInferenceTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,42 @@ class TypeInferenceTests extends JavaSrcCode2CpgFixture {
}

}

class TypeInferenceByArgSizeTests extends JavaSrcCode2CpgFixture {
"Calls having same namespace as a method" should {
val cpg = code("""
|import com.myorg.client.Client;
|import com.myorg.config.RestConfigBase;
|import com.myorg.config.RestConfig;
|import com.myorg.interceptor.Interceptor;
|
|public class Sample {
|
| public static Client createClient(RestConfigBase config) {
| return new MyClient(config);
| }
|
| public static Client createClient(RestConfigBase config, Interceptor interceptor) {
| return new MyClient(config, interceptor);
| }
|
| Client getClient() {
| return Sample.createClient(new RestConfig("someUrl", "othervalue"));
| }
|
|}
|""".stripMargin)

"have resolved methodFullName if argument and parameter size matches" in {
val createClientCalls = cpg.call("createClient").l

createClientCalls.size shouldBe 1
createClientCalls.methodFullName.l shouldBe List(
"Sample.createClient:com.myorg.client.Client(com.myorg.config.RestConfigBase)"
)
createClientCalls.callee.fullName.l shouldBe List(
"Sample.createClient:com.myorg.client.Client(com.myorg.config.RestConfigBase)"
)
}
}
}