diff --git a/doc/data/messages/r/raise-missing-from/bad.py b/doc/data/messages/r/raise-missing-from/bad.py new file mode 100644 index 0000000000..8b5ad265bc --- /dev/null +++ b/doc/data/messages/r/raise-missing-from/bad.py @@ -0,0 +1,4 @@ +try: + 1 / 0 +except ZeroDivisionError as e: + raise ValueError("Rectangle Area cannot be zero") # [raise-missing-from] diff --git a/doc/data/messages/r/raise-missing-from/good.py b/doc/data/messages/r/raise-missing-from/good.py new file mode 100644 index 0000000000..9a5cdef255 --- /dev/null +++ b/doc/data/messages/r/raise-missing-from/good.py @@ -0,0 +1,4 @@ +try: + 1 / 0 +except ZeroDivisionError as e: + raise ValueError("Rectangle Area cannot be zero") from e diff --git a/doc/data/messages/r/raise-missing-from/related.rst b/doc/data/messages/r/raise-missing-from/related.rst new file mode 100644 index 0000000000..1bcca1b7b7 --- /dev/null +++ b/doc/data/messages/r/raise-missing-from/related.rst @@ -0,0 +1 @@ +- `PEP 3132 `_ diff --git a/pylint/checkers/exceptions.py b/pylint/checkers/exceptions.py index 88cd527b73..52ef41a806 100644 --- a/pylint/checkers/exceptions.py +++ b/pylint/checkers/exceptions.py @@ -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: @@ -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', @@ -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): diff --git a/tests/functional/r/raise_missing_from.py b/tests/functional/r/raise_missing_from.py index d23cefb5e1..897a0e9c4f 100644 --- a/tests/functional/r/raise_missing_from.py +++ b/tests/functional/r/raise_missing_from.py @@ -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 @@ -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: @@ -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 @@ -107,10 +126,6 @@ except ZeroDivisionError as e: raise KeyError(whatever, whatever=whatever) from foo - -################################################################################ -# Other negatives: - try: 1 / 0 except ZeroDivisionError: diff --git a/tests/functional/r/raise_missing_from.txt b/tests/functional/r/raise_missing_from.txt index 1e163ed566..1cbcbdba4b 100644 --- a/tests/functional/r/raise_missing_from.txt +++ b/tests/functional/r/raise_missing_from.txt @@ -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