pylint
pylint copied to clipboard
overgeneral-exceptions doesn't work for exceptions outside builtins
Bug description
# pylint: disable=missing-docstring
class GeneralException(Exception):
pass
try:
raise GeneralException('...')
except GeneralException:
...
# for comparison, pylint triggers on this
except Exception:
...
Configuration
No response
Command used
pylint repro.py --overgeneral-exceptions BaseException,Exception,GeneralException
Pylint output
************* Module repro
repro.py:12:7: W0703: Catching too general exception Exception (broad-except)
------------------------------------------------------------------
Your code has been rated at 8.75/10 (previous run: 8.75/10, +0.00)
Expected behavior
I expected to get broad-except message for line 9 (which is not happening regardless of whether except Exception is there or not).
This is also reproducible when making a whole package and referring to the exception class using package.GeneralException name (which I personally think would be the best way of specifying non-builtin exceptions).
This seems to vastly limit the usefulness of --overgeneral-exceptions.
Pylint version
pylint 2.15.3
astroid 2.12.10
Python 3.10.4 (main, Jun 29 2022, 12:14:53) [GCC 11.2.0]
OS / Environment
Kubuntu 22.04.1 LTS
Additional dependencies
No response
For some reason we check whether the exception is in builtins here:
https://github.com/PyCQA/pylint/blob/7ef145b9a7042ab884aa2d7b554fcdd96768815a/pylint/checkers/exceptions.py#L554
I'd be happy to review a PR that removes this check, it does indeed seem silly.
Do you think pylint should only check the name, or should it try matching the module name if a dotted name is provided and built-in if not?
The only public usage I could find was my own (using the dotted name) and Home Assistant (using just the exception name), everyone else who specifies overgeneral-exceptions only specifies one or more of Exception, BaseException, and StandardError (which is surely just leftover from Python 1.x :smile:)
Do you think pylint should only check the name, or should it try matching the module name if a dotted name is provided and built-in if not?
The only public usage I could find was my own (using the dotted name) and Home Assistant (using just the exception name), everyone else who specifies
overgeneral-exceptionsonly specifies one or more ofException,BaseException, andStandardError(which is surely just leftover from Python 1.x 😄)
Perhaps do:
if name in {"Exception", "BaseException"} and name in self.linter.config.overgeneral_exceptions and exception.root().name == utils.EXCEPTIONS_MODULE`
or exception.qname() in self.linter.config.overgeneral_exceptions
So, for Exception and BaseException allow not having them as qualified names and for all others make sure they should include the module name. This is backwards incompatible but since the current form is completely unusable we can get away with this I think.
Personally I would even prefer if we could issue a DeprecationWarning for Exception and not builtins.Exception (so case 1 in the code) but that is perhaps a bit too much for someone just looking to quickly fix this.