search

github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.51k stars 1.49k forks source link

LGTM.com - false positive #3113

Open david-zwicker opened 4 years ago

david-zwicker commented 4 years ago

LGTM complains about Call to a non-callable of class CLS.

In my case, CLS refers to the class and the call should thus create a new instance. I guess the problem in my case is that I needed to add an additional decorator besides the @classmethod to mark the method as a classmethod; see code:

https://lgtm.com/projects/g/zwicker-group/py-pde/snapshot/b591dace7b765362f97b4435a99a1ec95787a504/files/pde/visualization/plotting.py?sort=name&dir=ASC&mode=heatmap#x3c4edeb8f4dc0a46:1

RasmusWL commented 4 years ago

Thanks for the report! :+1: The problem in your specific case is that from_storage is also decorated with fill_in_docstring, and our analysis is quite conservative when seeing decorators it doesn't know about (since they can do anything).

I've added a boiled-down version of your problem to our tests in https://github.com/Semmle/ql/pull/3114, and created an internal tracking ticket for this. But I can't make any promises that fixing this will be a priority

david-zwicker commented 4 years ago

Yes, I agree that the extra decorator complicates the automatic analysis, but I felt it is a reasonable use-case of decorates. Is there any way to make the algorithm aware of the decorator? After all, this is a trivial one, which only changes the doctoring.

RasmusWL commented 4 years ago

I agree that we as humans can easily see that this decorator is not causing any problems :wink: and personally I think it's a nice use of decorators.

I don't know of any way that you can make our analysis handle this case right now.