[kotlin2cpg] Partly revert #4187

[max-leuthaeuser]

This PR partly reverts #4187. The change to CompilerAPI.makeEnvironment breaks sptests. We may want to revisit that change later as it was intented to improve performance.

With publishing this PR here locally sptests runs fine again.

[pandurangpatil]

How do I reproduce the breaking issue you are referring to as sptest? Let me see how this can be fixed?

This consumes too much of memory and CPU for large repo. As it tries to build the type information cache for every path that is being passed as first parameter. The list we are passing is basically path to each and every child folder of the given project folder.

Which can be also achieved by passing only the root directory.

[max-leuthaeuser]

I don't know if you have access to codescience sptests but I guess you don't. On https://github.com/ShiftLeftSecurity/springboot-kotlin-webgoat the kotlin compiler completely errors out when only the root dir is given and that results in no type information at all.

[DavidBakerEffendi]

@max-leuthaeuser Do you have a deadline you could set for how long you're willing to wait for an alternative fix that achieves both goals? At least we have the webgoat for Kotlin to test against while @pandurangpatil looks into a fix

[max-leuthaeuser]

Its for a running POC where the customer waits for an update. I don't know how patient this customer is, sorry. Maybe @gacevedo can say more.

[max-leuthaeuser]

And from my point of view: the optimization in #4187 has never been released to the customers anyway. So we don't lose anything there. But we might lose type information now which may result in many lost findings as the broken sptests indicate. We should aim for high precision first and performance second.

[DavidBakerEffendi]

We should aim for high precision first and performance second.

This is a good point, and the most important imo

[max-leuthaeuser]

So I merged this to unblock the POC. A PR to get a fixed version of the performance optimization is very welcome nevertheless.

[pandurangpatil]

@DavidBakerEffendi @max-leuthaeuser

I understand the importance of precision. At the same time in my humble opinion, we should have the failing use cases incorporated as unit tests. Otherwise, it becomes difficult to collaborate. This becomes very crucial while making any performance improvements. As the only way to make sure nothing is breaking is by running the unit test cases.

cc: @tuxology @fabsx00

[max-leuthaeuser]

@pandurangpatil
This is very true. We did not have a test for that in first place and relying on our internal sptests is not sufficient here as you guys do not have access. Also, these tests are only triggered once kotlin2cpg is updated internally (https://github.com/ShiftLeftSecurity/kotlin2cpg) which did not happen for 3 month (thats why this slipped through for so long completely unnoticed).