Skip to content

Commit 840abbf

Browse files
tausbntausbn
authored
Merge pull request #18956 from github/tausbn/python-more-special-method-query-refactoring
Python: Modernize special method query
2 parents 342d4a6 + 074af6f commit 840abbf

File tree

4 files changed

+152
-74
lines changed

4 files changed

+152
-74
lines changed

‎python/ql/src/Functions/SignatureSpecialMethods.ql

+134-62
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,23 @@
44
* @kind problem
55
* @tags reliability
66
* correctness
7+
* quality
78
* @problem.severity error
89
* @sub-severity low
910
* @precision high
1011
* @id py/special-method-wrong-signature
1112
*/
1213

1314
import python
15+
import semmle.python.dataflow.new.internal.DataFlowDispatch as DD
1416

1517
predicate is_unary_op(string name) {
1618
name in [
1719
"__del__", "__repr__", "__neg__", "__pos__", "__abs__", "__invert__", "__complex__",
1820
"__int__", "__float__", "__long__", "__oct__", "__hex__", "__str__", "__index__", "__enter__",
19-
"__hash__", "__bool__", "__nonzero__", "__unicode__", "__len__", "__iter__", "__reversed__"
21+
"__hash__", "__bool__", "__nonzero__", "__unicode__", "__len__", "__iter__", "__reversed__",
22+
"__aenter__", "__aiter__", "__anext__", "__await__", "__ceil__", "__floor__", "__trunc__",
23+
"__length_hint__", "__dir__", "__bytes__"
2024
]
2125
}
2226

@@ -28,91 +32,138 @@ predicate is_binary_op(string name) {
2832
"__and__", "__xor__", "__or__", "__ne__", "__radd__", "__rsub__", "__rmul__", "__rfloordiv__",
2933
"__rdiv__", "__rtruediv__", "__rmod__", "__rdivmod__", "__rpow__", "__rlshift__", "__gt__",
3034
"__rrshift__", "__rand__", "__rxor__", "__ror__", "__iadd__", "__isub__", "__imul__",
31-
"__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__idivmod__", "__ipow__",
32-
"__ilshift__", "__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__",
33-
"__rcmp__", "__getattr___", "__getattribute___"
35+
"__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__ipow__", "__ilshift__",
36+
"__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__", "__rcmp__",
37+
"__getattr__", "__getattribute__", "__buffer__", "__release_buffer__", "__matmul__",
38+
"__rmatmul__", "__imatmul__", "__missing__", "__class_getitem__", "__mro_entries__",
39+
"__format__"
3440
]
3541
}
3642

3743
predicate is_ternary_op(string name) {
38-
name in ["__setattr__", "__set__", "__setitem__", "__getslice__", "__delslice__"]
44+
name in ["__setattr__", "__set__", "__setitem__", "__getslice__", "__delslice__", "__set_name__"]
3945
}
4046

41-
predicate is_quad_op(string name) { name = "__setslice__" or name = "__exit__" }
47+
predicate is_quad_op(string name) { name in ["__setslice__", "__exit__", "__aexit__"] }
4248

43-
int argument_count(PythonFunctionValue f, string name, ClassValue cls) {
44-
cls.declaredAttribute(name) = f and
45-
(
46-
is_unary_op(name) and result = 1
47-
or
48-
is_binary_op(name) and result = 2
49-
or
50-
is_ternary_op(name) and result = 3
51-
or
52-
is_quad_op(name) and result = 4
53-
)
49+
int argument_count(string name) {
50+
is_unary_op(name) and result = 1
51+
or
52+
is_binary_op(name) and result = 2
53+
or
54+
is_ternary_op(name) and result = 3
55+
or
56+
is_quad_op(name) and result = 4
57+
}
58+
59+
/**
60+
* Returns 1 if `func` is a static method, and 0 otherwise. This predicate is used to adjust the
61+
* number of expected arguments for a special method accordingly.
62+
*/
63+
int staticmethod_correction(Function func) {
64+
if DD::isStaticmethod(func) then result = 1 else result = 0
5465
}
5566

5667
predicate incorrect_special_method_defn(
57-
PythonFunctionValue func, string message, boolean show_counts, string name, ClassValue owner
68+
Function func, string message, boolean show_counts, string name, boolean is_unused_default
5869
) {
59-
exists(int required | required = argument_count(func, name, owner) |
70+
exists(int required, int correction |
71+
required = argument_count(name) - correction and correction = staticmethod_correction(func)
72+
|
6073
/* actual_non_default <= actual */
61-
if required > func.maxParameters()
62-
then message = "Too few parameters" and show_counts = true
74+
if required > func.getMaxPositionalArguments()
75+
then message = "Too few parameters" and show_counts = true and is_unused_default = false
6376
else
64-
if required < func.minParameters()
65-
then message = "Too many parameters" and show_counts = true
77+
if required < func.getMinPositionalArguments()
78+
then message = "Too many parameters" and show_counts = true and is_unused_default = false
6679
else (
67-
func.minParameters() < required and
68-
not func.getScope().hasVarArg() and
69-
message = (required - func.minParameters()) + " default values(s) will never be used" and
70-
show_counts = false
80+
func.getMinPositionalArguments() < required and
81+
not func.hasVarArg() and
82+
message =
83+
(required - func.getMinPositionalArguments()) + " default values(s) will never be used" and
84+
show_counts = false and
85+
is_unused_default = true
7186
)
7287
)
7388
}
7489

75-
predicate incorrect_pow(FunctionValue func, string message, boolean show_counts, ClassValue owner) {
76-
owner.declaredAttribute("__pow__") = func and
77-
(
78-
func.maxParameters() < 2 and message = "Too few parameters" and show_counts = true
90+
predicate incorrect_pow(
91+
Function func, string message, boolean show_counts, boolean is_unused_default
92+
) {
93+
exists(int correction | correction = staticmethod_correction(func) |
94+
func.getMaxPositionalArguments() < 2 - correction and
95+
message = "Too few parameters" and
96+
show_counts = true and
97+
is_unused_default = false
7998
or
80-
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true
99+
func.getMinPositionalArguments() > 3 - correction and
100+
message = "Too many parameters" and
101+
show_counts = true and
102+
is_unused_default = false
81103
or
82-
func.minParameters() < 2 and
83-
message = (2 - func.minParameters()) + " default value(s) will never be used" and
84-
show_counts = false
104+
func.getMinPositionalArguments() < 2 - correction and
105+
message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and
106+
show_counts = false and
107+
is_unused_default = true
85108
or
86-
func.minParameters() = 3 and
109+
func.getMinPositionalArguments() = 3 - correction and
87110
message = "Third parameter to __pow__ should have a default value" and
88-
show_counts = false
111+
show_counts = false and
112+
is_unused_default = false
89113
)
90114
}
91115

92-
predicate incorrect_get(FunctionValue func, string message, boolean show_counts, ClassValue owner) {
93-
owner.declaredAttribute("__get__") = func and
94-
(
95-
func.maxParameters() < 3 and message = "Too few parameters" and show_counts = true
116+
predicate incorrect_round(
117+
Function func, string message, boolean show_counts, boolean is_unused_default
118+
) {
119+
exists(int correction | correction = staticmethod_correction(func) |
120+
func.getMaxPositionalArguments() < 1 - correction and
121+
message = "Too few parameters" and
122+
show_counts = true and
123+
is_unused_default = false
96124
or
97-
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true
125+
func.getMinPositionalArguments() > 2 - correction and
126+
message = "Too many parameters" and
127+
show_counts = true and
128+
is_unused_default = false
98129
or
99-
func.minParameters() < 2 and
100-
not func.getScope().hasVarArg() and
101-
message = (2 - func.minParameters()) + " default value(s) will never be used" and
102-
show_counts = false
130+
func.getMinPositionalArguments() = 2 - correction and
131+
message = "Second parameter to __round__ should have a default value" and
132+
show_counts = false and
133+
is_unused_default = false
103134
)
104135
}
105136

106-
string should_have_parameters(PythonFunctionValue f, string name, ClassValue owner) {
107-
exists(int i | i = argument_count(f, name, owner) | result = i.toString())
108-
or
109-
owner.declaredAttribute(name) = f and
110-
(name = "__get__" or name = "__pow__") and
111-
result = "2 or 3"
137+
predicate incorrect_get(
138+
Function func, string message, boolean show_counts, boolean is_unused_default
139+
) {
140+
exists(int correction | correction = staticmethod_correction(func) |
141+
func.getMaxPositionalArguments() < 3 - correction and
142+
message = "Too few parameters" and
143+
show_counts = true and
144+
is_unused_default = false
145+
or
146+
func.getMinPositionalArguments() > 3 - correction and
147+
message = "Too many parameters" and
148+
show_counts = true and
149+
is_unused_default = false
150+
or
151+
func.getMinPositionalArguments() < 2 - correction and
152+
not func.hasVarArg() and
153+
message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and
154+
show_counts = false and
155+
is_unused_default = true
156+
)
112157
}
113158

114-
string has_parameters(PythonFunctionValue f) {
115-
exists(int i | i = f.minParameters() |
159+
string should_have_parameters(string name) {
160+
if name in ["__pow__", "__get__"]
161+
then result = "2 or 3"
162+
else result = argument_count(name).toString()
163+
}
164+
165+
string has_parameters(Function f) {
166+
exists(int i | i = f.getMinPositionalArguments() |
116167
i = 0 and result = "no parameters"
117168
or
118169
i = 1 and result = "1 parameter"
@@ -121,23 +172,44 @@ string has_parameters(PythonFunctionValue f) {
121172
)
122173
}
123174

175+
/** Holds if `f` is likely to be a placeholder, and hence not interesting enough to report. */
176+
predicate isLikelyPlaceholderFunction(Function f) {
177+
// Body has only a single statement.
178+
f.getBody().getItem(0) = f.getBody().getLastItem() and
179+
(
180+
// Body is a string literal. This is a common pattern for Zope interfaces.
181+
f.getBody().getLastItem().(ExprStmt).getValue() instanceof StringLiteral
182+
or
183+
// Body just raises an exception.
184+
f.getBody().getLastItem() instanceof Raise
185+
or
186+
// Body is a pass statement.
187+
f.getBody().getLastItem() instanceof Pass
188+
)
189+
}
190+
124191
from
125-
PythonFunctionValue f, string message, string sizes, boolean show_counts, string name,
126-
ClassValue owner
192+
Function f, string message, string sizes, boolean show_counts, string name, Class owner,
193+
boolean show_unused_defaults
127194
where
195+
owner.getAMethod() = f and
196+
f.getName() = name and
128197
(
129-
incorrect_special_method_defn(f, message, show_counts, name, owner)
198+
incorrect_special_method_defn(f, message, show_counts, name, show_unused_defaults)
199+
or
200+
incorrect_pow(f, message, show_counts, show_unused_defaults) and name = "__pow__"
130201
or
131-
incorrect_pow(f, message, show_counts, owner) and name = "__pow__"
202+
incorrect_get(f, message, show_counts, show_unused_defaults) and name = "__get__"
132203
or
133-
incorrect_get(f, message, show_counts, owner) and name = "__get__"
204+
incorrect_round(f, message, show_counts, show_unused_defaults) and
205+
name = "__round__"
134206
) and
207+
not isLikelyPlaceholderFunction(f) and
208+
show_unused_defaults = false and
135209
(
136210
show_counts = false and sizes = ""
137211
or
138212
show_counts = true and
139-
sizes =
140-
", which has " + has_parameters(f) + ", but should have " +
141-
should_have_parameters(f, name, owner)
213+
sizes = ", which has " + has_parameters(f) + ", but should have " + should_have_parameters(name)
142214
)
143215
select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName()

‎python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
- The `py/special-method-wrong-signature` has been modernized and rewritten to no longer rely on outdated APIs. Moreover, the query no longer flags cases where a default value is never used, as these alerts were rarely useful.

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

+6-9
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
1-
| om_test.py:59:5:59:28 | Function WrongSpecials.__div__ | Too many parameters for special method __div__, which has 3 parameters, but should have 2, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
2-
| om_test.py:62:5:62:22 | Function WrongSpecials.__mul__ | Too few parameters for special method __mul__, which has 1 parameter, but should have 2, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials |
3-
| 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 |
4-
| 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 |
5-
| 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 |
1+
| om_test.py:59:5:59:28 | Function __div__ | Too many parameters for special method __div__, which has 3 parameters, but should have 2, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials |
2+
| om_test.py:62:5:62:22 | Function __mul__ | Too few parameters for special method __mul__, which has 1 parameter, but should have 2, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials |
3+
| om_test.py:65:5:65:29 | Function __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 |
4+
| om_test.py:68:5:68:35 | Function __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 |
5+
| om_test.py:71:5:71:19 | Function __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:83:5:83:18 | Function __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

+7-3
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)