diff options
author | Jakub Kuczys <me@jacken.men> | 2022-11-13 12:41:38 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-13 11:41:38 +0000 |
commit | ac65bdc0741379c3f2ac964626b31db93d49bf1e (patch) | |
tree | 265762220933cf40e60cac448e95b5fc73e231aa | |
parent | 88701fab1a87e57c3f5c42dda95c51915022ba51 (diff) | |
download | pylint-git-ac65bdc0741379c3f2ac964626b31db93d49bf1e.tar.gz |
Use qualified name when checking for overgeneral exceptions (#7497)
* Update tests
* Use qualified name when checking for overgeneral exceptions
* WIP: Add deprecation warning
* Add changelog fragment
* Use qualified name in test case
* spell check fix
* Update changelog fragment with suggested fixes
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
* Move OVERGENERAL_EXCEPTIONS directly to the default value in dict
* Mark as TODO for pylint 3.0
* Properly warn on each occurrence of name without dots
* Update the warning per the review
* Rephrase the warning to mention pylint 3.0
* Remove unnecessary nesting of the if condition
* Quote the exception name in deprecation warning
* Use config value for overgeneral exceptions in broad-exception-raised
* Infer qualified name of the exception in broad-exception-raised
* e.g. -> maybe?
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
* Suppress missing class docstrings
* Add few more tests for broad-exception-raised
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
* Fix unexpected missing-raise-from
* Revert "Fix unexpected missing-raise-from"
This reverts commit d796e72035b7f7578b9e6bb1e45a30935e80b009.
* Revert "Add few more tests for broad-exception-raised"
This reverts commit e5a193ee136f8566d43450fbb9fbf28cc717d307.
* Change confidence of broad-exception-raised from HIGH to INFERENCE
* Only trigger broad-exception-raised for raise with new exc instance
* Update overgeneral-exceptions definition in example pylintrc file
* Update pylint/checkers/exceptions.py
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
-rw-r--r-- | doc/user_guide/configuration/all-options.rst | 2 | ||||
-rw-r--r-- | doc/whatsnew/fragments/7495.bugfix | 4 | ||||
-rw-r--r-- | examples/pylintrc | 4 | ||||
-rw-r--r-- | examples/pyproject.toml | 2 | ||||
-rw-r--r-- | pylint/checkers/exceptions.py | 56 | ||||
-rw-r--r-- | pylintrc | 2 | ||||
-rw-r--r-- | tests/functional/b/bad_exception_cause.txt | 2 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_caught.py | 26 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_caught.rc | 4 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_caught.txt | 5 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_raised.py | 38 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_raised.rc | 4 | ||||
-rw-r--r-- | tests/functional/b/broad_exception_raised.txt | 13 |
13 files changed, 134 insertions, 28 deletions
diff --git a/doc/user_guide/configuration/all-options.rst b/doc/user_guide/configuration/all-options.rst index 0b38c0d2f..da0ce4669 100644 --- a/doc/user_guide/configuration/all-options.rst +++ b/doc/user_guide/configuration/all-options.rst @@ -812,7 +812,7 @@ Standard Checkers .. code-block:: toml [tool.pylint.exceptions] - overgeneral-exceptions = ["BaseException", "Exception"] + overgeneral-exceptions = ["builtins.BaseException", "builtins.Exception"] diff --git a/doc/whatsnew/fragments/7495.bugfix b/doc/whatsnew/fragments/7495.bugfix new file mode 100644 index 000000000..cec2bcf53 --- /dev/null +++ b/doc/whatsnew/fragments/7495.bugfix @@ -0,0 +1,4 @@ +Allow specifying non-builtin exceptions in the ``overgeneral-exception`` option +using an exception's qualified name. + +Closes #7495 diff --git a/examples/pylintrc b/examples/pylintrc index a461b24d5..608a8f269 100644 --- a/examples/pylintrc +++ b/examples/pylintrc @@ -307,8 +307,8 @@ min-public-methods=2 [EXCEPTIONS] # Exceptions that will emit a warning when caught. -overgeneral-exceptions=BaseException, - Exception +overgeneral-exceptions=builtins.BaseException, + builtins.Exception [FORMAT] diff --git a/examples/pyproject.toml b/examples/pyproject.toml index b419f83e7..c02538a7c 100644 --- a/examples/pyproject.toml +++ b/examples/pyproject.toml @@ -264,7 +264,7 @@ min-public-methods = 2 [tool.pylint.exceptions] # Exceptions that will emit a warning when caught. -overgeneral-exceptions = ["BaseException", "Exception"] +overgeneral-exceptions = ["builtins.BaseException", "builtins.Exception"] [tool.pylint.format] # Expected format of line ending, e.g. empty (any line ending), LF or CRLF. diff --git a/pylint/checkers/exceptions.py b/pylint/checkers/exceptions.py index 947d02204..a563c6eb9 100644 --- a/pylint/checkers/exceptions.py +++ b/pylint/checkers/exceptions.py @@ -8,6 +8,7 @@ from __future__ import annotations import builtins import inspect +import warnings from collections.abc import Generator from typing import TYPE_CHECKING, Any @@ -59,8 +60,6 @@ def _is_raising(body: list[nodes.NodeNG]) -> bool: return any(isinstance(node, nodes.Raise) for node in body) -OVERGENERAL_EXCEPTIONS = ("BaseException", "Exception") - MSGS: dict[str, MessageDefinitionTuple] = { "E0701": ( "Bad except clauses order (%s)", @@ -201,13 +200,23 @@ class ExceptionRaiseRefVisitor(BaseVisitor): self._checker.add_message( "notimplemented-raised", node=self._node, confidence=HIGH ) - elif node.name in OVERGENERAL_EXCEPTIONS: - self._checker.add_message( - "broad-exception-raised", - args=node.name, - node=self._node, - confidence=HIGH, - ) + return + + try: + exceptions = list(_annotated_unpack_infer(node)) + except astroid.InferenceError: + return + + for _, exception in exceptions: + if isinstance( + exception, nodes.ClassDef + ) and self._checker._is_overgeneral_exception(exception): + self._checker.add_message( + "broad-exception-raised", + args=exception.name, + node=self._node, + confidence=INFERENCE, + ) def visit_call(self, node: nodes.Call) -> None: if isinstance(node.func, nodes.Name): @@ -278,7 +287,7 @@ class ExceptionsChecker(checkers.BaseChecker): ( "overgeneral-exceptions", { - "default": OVERGENERAL_EXCEPTIONS, + "default": ("builtins.BaseException", "builtins.Exception"), "type": "csv", "metavar": "<comma-separated class names>", "help": "Exceptions that will emit a warning when caught.", @@ -288,6 +297,18 @@ class ExceptionsChecker(checkers.BaseChecker): def open(self) -> None: self._builtin_exceptions = _builtin_exceptions() + for exc_name in self.linter.config.overgeneral_exceptions: + if "." not in exc_name: + warnings.warn_explicit( + "Specifying exception names in the overgeneral-exceptions option" + " without module name is deprecated and support for it" + " will be removed in pylint 3.0." + f" Use fully qualified name (maybe 'builtins.{exc_name}' ?) instead.", + category=UserWarning, + filename="pylint: Command line or configuration file", + lineno=1, + module="pylint", + ) super().open() @utils.only_required_for_messages( @@ -592,10 +613,8 @@ class ExceptionsChecker(checkers.BaseChecker): args=msg, confidence=INFERENCE, ) - if ( - exception.name in self.linter.config.overgeneral_exceptions - and exception.root().name == utils.EXCEPTIONS_MODULE - and not _is_raising(handler.body) + if self._is_overgeneral_exception(exception) and not _is_raising( + handler.body ): self.add_message( "broad-exception-caught", @@ -614,6 +633,15 @@ class ExceptionsChecker(checkers.BaseChecker): exceptions_classes += [exc for _, exc in exceptions] + def _is_overgeneral_exception(self, exception: nodes.ClassDef) -> bool: + return ( + exception.qname() in self.linter.config.overgeneral_exceptions + # TODO: 3.0: not a qualified name, deprecated + or "." not in exception.name + and exception.name in self.linter.config.overgeneral_exceptions + and exception.root().name == utils.EXCEPTIONS_MODULE + ) + def register(linter: PyLinter) -> None: linter.register_checker(ExceptionsChecker(linter)) @@ -510,7 +510,7 @@ preferred-modules= # Exceptions that will emit a warning when being caught. Defaults to # "Exception" -overgeneral-exceptions=Exception +overgeneral-exceptions=builtins.Exception [TYPING] diff --git a/tests/functional/b/bad_exception_cause.txt b/tests/functional/b/bad_exception_cause.txt index 446a34ede..3aa50fa37 100644 --- a/tests/functional/b/bad_exception_cause.txt +++ b/tests/functional/b/bad_exception_cause.txt @@ -3,4 +3,4 @@ bad-exception-cause:16:4:16:34:test:Exception cause set to something which is no bad-exception-cause:22:4:22:36:test:Exception cause set to something which is not an exception, nor None:INFERENCE catching-non-exception:30:7:30:15::"Catching an exception which doesn't inherit from Exception: function":UNDEFINED bad-exception-cause:31:4:31:28::Exception cause set to something which is not an exception, nor None:INFERENCE -broad-exception-raised:31:4:31:28::"Raising too general exception: Exception":HIGH +broad-exception-raised:31:4:31:28::"Raising too general exception: Exception":INFERENCE diff --git a/tests/functional/b/broad_exception_caught.py b/tests/functional/b/broad_exception_caught.py index 5a14cc097..0a69a7015 100644 --- a/tests/functional/b/broad_exception_caught.py +++ b/tests/functional/b/broad_exception_caught.py @@ -1,6 +1,14 @@ # pylint: disable=missing-docstring __revision__ = 0 +class CustomBroadException(Exception): + pass + + +class CustomNarrowException(CustomBroadException): + pass + + try: __revision__ += 1 except Exception: # [broad-exception-caught] @@ -11,3 +19,21 @@ try: __revision__ += 1 except BaseException: # [broad-exception-caught] print('error') + + +try: + __revision__ += 1 +except ValueError: + print('error') + + +try: + __revision__ += 1 +except CustomBroadException: # [broad-exception-caught] + print('error') + + +try: + __revision__ += 1 +except CustomNarrowException: + print('error') diff --git a/tests/functional/b/broad_exception_caught.rc b/tests/functional/b/broad_exception_caught.rc new file mode 100644 index 000000000..e0e1a7b6c --- /dev/null +++ b/tests/functional/b/broad_exception_caught.rc @@ -0,0 +1,4 @@ +[EXCEPTIONS] +overgeneral-exceptions=builtins.BaseException, + builtins.Exception, + functional.b.broad_exception_caught.CustomBroadException diff --git a/tests/functional/b/broad_exception_caught.txt b/tests/functional/b/broad_exception_caught.txt index 256f5986f..386423b63 100644 --- a/tests/functional/b/broad_exception_caught.txt +++ b/tests/functional/b/broad_exception_caught.txt @@ -1,2 +1,3 @@ -broad-exception-caught:6:7:6:16::Catching too general exception Exception:INFERENCE -broad-exception-caught:12:7:12:20::Catching too general exception BaseException:INFERENCE +broad-exception-caught:14:7:14:16::Catching too general exception Exception:INFERENCE +broad-exception-caught:20:7:20:20::Catching too general exception BaseException:INFERENCE +broad-exception-caught:32:7:32:27::Catching too general exception CustomBroadException:INFERENCE diff --git a/tests/functional/b/broad_exception_raised.py b/tests/functional/b/broad_exception_raised.py index 65a509fb9..c6ce64b46 100644 --- a/tests/functional/b/broad_exception_raised.py +++ b/tests/functional/b/broad_exception_raised.py @@ -1,4 +1,14 @@ -# pylint: disable=missing-module-docstring, missing-function-docstring, unreachable +# pylint: disable=missing-docstring, unreachable + +ExceptionAlias = Exception + +class CustomBroadException(Exception): + pass + + +class CustomNarrowException(CustomBroadException): + pass + def exploding_apple(apple): print(f"{apple} is about to explode") @@ -11,6 +21,32 @@ def raise_and_catch(): except Exception as ex: # [broad-exception-caught] print(ex) + +def raise_catch_reraise(): + try: + exploding_apple("apple") + except Exception as ex: + print(ex) + raise ex + + +def raise_catch_raise(): + try: + exploding_apple("apple") + except Exception as ex: + print(ex) + raise Exception() from None # [broad-exception-raised] + + +def raise_catch_raise_using_alias(): + try: + exploding_apple("apple") + except Exception as ex: + print(ex) + raise ExceptionAlias() from None # [broad-exception-raised] + raise Exception() # [broad-exception-raised] raise BaseException() # [broad-exception-raised] +raise CustomBroadException() # [broad-exception-raised] raise IndexError from None +raise CustomNarrowException() from None diff --git a/tests/functional/b/broad_exception_raised.rc b/tests/functional/b/broad_exception_raised.rc new file mode 100644 index 000000000..4f85d2933 --- /dev/null +++ b/tests/functional/b/broad_exception_raised.rc @@ -0,0 +1,4 @@ +[EXCEPTIONS] +overgeneral-exceptions=builtins.BaseException, + builtins.Exception, + functional.b.broad_exception_raised.CustomBroadException diff --git a/tests/functional/b/broad_exception_raised.txt b/tests/functional/b/broad_exception_raised.txt index 9482775c3..1e27b23f9 100644 --- a/tests/functional/b/broad_exception_raised.txt +++ b/tests/functional/b/broad_exception_raised.txt @@ -1,5 +1,8 @@ -broad-exception-raised:5:4:5:41:exploding_apple:"Raising too general exception: Exception":HIGH -broad-exception-raised:10:8:10:34:raise_and_catch:"Raising too general exception: Exception":HIGH -broad-exception-caught:11:11:11:20:raise_and_catch:Catching too general exception Exception:INFERENCE -broad-exception-raised:14:0:14:17::"Raising too general exception: Exception":HIGH -broad-exception-raised:15:0:15:21::"Raising too general exception: BaseException":HIGH +broad-exception-raised:15:4:15:41:exploding_apple:"Raising too general exception: Exception":INFERENCE +broad-exception-raised:20:8:20:34:raise_and_catch:"Raising too general exception: Exception":INFERENCE +broad-exception-caught:21:11:21:20:raise_and_catch:Catching too general exception Exception:INFERENCE +broad-exception-raised:38:8:38:35:raise_catch_raise:"Raising too general exception: Exception":INFERENCE +broad-exception-raised:46:8:46:40:raise_catch_raise_using_alias:"Raising too general exception: Exception":INFERENCE +broad-exception-raised:48:0:48:17::"Raising too general exception: Exception":INFERENCE +broad-exception-raised:49:0:49:21::"Raising too general exception: BaseException":INFERENCE +broad-exception-raised:50:0:50:28::"Raising too general exception: CustomBroadException":INFERENCE |