Skip to content

[raise-missing-from] Clearer message and example in the documentation #6576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/data/messages/r/raise-missing-from/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
try:
1 / 0
except ZeroDivisionError as e:
raise ValueError("Rectangle Area cannot be zero") # [raise-missing-from]
4 changes: 4 additions & 0 deletions doc/data/messages/r/raise-missing-from/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
try:
1 / 0
except ZeroDivisionError as e:
raise ValueError("Rectangle Area cannot be zero") from e
1 change: 1 addition & 0 deletions doc/data/messages/r/raise-missing-from/related.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `PEP 3132 <https://peps.python.org/pep-3132/>`_
44 changes: 31 additions & 13 deletions pylint/checkers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from pylint import checkers
from pylint.checkers import utils
from pylint.interfaces import HIGH
from pylint.typing import MessageDefinitionTuple

if TYPE_CHECKING:
Expand Down Expand Up @@ -131,13 +132,13 @@ def _is_raising(body: list) -> bool:
"try-except-raise block!",
),
"W0707": (
"Consider explicitly re-raising using the 'from' keyword",
"Consider explicitly re-raising using %s'%s from %s'",
"raise-missing-from",
"Python 3's exception chaining means it shows the traceback of the "
"current exception, but also the original exception. Not using `raise "
"from` makes the traceback inaccurate, because the message implies "
"there is a bug in the exception-handling code itself, which is a "
"separate situation than wrapping an exception.",
"Python's exception chaining shows the traceback of the current exception, "
"but also of the original exception. When you raise a new exception after "
"another exception was caught it's likely that the second exception is a "
"friendly re-wrapping of the first exception. In such cases `raise from` "
"provides a better link between the two tracebacks in the final error.",
),
"W0711": (
'Exception to catch is the result of a binary "%s" operation',
Expand Down Expand Up @@ -334,16 +335,33 @@ def _check_raise_missing_from(self, node: nodes.Raise) -> None:
if containing_except_node.name is None:
# The `except` doesn't have an `as exception:` part, meaning there's no way that
# the `raise` is raising the same exception.
self.add_message("raise-missing-from", node=node)
elif isinstance(node.exc, nodes.Call) and isinstance(node.exc.func, nodes.Name):
# We have a `raise SomeException(whatever)`.
self.add_message("raise-missing-from", node=node)
class_of_old_error = "Exception"
if isinstance(containing_except_node.type, (nodes.Name, nodes.Tuple)):
# 'except ZeroDivisionError' or 'except (ZeroDivisionError, ValueError)'
class_of_old_error = containing_except_node.type.as_string()
self.add_message(
"raise-missing-from",
node=node,
args=(
f"'except {class_of_old_error} as exc' and ",
node.as_string(),
"exc",
),
confidence=HIGH,
)
elif (
isinstance(node.exc, nodes.Name)
isinstance(node.exc, nodes.Call)
and isinstance(node.exc.func, nodes.Name)
or isinstance(node.exc, nodes.Name)
and node.exc.name != containing_except_node.name.name
):
# We have a `raise SomeException`.
self.add_message("raise-missing-from", node=node)
# We have a `raise SomeException(whatever)` or a `raise SomeException`
self.add_message(
"raise-missing-from",
node=node,
args=("", node.as_string(), containing_except_node.name.name),
confidence=HIGH,
)

def _check_catching_non_exception(self, handler, exc, part):
if isinstance(exc, nodes.Tuple):
Expand Down
37 changes: 26 additions & 11 deletions tests/functional/r/raise_missing_from.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# pylint:disable=missing-docstring, unreachable, using-constant-test, invalid-name, bare-except
# pylint:disable=try-except-raise, undefined-variable, too-few-public-methods, superfluous-parens

################################################################################
# Positives:
try:
1 / 0
except:
raise ValueError('Invalid integer') # [raise-missing-from]

try:
1 / 0
Expand All @@ -13,9 +15,8 @@
try:
1 / 0
except ZeroDivisionError:
# Our algorithm doesn't have to be careful about the complicated expression below, because
# the exception above wasn't bound to a name.
# +1: [raise-missing-from]
# Our algorithm doesn't have to be careful about the complicated expression below,
# because the exception above wasn't bound to a name. # +1: [raise-missing-from]
raise (foo + bar).baz

try:
Expand Down Expand Up @@ -59,8 +60,26 @@
raise KeyError(whatever, whatever=whatever)


################################################################################
# Negatives (Same cases as above, except with `from`):
try:
1 / 0
except (ZeroDivisionError, ValueError, KeyError):
if 1:
if 2:
pass
else:
with whatever:
# +1: [raise-missing-from]
raise KeyError(whatever, whatever=whatever)
else:
# +1: [raise-missing-from]
raise KeyError(whatever, overever=12)

try:
# Taken from https://github.com/python/cpython/blob/3.10/Lib/plistlib.py#L459
pass
except (OSError, IndexError, struct.error, OverflowError,
ValueError):
raise InvalidFileException() # [raise-missing-from]

try:
1 / 0
Expand Down Expand Up @@ -107,10 +126,6 @@
except ZeroDivisionError as e:
raise KeyError(whatever, whatever=whatever) from foo


################################################################################
# Other negatives:

try:
1 / 0
except ZeroDivisionError:
Expand Down
18 changes: 11 additions & 7 deletions tests/functional/r/raise_missing_from.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
raise-missing-from:11:4:11:18::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:19:4:19:25::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:25:4:25:18::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:31:4:31:18::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:45:20:45:34::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:53:4:53:20::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:59:4:59:47::Consider explicitly re-raising using the 'from' keyword:UNDEFINED
raise-missing-from:7:4:7:39::Consider explicitly re-raising using 'except Exception as exc' and 'raise ValueError('Invalid integer') from exc':HIGH
raise-missing-from:13:4:13:18::Consider explicitly re-raising using 'except ZeroDivisionError as exc' and 'raise KeyError from exc':HIGH
raise-missing-from:20:4:20:25::Consider explicitly re-raising using 'except ZeroDivisionError as exc' and 'raise (foo + bar).baz from exc':HIGH
raise-missing-from:26:4:26:18::Consider explicitly re-raising using 'raise KeyError from e':HIGH
raise-missing-from:32:4:32:18::Consider explicitly re-raising using 'raise KeyError from e':HIGH
raise-missing-from:46:20:46:34::Consider explicitly re-raising using 'raise KeyError from e':HIGH
raise-missing-from:54:4:54:20::Consider explicitly re-raising using 'raise KeyError() from e':HIGH
raise-missing-from:60:4:60:47::Consider explicitly re-raising using 'raise KeyError(whatever, whatever=whatever) from e':HIGH
raise-missing-from:72:16:72:59::Consider explicitly re-raising using 'except (ZeroDivisionError, ValueError, KeyError) as exc' and 'raise KeyError(whatever, whatever=whatever) from exc':HIGH
raise-missing-from:75:8:75:45::Consider explicitly re-raising using 'except (ZeroDivisionError, ValueError, KeyError) as exc' and 'raise KeyError(whatever, overever=12) from exc':HIGH
raise-missing-from:82:4:82:32::Consider explicitly re-raising using 'except (OSError, IndexError, struct.error, OverflowError, ValueError) as exc' and 'raise InvalidFileException() from exc':HIGH