Skip to content

Commit c9e9deb

Browse files
committed
Python: Adapt to a points-to-less world
Technically we still depend on points-to in that we still mention `PythonFunctionValue` and `ClassValue` in the query. However, we immediately move to working with the corresponding `Function` and `Class` AST nodes, and so we're not really using points-to. (The reason for doing things this way is that otherwise the `.toString()` for all of the alerts would change, which would make the diff hard to interpret. This way, it should be fairly simple to see which changes are actually relevant.) We do lose some precision when moving away from points-to, and this is reflected in the changes in the `.expected` file. In particular we no longer do complicated tracking of values, but rather look at the syntactic structure of the classes in question. This causes us to lose out on some results where a special method is defined elsewhere, and causes a single FP where a special method initially has the wrong signature, but is subsequently overwritten with a function with the correct signature. We also lose out on results having to do with default values, as these are now disabled. Finally, it was necessary to add special handling of methods marked with the `staticmethod` decorator, as these expect to receive fewer arguments. This was motivated by a MRVA run, where e.g. sympy showed a lot of examples along the lines of ``` @staticmethod def __abs__(): return ... ```
1 parent bf688b8 commit c9e9deb

File tree

3 files changed

+34
-18
lines changed

3 files changed

+34
-18
lines changed

python/ql/src/Functions/SignatureSpecialMethods.ql

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import python
14+
import semmle.python.dataflow.new.internal.DataFlowDispatch as DD
1415

1516
predicate is_unary_op(string name) {
1617
name in [
@@ -54,10 +55,20 @@ int argument_count(string name) {
5455
is_quad_op(name) and result = 4
5556
}
5657

58+
/**
59+
* Returns 1 if `func` is a static method, and 0 otherwise. This predicate is used to adjust the
60+
* number of expected arguments for a special method accordingly.
61+
*/
62+
int staticmethod_correction(Function func) {
63+
if DD::isStaticmethod(func) then result = 1 else result = 0
64+
}
65+
5766
predicate incorrect_special_method_defn(
5867
Function func, string message, boolean show_counts, string name, boolean is_unused_default
5968
) {
60-
exists(int required | required = argument_count(name) |
69+
exists(int required, int correction |
70+
required = argument_count(name) - correction and correction = staticmethod_correction(func)
71+
|
6172
/* actual_non_default <= actual */
6273
if required > func.getMaxPositionalArguments()
6374
then message = "Too few parameters" and show_counts = true and is_unused_default = false
@@ -78,23 +89,23 @@ predicate incorrect_special_method_defn(
7889
predicate incorrect_pow(
7990
Function func, string message, boolean show_counts, boolean is_unused_default
8091
) {
81-
(
82-
func.getMaxPositionalArguments() < 2 and
92+
exists(int correction | correction = staticmethod_correction(func) |
93+
func.getMaxPositionalArguments() < 2 - correction and
8394
message = "Too few parameters" and
8495
show_counts = true and
8596
is_unused_default = false
8697
or
87-
func.getMinPositionalArguments() > 3 and
98+
func.getMinPositionalArguments() > 3 - correction and
8899
message = "Too many parameters" and
89100
show_counts = true and
90101
is_unused_default = false
91102
or
92-
func.getMinPositionalArguments() < 2 and
103+
func.getMinPositionalArguments() < 2 - correction and
93104
message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and
94105
show_counts = false and
95106
is_unused_default = true
96107
or
97-
func.getMinPositionalArguments() = 3 and
108+
func.getMinPositionalArguments() = 3 - correction and
98109
message = "Third parameter to __pow__ should have a default value" and
99110
show_counts = false and
100111
is_unused_default = false
@@ -125,18 +136,18 @@ predicate incorrect_round(
125136
predicate incorrect_get(
126137
Function func, string message, boolean show_counts, boolean is_unused_default
127138
) {
128-
(
129-
func.getMaxPositionalArguments() < 3 and
139+
exists(int correction | correction = staticmethod_correction(func) |
140+
func.getMaxPositionalArguments() < 3 - correction and
130141
message = "Too few parameters" and
131142
show_counts = true and
132143
is_unused_default = false
133144
or
134-
func.getMinPositionalArguments() > 3 and
145+
func.getMinPositionalArguments() > 3 - correction and
135146
message = "Too many parameters" and
136147
show_counts = true and
137148
is_unused_default = false
138149
or
139-
func.getMinPositionalArguments() < 2 and
150+
func.getMinPositionalArguments() < 2 - correction and
140151
not func.hasVarArg() and
141152
message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and
142153
show_counts = false and
@@ -170,14 +181,18 @@ predicate isLikelyPlaceholderFunction(Function f) {
170181
or
171182
// Body just raises an exception.
172183
f.getBody().getLastItem() instanceof Raise
184+
or
185+
// Body is a pass statement.
186+
f.getBody().getLastItem() instanceof Pass
173187
)
174188
}
175189

176190
from
177191
PythonFunctionValue f, string message, string sizes, boolean show_counts, string name,
178192
ClassValue owner, boolean show_unused_defaults
179193
where
180-
owner.declaredAttribute(name) = f and
194+
owner.getScope().getAMethod() = f.getScope() and
195+
f.getScope().getName() = name and
181196
(
182197
incorrect_special_method_defn(f.getScope(), message, show_counts, name, show_unused_defaults)
183198
or

python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,4 @@
33
| om_test.py:65:5:65:29 | Function WrongSpecials.__neg__ | Too many parameters for special method __neg__, which has 2 parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
44
| om_test.py:68:5:68:35 | Function WrongSpecials.__exit__ | Too few parameters for special method __exit__, which has 3 parameters, but should have 4, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
55
| om_test.py:71:5:71:19 | Function WrongSpecials.__repr__ | Too few parameters for special method __repr__, which has no parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
6-
| om_test.py:74:5:74:46 | Function WrongSpecials.__add__ | 1 default values(s) will never be used for special method __add__, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
7-
| om_test.py:97:15:97:34 | Function NotOKSpecials.lambda | Too few parameters for special method __sub__, which has 1 parameter, but should have 2, in class $@. | om_test.py:95:1:95:28 | class NotOKSpecials | NotOKSpecials |
8-
| protocols.py:107:1:107:12 | Function f | Too few parameters for special method __add__, which has 1 parameter, but should have 2, in class $@. | protocols.py:110:1:110:29 | class MissingMethods | MissingMethods |
9-
| protocols.py:107:1:107:12 | Function f | Too few parameters for special method __set__, which has 1 parameter, but should have 3, in class $@. | protocols.py:110:1:110:29 | class MissingMethods | MissingMethods |
6+
| om_test.py:83:5:83:18 | Function OKSpecials.__del__ | Too few parameters for special method __del__, which has no parameters, but should have 1, in class $@. | om_test.py:81:1:81:25 | class OKSpecials | OKSpecials |

python/ql/test/query-tests/Functions/general/om_test.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ def __exit__(self, arg0, arg1):
6969
return arg0 == arg1
7070

7171
def __repr__():
72-
pass
72+
return ""
7373

7474
def __add__(self, other="Unused default"):
75-
pass
76-
75+
return 4
76+
7777
@staticmethod
7878
def __abs__():
7979
return 42
@@ -105,3 +105,7 @@ def pop(self):
105105

106106

107107

108+
class MoreSpecialMethods:
109+
@staticmethod
110+
def __abs__():
111+
return 42

0 commit comments

Comments
 (0)