Skip to content

Commit 2dcd789

Browse files
committed
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.
1 parent e2ed848 commit 2dcd789

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

python/ql/src/Functions/ReturnConsistentTupleSizes.ql

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,32 @@
44
* @kind problem
55
* @tags reliability
66
* maintainability
7+
* quality
78
* @problem.severity recommendation
89
* @sub-severity high
910
* @precision high
1011
* @id py/mixed-tuple-returns
1112
*/
1213

1314
import python
15+
import semmle.python.ApiGraphs
1416

15-
predicate returns_tuple_of_size(Function func, int size, AstNode origin) {
16-
exists(Return return, TupleValue val |
17+
predicate returns_tuple_of_size(Function func, int size, Tuple tuple) {
18+
exists(Return return, DataFlow::Node value |
19+
value.asExpr() = return.getValue() and
1720
return.getScope() = func and
18-
return.getValue().pointsTo(val, origin)
21+
any(DataFlow::LocalSourceNode n | n.asExpr() = tuple).flowsTo(value)
1922
|
20-
size = val.length()
23+
size = count(int n | exists(tuple.getElt(n)))
2124
)
2225
}
2326

2427
from Function func, int s1, int s2, AstNode t1, AstNode t2
2528
where
2629
returns_tuple_of_size(func, s1, t1) and
2730
returns_tuple_of_size(func, s2, t2) and
28-
s1 < s2
31+
s1 < s2 and
32+
// Don't report on functions that have a return type annotation
33+
not exists(func.getDefinition().(FunctionExpr).getReturns())
2934
select func, func.getQualifiedName() + " returns $@ and $@.", t1, "tuple of size " + s1, t2,
3035
"tuple of size " + s2

0 commit comments

Comments
 (0)