diff --git a/doc/data/messages/r/redundant-exception-message/bad.py b/doc/data/messages/r/redundant-exception-message/bad.py new file mode 100644 index 0000000000..d8606ded9c --- /dev/null +++ b/doc/data/messages/r/redundant-exception-message/bad.py @@ -0,0 +1,4 @@ +try: + save_config(data) +except OSError as err: + raise ConfigError(f"Error: {err}") from err # [redundant-exception-message] diff --git a/doc/data/messages/r/redundant-exception-message/details.rst b/doc/data/messages/r/redundant-exception-message/details.rst new file mode 100644 index 0000000000..ad39511fde --- /dev/null +++ b/doc/data/messages/r/redundant-exception-message/details.rst @@ -0,0 +1,53 @@ +When using ``raise ... from original_exception``, Python automatically displays +the original exception in the traceback with the message "The above exception +was the direct cause of the following exception". Including the original +exception in the new message is therefore redundant. + +**With redundant message (bad):** + +.. code-block:: python + + try: + raise ValueError("Invalid format in config.yaml") + except ValueError as e: + raise RuntimeError(f"Failed to load config: {e}") from e + +.. code-block:: text + + Traceback (most recent call last): + File "example.py", line 2, in load_config + raise ValueError("Invalid format in config.yaml") + ValueError: Invalid format in config.yaml + + The above exception was the direct cause of the following exception: + + Traceback (most recent call last): + File "example.py", line 4, in load_config + raise RuntimeError(f"Failed to load config: {e}") from e + RuntimeError: Failed to load config: Invalid format in config.yaml + +**Without redundant message (good):** + +.. code-block:: python + + try: + raise ValueError("Invalid format in config.yaml") + except ValueError as e: + raise RuntimeError("Failed to load config") from e + +.. code-block:: text + + Traceback (most recent call last): + File "example.py", line 2, in load_config + raise ValueError("Invalid format in config.yaml") + ValueError: Invalid format in config.yaml + + The above exception was the direct cause of the following exception: + + Traceback (most recent call last): + File "example.py", line 4, in load_config + raise RuntimeError("Failed to load config") from e + RuntimeError: Failed to load config + +The exception chaining mechanism ensures all context is preserved without +message duplication. diff --git a/doc/data/messages/r/redundant-exception-message/good.py b/doc/data/messages/r/redundant-exception-message/good.py new file mode 100644 index 0000000000..4dc0de18c1 --- /dev/null +++ b/doc/data/messages/r/redundant-exception-message/good.py @@ -0,0 +1,4 @@ +try: + save_config(data) +except OSError as err: + raise ConfigError("Failed to save configuration") from err diff --git a/doc/data/messages/r/redundant-exception-message/related.rst b/doc/data/messages/r/redundant-exception-message/related.rst new file mode 100644 index 0000000000..5dfb0d03ca --- /dev/null +++ b/doc/data/messages/r/redundant-exception-message/related.rst @@ -0,0 +1,2 @@ +- `PEP 3134 - Exception Chaining `_ +- `raise-missing-from `_ diff --git a/doc/whatsnew/fragments/10792.new_check b/doc/whatsnew/fragments/10792.new_check new file mode 100644 index 0000000000..945774b01e --- /dev/null +++ b/doc/whatsnew/fragments/10792.new_check @@ -0,0 +1,3 @@ +Add ``redundant-exception-message`` check (W0720) that detects when the original exception is included in the message of a re-raised exception using ``raise ... from``. This is redundant since Python's exception chaining preserves the original exception via ``__cause__``. + +Closes #10792 diff --git a/pylint/checkers/exceptions.py b/pylint/checkers/exceptions.py index b8897a17f0..af5ff606ee 100644 --- a/pylint/checkers/exceptions.py +++ b/pylint/checkers/exceptions.py @@ -177,6 +177,16 @@ def _is_raising(body: list[nodes.NodeNG]) -> bool: "you expect to catch. This can hide bugs or make it harder to debug programs " "when unrelated errors are hidden.", ), + "W0720": ( + "Redundant exception message: '%s' included in message with 'raise ... from %s'", + "redundant-exception-message", + "When using 'raise ... from', the original exception is automatically " + "chained and its message is preserved via __cause__. Including the " + "original error message (via str(err), f-string, or concatenation) " + "results in duplicate information in logs. Use a descriptive message " + "without the original error, or extract specific context like file paths.", + {"default_enabled": False}, + ), } @@ -311,6 +321,7 @@ def open(self) -> None: "raising-format-tuple", "raise-missing-from", "broad-exception-raised", + "redundant-exception-message", ) def visit_raise(self, node: nodes.Raise) -> None: if node.exc is None: @@ -321,6 +332,7 @@ def visit_raise(self, node: nodes.Raise) -> None: self._check_raise_missing_from(node) else: self._check_bad_exception_cause(node) + self._check_redundant_exception_message(node) expr = node.exc ExceptionRaiseRefVisitor(self, node).visit(expr) @@ -414,6 +426,80 @@ def _check_raise_missing_from(self, node: nodes.Raise) -> None: confidence=HIGH, ) + def _check_redundant_exception_message(self, node: nodes.Raise) -> None: + """Check for redundant exception message when using 'raise ... from'. + + Detects patterns like: + raise SomeError(f"message: {err}") from err + raise SomeError(f"message: {str(err)}") from err + raise SomeError("message: " + str(err)) from err + """ + if node.cause is None or not isinstance(node.exc, nodes.Call): + return + + # Get the name of the chained exception variable + cause_name: str | None = None + if isinstance(node.cause, nodes.Name): + cause_name = node.cause.name + else: + return # Can't analyze complex cause expressions + + # Check the arguments of the raised exception + for arg in node.exc.args: + if self._contains_exception_in_message(arg, cause_name): + self.add_message( + "redundant-exception-message", + node=node, + args=(cause_name, cause_name), + confidence=HIGH, + ) + return + + def _is_str_call_of(self, node: nodes.NodeNG, name: str) -> bool: + """Check if node is a str(name) call.""" + return ( + isinstance(node, nodes.Call) + and isinstance(node.func, nodes.Name) + and node.func.name == "str" + and node.args + and isinstance(node.args[0], nodes.Name) + and node.args[0].name == name + ) + + def _contains_exception_in_message(self, node: nodes.NodeNG, exc_name: str) -> bool: + """Check if the node contains a reference to the exception variable. + + Detects: + - f"...{err}..." or f"...{err!s}..." or f"...{err!r}..." + - f"...{str(err)}..." + - "..." + str(err) + - str(err) in concatenation + """ + if isinstance(node, nodes.JoinedStr): + # f-string: check for {err} or {str(err)} + for value in node.values: + if isinstance(value, nodes.FormattedValue): + inner = value.value + # Direct reference: f"{err}" + if isinstance(inner, nodes.Name) and inner.name == exc_name: + return True + # str(err) call: f"{str(err)}" + if self._is_str_call_of(inner, exc_name): + return True + elif isinstance(node, nodes.BinOp) and node.op == "+": + # String concatenation: "message: " + str(err) + return self._contains_exception_in_message( + node.left, exc_name + ) or self._contains_exception_in_message(node.right, exc_name) + elif self._is_str_call_of(node, exc_name): + # str(err) directly as argument + return True + elif isinstance(node, nodes.Name) and node.name == exc_name: + # Direct reference as argument (rare but possible) + return True + + return False + def _check_catching_non_exception( self, handler: nodes.ExceptHandler, diff --git a/tests/checkers/unittest_unicode/unittest_bad_chars.py b/tests/checkers/unittest_unicode/unittest_bad_chars.py index 293a4acd60..5e56bd7591 100644 --- a/tests/checkers/unittest_unicode/unittest_bad_chars.py +++ b/tests/checkers/unittest_unicode/unittest_bad_chars.py @@ -72,7 +72,7 @@ def _bad_char_file_generator( byte_line.decode(codec, "strict") except UnicodeDecodeError as e: raise ValueError( - f"Line {lineno} did raise unexpected error: {byte_line!r}\n{e}" + f"Line {lineno} did raise unexpected error: {byte_line!r}" ) from e else: try: diff --git a/tests/functional/r/redundant_exception_message.py b/tests/functional/r/redundant_exception_message.py new file mode 100644 index 0000000000..dc1f0097ae --- /dev/null +++ b/tests/functional/r/redundant_exception_message.py @@ -0,0 +1,93 @@ +# pylint: disable=missing-docstring, broad-exception-caught, undefined-variable +# pylint: disable=raise-missing-from, line-too-long, invalid-name + + +class ConfigError(Exception): + """Custom exception for configuration errors.""" + + +# ===== BAD CASES (should trigger redundant-exception-message) ===== + +# f-string with direct exception reference +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError(f"Failed to save config: {err}") from err # [redundant-exception-message] + +# f-string with str(err) +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError(f"Failed to save config: {str(err)}") from err # [redundant-exception-message] + +# String concatenation with str(err) +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError("Failed to save config: " + str(err)) from err # [redundant-exception-message] + +# str(err) as sole argument +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError(str(err)) from err # [redundant-exception-message] + +# Direct exception reference as argument +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError(err) from err # [redundant-exception-message] + +# Nested concatenation +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError("Error: " + "details: " + str(err)) from err # [redundant-exception-message] + + +# ===== GOOD CASES (should NOT trigger redundant-exception-message) ===== + +# Simple message without exception reference +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError("Failed to save configuration") from err + +# Message with specific context (not the exception itself) +try: + 1 / 0 +except ZeroDivisionError as err: + path = "/path/to/config" + raise ConfigError(f"Config file not found: {path}") from err + +# f-string with different variable +try: + 1 / 0 +except ZeroDivisionError as err: + context = "user settings" + raise ConfigError(f"Failed to save {context}") from err + +# raise without from (separate rule: raise-missing-from) +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError(f"Failed: {err}") + +# raise from None (explicit suppression of context) +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError("Failed to save config") from None + +# No arguments to exception +try: + 1 / 0 +except ZeroDivisionError as err: + raise ConfigError from err + +# Different exception variable in from clause +try: + 1 / 0 +except ZeroDivisionError as err: + other_err = ValueError("other") + raise ConfigError(f"Error: {err}") from other_err diff --git a/tests/functional/r/redundant_exception_message.txt b/tests/functional/r/redundant_exception_message.txt new file mode 100644 index 0000000000..bbd7d5308f --- /dev/null +++ b/tests/functional/r/redundant_exception_message.txt @@ -0,0 +1,6 @@ +redundant-exception-message:15:4:15:63::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH +redundant-exception-message:21:4:21:68::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH +redundant-exception-message:27:4:27:68::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH +redundant-exception-message:33:4:33:40::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH +redundant-exception-message:39:4:39:35::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH +redundant-exception-message:45:4:45:66::"Redundant exception message: 'err' included in message with 'raise ... from err'":HIGH