Skip to content

Commit a59fd6e

Browse files
committed
Add rule raise-missing-from
1 parent 346bdcf commit a59fd6e

12 files changed

+188
-14
lines changed

CONTRIBUTORS.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,3 +391,5 @@ contributors:
391391
* Shiv Venkatasubrahmanyam
392392

393393
* Jochen Preusche (iilei): contributor
394+
395+
* Ram Rachum (cool-RR)

pylint/checkers/exceptions.py

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ def _is_raising(body: typing.List) -> bool:
7676
return False
7777

7878

79+
def iterate_parents(node: NodeNG) -> typing.Iterator[NodeNG]:
80+
current = node.parent
81+
while current is not None:
82+
yield current
83+
current = current.parent
84+
85+
7986
OVERGENERAL_EXCEPTIONS = ("BaseException", "Exception")
8087
BUILTINS_NAME = builtins.__name__
8188

@@ -152,6 +159,11 @@ def _is_raising(body: typing.List) -> bool:
152159
"immediately. Remove the raise operator or the entire "
153160
"try-except-raise block!",
154161
),
162+
"W0707": (
163+
"Exception needs to be raised with cause",
164+
"raise-missing-from",
165+
"Long explanation",
166+
),
155167
"W0711": (
156168
'Exception to catch is the result of a binary "%s" operation',
157169
"binary-op-exception",
@@ -279,14 +291,15 @@ def open(self):
279291
"notimplemented-raised",
280292
"bad-exception-context",
281293
"raising-format-tuple",
294+
"raise-missing-from",
282295
)
283296
def visit_raise(self, node):
284297
if node.exc is None:
285298
self._check_misplaced_bare_raise(node)
286299
return
287300

288-
if node.cause:
289-
self._check_bad_exception_context(node)
301+
self._check_bad_exception_cause(node)
302+
self._check_raise_missing_from(node)
290303

291304
expr = node.exc
292305
ExceptionRaiseRefVisitor(self, node).visit(expr)
@@ -320,15 +333,16 @@ def _check_misplaced_bare_raise(self, node):
320333
if not current or not isinstance(current.parent, expected):
321334
self.add_message("misplaced-bare-raise", node=node)
322335

323-
def _check_bad_exception_context(self, node):
324-
"""Verify that the exception context is properly set.
336+
def _check_bad_exception_cause(self, node: astroid.Raise) -> None:
337+
"""Verify that the exception cause is properly set.
325338
326-
An exception context can be only `None` or an exception.
339+
An exception cause can be only `None` or an exception.
327340
"""
341+
if not node.cause:
342+
return
328343
cause = utils.safe_infer(node.cause)
329344
if cause in (astroid.Uninferable, None):
330345
return
331-
332346
if isinstance(cause, astroid.Const):
333347
if cause.value is not None:
334348
self.add_message("bad-exception-context", node=node)
@@ -337,6 +351,46 @@ def _check_bad_exception_context(self, node):
337351
):
338352
self.add_message("bad-exception-context", node=node)
339353

354+
def _check_raise_missing_from(self, node: astroid.Raise) -> None:
355+
if node.cause is not None:
356+
return
357+
if node.exc is None:
358+
# This is a plain `raise`, raising the previously-caught exception. No need for
359+
# a cause.
360+
return
361+
# We'd like to check whether we're inside an `except` clause:
362+
for parent in iterate_parents(node):
363+
if isinstance(parent, astroid.ExceptHandler):
364+
except_parent = parent
365+
# We found a surrounding `except`! We're almost done proving there's a
366+
# `raise-missing-from` here. The only thing we need to protect against is that
367+
# maybe the `raise` is raising the exception that was caught, possibly with
368+
# some shenanigans like `exc.with_traceback(whatever)`. We won't analyze these,
369+
# we'll just assume there's a violation on two simple cases: `raise
370+
# SomeException(whatever)` and `raise SomeException`.
371+
if except_parent.name is None:
372+
# The `except` doesn't have an `as exception:` part, meaning there's no way
373+
# that the `raise` is raising the same exception.
374+
self.add_message("raise-missing-from", node=node)
375+
elif isinstance(node.exc, (astroid.Call)) and isinstance(
376+
node.exc.func, (astroid.Name)
377+
):
378+
# We have a `raise SomeException(whatever)`.
379+
self.add_message("raise-missing-from", node=node)
380+
elif (
381+
isinstance(node.exc, (astroid.Name))
382+
and node.exc.name != except_parent.name.name
383+
):
384+
# We have a `raise SomeException`.
385+
self.add_message("raise-missing-from", node=node)
386+
break
387+
if isinstance(parent, astroid.scoped_nodes.LocalsDictNodeNG):
388+
# If we're inside a function/class definition, we don't want to keep checking
389+
# higher parents for `except` clauses, because if these exist, it means our
390+
# function/class was defined in an `except` clause, rather than the current
391+
# code actually running in an `except` clause.
392+
break
393+
340394
def _check_catching_non_exception(self, handler, exc, part):
341395
if isinstance(exc, astroid.Tuple):
342396
# Check if it is a tuple of exceptions.

tests/functional/m/misplaced_bare_raise.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# pylint: disable=missing-docstring, broad-except, unreachable, try-except-raise
1+
# pylint: disable=missing-docstring, broad-except, unreachable, try-except-raise, raise-missing-from
22
# pylint: disable=unused-variable, too-few-public-methods, invalid-name, useless-object-inheritance
33

44
try:

tests/functional/n/no_else_raise.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
""" Test that superfluous else return are detected. """
22

3-
# pylint:disable=invalid-name,missing-docstring,unused-variable
3+
# pylint:disable=invalid-name,missing-docstring,unused-variable,raise-missing-from
44

55
def foo1(x, y, z):
66
if x: # [no-else-raise]

tests/functional/n/no_else_return.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
""" Test that superfluous else return are detected. """
22

3-
# pylint:disable=invalid-name,missing-docstring,unused-variable
43

4+
# pylint:disable=invalid-name,missing-docstring,unused-variable
55
def foo1(x, y, z):
66
if x: # [no-else-return]
77
a = 1
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# pylint:disable=missing-docstring, unreachable, using-constant-test, invalid-name, bare-except
2+
# pylint:disable=try-except-raise, undefined-variable, too-few-public-methods, superfluous-parens
3+
4+
5+
# Positives:
6+
7+
try:
8+
1 / 0
9+
except ZeroDivisionError:
10+
# +1: [raise-missing-from]
11+
raise KeyError
12+
13+
try:
14+
1 / 0
15+
except ZeroDivisionError:
16+
# Our algorithm doesn't have to be careful about the complicated expression below, because
17+
# the exception above wasn't bound to a name.
18+
# +1: [raise-missing-from]
19+
raise (foo + bar).baz
20+
21+
try:
22+
1 / 0
23+
except ZeroDivisionError as e:
24+
# +1: [raise-missing-from]
25+
raise KeyError
26+
27+
try:
28+
1 / 0
29+
except ZeroDivisionError as e:
30+
# +1: [raise-missing-from]
31+
raise KeyError
32+
else:
33+
pass
34+
finally:
35+
pass
36+
37+
try:
38+
1 / 0
39+
except ZeroDivisionError as e:
40+
if 1:
41+
if 1:
42+
with whatever:
43+
try:
44+
# +1: [raise-missing-from]
45+
raise KeyError
46+
except:
47+
pass
48+
49+
try:
50+
1 / 0
51+
except ZeroDivisionError as e:
52+
# +1: [raise-missing-from]
53+
raise KeyError()
54+
55+
try:
56+
1 / 0
57+
except ZeroDivisionError as e:
58+
# +1: [raise-missing-from]
59+
raise KeyError(whatever, whatever=whatever)
60+
61+
62+
63+
# Negatives:
64+
65+
try:
66+
1 / 0
67+
except ZeroDivisionError:
68+
raise
69+
70+
try:
71+
1 / 0
72+
except ZeroDivisionError as e:
73+
raise
74+
75+
try:
76+
1 / 0
77+
except ZeroDivisionError as e:
78+
raise e
79+
80+
try:
81+
1 / 0
82+
except ZeroDivisionError as e:
83+
if 1:
84+
if 1:
85+
if 1:
86+
raise e
87+
88+
try:
89+
1 / 0
90+
except ZeroDivisionError as e:
91+
raise e.with_traceback(e.__traceback__)
92+
93+
try:
94+
1 / 0
95+
except ZeroDivisionError as e:
96+
raise (e + 7)
97+
98+
try:
99+
1 / 0
100+
except ZeroDivisionError as e:
101+
def f():
102+
raise KeyError
103+
104+
try:
105+
1 / 0
106+
except ZeroDivisionError as e:
107+
class Foo:
108+
raise KeyError
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
raise-missing-from:11::Exception needs to be raised with cause
2+
raise-missing-from:19::Exception needs to be raised with cause
3+
raise-missing-from:25::Exception needs to be raised with cause
4+
raise-missing-from:31::Exception needs to be raised with cause
5+
raise-missing-from:45::Exception needs to be raised with cause
6+
raise-missing-from:53::Exception needs to be raised with cause
7+
raise-missing-from:59::Exception needs to be raised with cause

tests/functional/r/regression_3416_unused_argument_raise.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
https://github.com/PyCQA/pylint/issues/3416
44
"""
55

6+
# pylint:disable=raise-missing-from
7+
68
# +1: [unused-argument, unused-argument, unused-argument]
79
def fun(arg_a, arg_b, arg_c) -> None:
810
"""Routine docstring"""
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
unused-argument:7:fun:Unused argument 'arg_a'
2-
unused-argument:7:fun:Unused argument 'arg_b'
3-
unused-argument:7:fun:Unused argument 'arg_c'
1+
unused-argument:9:fun:Unused argument 'arg_a'
2+
unused-argument:9:fun:Unused argument 'arg_b'
3+
unused-argument:9:fun:Unused argument 'arg_c'

tests/functional/s/stop_iteration_inside_generator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""
22
Test that no StopIteration is raised inside a generator
33
"""
4-
# pylint: disable=missing-docstring,invalid-name,import-error, try-except-raise, wrong-import-position,not-callable
4+
# pylint: disable=missing-docstring,invalid-name,import-error, try-except-raise, wrong-import-position,not-callable,raise-missing-from
55
import asyncio
66

77
class RebornStopIteration(StopIteration):

0 commit comments

Comments
 (0)