Open compaluca opened 1 year ago
Some of my already review patterns have an error
.
object of type 'int' has no len()
. I can't make anything of this yet. @mal-tee : the logger can be extended with some more info and I will try to do so. However that specific error: object of type 'int' has no len()
seems to be caused by the parsing of the discovery rule results. Rather than a list of findings we only get 1
as result (see last element of the raw output in the log). Does that indicate a failure in executing the discovery rule?
2023-02-01 16:57:41 - tpframework.core.discovery - INFO - run_discovery_rule - Discovery - rule raw output: (/tp-framework/out/20230201_check_discovery_rules-PHP/check_discovery_rules_2023-02-01-16-57-11_PHP_1_instance_37_callstatic_overloading/cpg_2023-02-01-16-57-11_PHP_1_instance_37_callstatic_overloading.bin,37_callstatic_overloading_iall,1)
You can try to run yourself on the branch refactoring (here done for pattern 37
but you can change it):
tpframework checkdiscoveryrules --export test_export.csv -l PHP -p 37 --output-dir /tp-framework/out/20230201_check_discovery_rules-PHP
You will get a sub-folder within the results folder that will contain the generated cpg in case you want to run it manually.
Seems like some discovery rules end with .size
and therefore return a int. I'll change them so that they end with .toJson
and can be properly parsed by the framework.
Hmm, some nodes in the php cpg don't have a lineNumber
. How can we deal with this?
What I was usually doing outside of this framework is going up the ast-parent chain, until I encounter a lineNumber. I did this inside scala code and not the joern shell.
@pr0me what's a smart way to fix this here? My current solution is to fix this via
.repeat(_.astParent)(_.until(_.filter(x => x.lineNumber.getOrElse(-1) != -1))).location.toJson
// sometimes the generator might assign Some(-1) for unknown numbers. That's why I use the .getOrElse(-1) != -1 construct, to cover None and Some(-1).
instead of
.location.toJson
Is there a smarter way? This would make the discovery rules quite bulky.
FYI, When working on issue #10, I am facing the same problem for pattern 76 in JS. The query is as below and joern is not returning the line number.
cpg.assignment.where(n=> n.code("^(?!(function|_tmp)).*\\..*=.*$")).method.callIn.location.toJson
If anyone has some hints on why that happens, it is really appreciated!
@mal-tee I tried your solution of traversing the AST upwards (as above), and it seem to not work in my case, returning empty results like below. Have you also encountered this issue?
{'symbol': '<empty>', 'packageName': '<empty>', 'nodeLabel': '', 'methodShortName': '<empty>', 'methodFullName': '<empty>', 'filename': '<empty>', 'classShortName': '<empty>', 'className': '<empty>'}
Interesting!
@mal-tee I tried your solution of traversing the AST upwards (as above), and it seem to not work in my case, returning empty results like below. Have you also encountered this issue?
{'symbol': '<empty>', 'packageName': '<empty>', 'nodeLabel': '', 'methodShortName': '<empty>', 'methodFullName': '<empty>', 'filename': '<empty>', 'classShortName': '<empty>', 'className': '<empty>'}
No, haven't seen that so far.
Hmm, some nodes in the php cpg don't have a
lineNumber
. How can we deal with this? What I was usually doing outside of this framework is going up the ast-parent chain, until I encounter a lineNumber. I did this inside scala code and not the joern shell.@pr0me what's a smart way to fix this here? My current solution is to fix this via
.repeat(_.astParent)(_.until(_.filter(x => x.lineNumber.getOrElse(-1) != -1))).location.toJson // sometimes the generator might assign Some(-1) for unknown numbers. That's why I use the .getOrElse(-1) != -1 construct, to cover None and Some(-1).
instead of
.location.toJson
Is there a smarter way? This would make the discovery rules quite bulky.
@compaluca As discussed in our last meeting, I manually tried this workaround on a few instances (58, 19_1, 56,3_1,and 3_2). The fix didn't change the outcome of the discovery rules for the instances that were ok beforehand (58, 19_1). But it also fixed 56, 3_1, and 3_2 which had missing linenos before.
Speaking from this data integrating this into the framework should be ok until we fix linenumbers in the php-cpg.
After implementing the work-around in the framework, we will repeat the test
Problem statement
Discovery rules (when avaliable) were tested for PHP patterns via the tpframework new functionality:
This means that:
Here the raw data in a zip comprising:
Proposed changes
Check the issues reported and fix the patterns that can be fixed