Skip to content

Commit 362bfba

Browse files
Update unit tests
1 parent b9f6657 commit 362bfba

File tree

6 files changed

+79
-8
lines changed

6 files changed

+79
-8
lines changed

python/ql/src/Functions/IncorrectRaiseInSpecialMethod.ql

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* error-handling
88
* @problem.severity recommendation
99
* @sub-severity high
10-
* @precision very-high
10+
* @precision high
1111
* @id py/unexpected-raise-in-special-method
1212
*/
1313

@@ -16,7 +16,7 @@ import semmle.python.ApiGraphs
1616
import semmle.python.dataflow.new.internal.DataFlowDispatch
1717

1818
private predicate attributeMethod(string name) {
19-
name = ["__getattribute__", "__getattr__"] // __setattr__ excluded as it makes sense to raise different kinds of errors based on the `value` parameter
19+
name = ["__getattribute__", "__getattr__", "__delattr__"] // __setattr__ excluded as it makes sense to raise different kinds of errors based on the `value` parameter
2020
}
2121

2222
private predicate indexingMethod(string name) {
@@ -50,7 +50,7 @@ private predicate castMethod(string name) {
5050
[
5151
"__int__",
5252
"__float__",
53-
"__long__",
53+
"__index__",
5454
"__trunc__",
5555
"__complex__"
5656
]
@@ -61,6 +61,7 @@ predicate correctRaise(string name, Expr exec) {
6161
(
6262
indexingMethod(name) or
6363
attributeMethod(name) or
64+
// Allow add methods to raise a TypeError, as they can be used for sequence concatenation as well as arithmetic
6465
name = ["__add__", "__iadd__", "__radd__"]
6566
)
6667
or
@@ -125,7 +126,7 @@ predicate noNeedToAlwaysRaise(Function meth, string message, boolean allowNotImp
125126
not exists(Function overridden |
126127
overridden.getName() = meth.getName() and
127128
overridden.getScope() = getADirectSuperclass+(meth.getScope()) and
128-
alwaysRaises(overridden, _)
129+
not alwaysRaises(overridden, _)
129130
)
130131
}
131132

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| test.py:6:5:6:33 | Function __getitem__ | This method always raises $@ - should raise a LookupError (KeyError or IndexError) instead. | test.py:7:15:7:33 | ZeroDivisionError() | ZeroDivisionError |
2+
| test.py:9:5:9:32 | Function __getattr__ | This method always raises $@ - should raise an AttributeError instead. | test.py:10:15:10:33 | ZeroDivisionError() | ZeroDivisionError |
3+
| test.py:12:5:12:23 | Function __bool__ | This method always raises $@ - should raise a TypeError instead. | test.py:13:15:13:26 | ValueError() | ValueError |
4+
| test.py:15:5:15:22 | Function __int__ | This method always raises $@ - this method does not need to be implemented. | test.py:16:15:16:26 | ValueError() | ValueError |
5+
| test.py:24:5:24:23 | Function __hash__ | This method always raises $@ - use __hash__ = None instead. | test.py:25:15:25:35 | NotImplementedError() | NotImplementedError |
6+
| test.py:28:5:28:29 | Function __sub__ | This method raises $@ - should raise an ArithmeticError or return NotImplemented instead. | test.py:30:19:30:29 | TypeError() | TypeError |
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: Functions/IncorrectRaiseInSpecialMethod.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
class A:
2+
3+
def __add__(self, other): # No alert - Always allow NotImplementedError
4+
raise NotImplementedError()
5+
6+
def __getitem__(self, index): # $ Alert
7+
raise ZeroDivisionError()
8+
9+
def __getattr__(self, name): # $ Alert
10+
raise ZeroDivisionError()
11+
12+
def __bool__(self): # $ Alert
13+
raise ValueError()
14+
15+
def __int__(self): # $ Alert # Cast method need not be defined to always raise
16+
raise ValueError()
17+
18+
def __float__(self): # No alert - OK to raise conditionally
19+
if self.z:
20+
return 0
21+
else:
22+
raise ValueError()
23+
24+
def __hash__(self): # $ Alert # should use __hash__=None rather than stub implementation to make class unhashable
25+
raise NotImplementedError()
26+
27+
class B:
28+
def __sub__(self, other): # $ Alert # should return NotImplemented instead
29+
if not isinstance(other,B):
30+
raise TypeError()
31+
return self
32+
33+
def __add__(self, other): # No alert - allow add to raise a TypeError, as it is sometimes used for sequence concatenation as well as arithmetic
34+
if not isinstance(other,B):
35+
raise TypeError()
36+
return self
37+
38+
def __setitem__(self, key, val): # No alert - allow setitem to raise arbitrary exceptions as they could be due to the value, rather than a LookupError relating to the key
39+
if val < 0:
40+
raise ValueError()
41+
42+
def __getitem__(self, key): # No alert - indexing method allowed to raise TypeError or subclasses of LookupError.
43+
if not isinstance(key, int):
44+
raise TypeError()
45+
if key < 0:
46+
raise KeyError()
47+
return 3
48+
49+
def __getattribute__(self, name):
50+
if name != "a":
51+
raise AttributeError()
52+
return 2
53+
54+
def __div__(self, other):
55+
if other == 0:
56+
raise ZeroDivisionError()
57+
return self
58+
59+
60+
class D:
61+
def __int__(self):
62+
return 2
63+
64+
class E(D):
65+
def __int__(self): # No alert - cast method may override to raise exception
66+
raise TypeError()

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

Lines changed: 0 additions & 3 deletions
This file was deleted.

python/ql/test/query-tests/Functions/general/IncorrectRaiseInSpecialMethod.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)