mirror of
https://github.com/github/codeql.git
synced 2026-04-25 16:55:19 +02:00
Python: Modernise py/mixed-tuple-returns
Removes the dependence on points-to in favour of an approach based on (local) data-flow. I first tried a version that used type tracking, as this more accurately mimics the behaviour of the old query. However, I soon discovered that there were _many_ false positives in this setup. The main bad pattern I saw was a helper function somewhere deep inside the code that both receives and returns an argument that can be tuples with different sizes and origins. In this case, global flow produces something akin to a cartesian product of "n-tuples that flow into the function" and "m-tuples that flow into the function" where m < n. To combat this, I decided to instead focus on only flow _within_ a given function (and so local data-flow was sufficient). Additionally, another class of false positives I saw was cases where the return type actually witnessed that the function in question could return tuples of varying sizes. In this case it seems reasonable to not flag these instances, since they are already (presumably) being checked by a type checker. More generally, if you've annotated the return type of the function with anything (not just `Tuple[...]`), then there's probably little need to flag it.
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* maintainability
|
||||
* quality
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision high
|
||||
@@ -11,13 +12,15 @@
|
||||
*/
|
||||
|
||||
import python
|
||||
import semmle.python.ApiGraphs
|
||||
|
||||
predicate returns_tuple_of_size(Function func, int size, AstNode origin) {
|
||||
exists(Return return, TupleValue val |
|
||||
predicate returns_tuple_of_size(Function func, int size, Tuple tuple) {
|
||||
exists(Return return, DataFlow::Node value |
|
||||
value.asExpr() = return.getValue() and
|
||||
return.getScope() = func and
|
||||
return.getValue().pointsTo(val, origin)
|
||||
any(DataFlow::LocalSourceNode n | n.asExpr() = tuple).flowsTo(value)
|
||||
|
|
||||
size = val.length()
|
||||
size = count(int n | exists(tuple.getElt(n)))
|
||||
)
|
||||
}
|
||||
|
||||
@@ -25,6 +28,8 @@ from Function func, int s1, int s2, AstNode t1, AstNode t2
|
||||
where
|
||||
returns_tuple_of_size(func, s1, t1) and
|
||||
returns_tuple_of_size(func, s2, t2) and
|
||||
s1 < s2
|
||||
s1 < s2 and
|
||||
// Don't report on functions that have a return type annotation
|
||||
not exists(func.getDefinition().(FunctionExpr).getReturns())
|
||||
select func, func.getQualifiedName() + " returns $@ and $@.", t1, "tuple of size " + s1, t2,
|
||||
"tuple of size " + s2
|
||||
|
||||
Reference in New Issue
Block a user