Skip to content
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

rename broad-except and new check broad-exception-raised #7709

Merged
merged 8 commits into from
Nov 5, 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: 0 additions & 4 deletions doc/data/messages/b/broad-except/bad.py

This file was deleted.

4 changes: 4 additions & 0 deletions doc/data/messages/b/broad-exception-caught/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
try:
1 / 0
except Exception: # [broad-exception-caught]
pass
4 changes: 4 additions & 0 deletions doc/data/messages/b/broad-exception-raised/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
def small_apple(apple, length):
if len(apple) < length:
raise Exception("Apple is too small!") # [broad-exception-raised]
print(f"{apple} is proper size.")
4 changes: 4 additions & 0 deletions doc/data/messages/b/broad-exception-raised/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
def small_apple(apple, length):
if len(apple) < length:
raise ValueError("Apple is too small!")
print(f"{apple} is proper size.")
4 changes: 3 additions & 1 deletion doc/user_guide/checkers/features.rst
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ Exceptions checker Messages
:duplicate-except (W0705): *Catching previously caught exception type %s*
Used when an except catches a type that was already caught by a previous
handler.
:broad-except (W0703): *Catching too general exception %s*
:broad-exception-caught (W0718): *Catching too general exception %s*
Used when an except catches a too general exception, possibly burying
unrelated errors.
:raise-missing-from (W0707): *Consider explicitly re-raising using %s'%s from %s'*
Expand All @@ -462,6 +462,8 @@ Exceptions checker Messages
operations between exceptions in except handlers.
:bare-except (W0702): *No exception type(s) specified*
Used when an except clause doesn't specify exceptions type to catch.
:broad-exception-raised (W0719): *Raising too general exception: %s*
Used when an except raises a too general exception.
:try-except-raise (W0706): *The except handler raises immediately*
Used when an except handler uses raise as its first or only operator. This is
useless because it raises back the exception immediately. Remove the raise
Expand Down
4 changes: 3 additions & 1 deletion doc/user_guide/messages/messages_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ All messages in the warning category:
warning/bare-except
warning/binary-op-exception
warning/boolean-datetime
warning/broad-except
warning/broad-exception-caught
warning/broad-exception-raised
warning/cell-var-from-loop
warning/comparison-with-callable
warning/confusing-with-statement
Expand Down Expand Up @@ -344,6 +345,7 @@ All renamed messages in the warning category:
:maxdepth: 1
:titlesonly:

warning/broad-except
warning/cache-max-size-none
warning/implicit-str-concat-in-sequence
warning/lru-cache-decorating-method
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7494.new_check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Rename ``broad-except`` to ``broad-exception-caught`` and add new checker ``broad-exception-raised``
which will warn if general exceptions ``BaseException`` or ``Exception`` are raised.

Closes #7494
24 changes: 20 additions & 4 deletions pylint/checkers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,12 @@ def _is_raising(body: list[nodes.NodeNG]) -> bool:
"bare-except",
"Used when an except clause doesn't specify exceptions type to catch.",
),
"W0703": (
"W0718": (
"Catching too general exception %s",
"broad-except",
"broad-exception-caught",
"Used when an except catches a too general exception, "
"possibly burying unrelated errors.",
{"old_names": [("W0703", "broad-except")]},
),
"W0705": (
"Catching previously caught exception type %s",
Expand Down Expand Up @@ -165,6 +166,11 @@ def _is_raising(body: list[nodes.NodeNG]) -> bool:
"is not valid for the exception in question. Usually emitted when having "
"binary operations between exceptions in except handlers.",
),
"W0719": (
"Raising too general exception: %s",
"broad-exception-raised",
"Used when an except raises a too general exception.",
),
}


Expand Down Expand Up @@ -193,6 +199,13 @@ class ExceptionRaiseRefVisitor(BaseVisitor):
def visit_name(self, node: nodes.Name) -> None:
if node.name == "NotImplemented":
self._checker.add_message("notimplemented-raised", node=self._node)
elif node.name in OVERGENERAL_EXCEPTIONS:
self._checker.add_message(
"broad-exception-raised",
args=node.name,
node=self._node,
confidence=HIGH,
)

def visit_call(self, node: nodes.Call) -> None:
if isinstance(node.func, nodes.Name):
Expand Down Expand Up @@ -264,6 +277,7 @@ def open(self) -> None:
"bad-exception-cause",
"raising-format-tuple",
"raise-missing-from",
"broad-exception-raised",
)
def visit_raise(self, node: nodes.Raise) -> None:
if node.exc is None:
Expand Down Expand Up @@ -493,7 +507,7 @@ def visit_compare(self, node: nodes.Compare) -> None:

@utils.only_required_for_messages(
"bare-except",
"broad-except",
"broad-exception-caught",
"try-except-raise",
"binary-op-exception",
"bad-except-order",
Expand Down Expand Up @@ -555,7 +569,9 @@ def visit_tryexcept(self, node: nodes.TryExcept) -> None:
and not _is_raising(handler.body)
):
self.add_message(
"broad-except", args=exception.name, node=handler.type
"broad-exception-caught",
args=exception.name,
node=handler.type,
clavedeluna marked this conversation as resolved.
Show resolved Hide resolved
)

if exception in exceptions_classes:
Expand Down
2 changes: 1 addition & 1 deletion pylint/config/option.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def _py_version_validator(_: Any, name: str, value: Any) -> tuple[int, int, int]

def _call_validator(opttype: str, optdict: Any, option: str, value: Any) -> Any:
if opttype not in VALIDATORS:
raise Exception(f'Unsupported type "{opttype}"')
raise TypeError(f'Unsupported type "{opttype}"')
try:
return VALIDATORS[opttype](optdict, option, value) # type: ignore[call-arg]
except TypeError:
Expand Down
2 changes: 1 addition & 1 deletion pylint/lint/parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def _worker_check_single_file(
defaultdict[str, list[Any]],
]:
if not _worker_linter:
raise Exception("Worker linter not yet initialised")
raise RuntimeError("Worker linter not yet initialised")
_worker_linter.open()
_worker_linter.check_single_file_item(file_item)
mapreduce_data = defaultdict(list)
Expand Down
4 changes: 2 additions & 2 deletions pylint/pyreverse/vcg_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def _write_attributes(
try:
_type = attributes_dict[key]
except KeyError as e:
raise Exception(
raise AttributeError(
f"no such attribute {key}\npossible attributes are {attributes_dict.keys()}"
) from e

Expand All @@ -284,6 +284,6 @@ def _write_attributes(
elif value in _type:
self.emit(f"{key}:{value}\n")
else:
raise Exception(
raise ValueError(
f"value {value} isn't correct for attribute {key} correct values are {type}"
)
2 changes: 1 addition & 1 deletion tests/functional/b/bad_exception_cause.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ def function():
try:
pass
except function as exc: # [catching-non-exception]
raise Exception from exc # [bad-exception-cause]
raise Exception from exc # [bad-exception-cause, broad-exception-raised]
1 change: 1 addition & 0 deletions tests/functional/b/bad_exception_cause.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +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
2 changes: 0 additions & 2 deletions tests/functional/b/broad_except.txt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

try:
__revision__ += 1
except Exception: # [broad-except]
except Exception: # [broad-exception-caught]
print('error')


try:
__revision__ += 1
except BaseException: # [broad-except]
except BaseException: # [broad-exception-caught]
print('error')
2 changes: 2 additions & 0 deletions tests/functional/b/broad_exception_caught.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
broad-exception-caught:6:7:6:16::Catching too general exception Exception:UNDEFINED
broad-exception-caught:12:7:12:20::Catching too general exception BaseException:UNDEFINED
16 changes: 16 additions & 0 deletions tests/functional/b/broad_exception_raised.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# pylint: disable=missing-module-docstring, missing-function-docstring, unreachable

def exploding_apple(apple):
print(f"{apple} is about to explode")
raise Exception("{apple} exploded !") # [broad-exception-raised]


def raise_and_catch():
try:
raise Exception("Oh No!!") # [broad-exception-raised]
except Exception as ex: # [broad-exception-caught]
print(ex)

raise Exception() # [broad-exception-raised]
raise BaseException() # [broad-exception-raised]
raise IndexError from None
5 changes: 5 additions & 0 deletions tests/functional/b/broad_exception_raised.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
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:UNDEFINED
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
2 changes: 1 addition & 1 deletion tests/functional/c/comparison_with_callable.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable = disallowed-name, missing-docstring, useless-return, invalid-name, line-too-long, comparison-of-constants
# pylint: disable = disallowed-name, missing-docstring, useless-return, invalid-name, line-too-long, comparison-of-constants, broad-exception-raised
def foo():
return None

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/ext/docparams/docparams.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""Fixture for testing missing documentation in docparams."""

# pylint: disable=broad-exception-raised

def _private_func1( # [missing-return-doc, missing-return-type-doc, missing-any-param-doc]
param1,
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/ext/mccabe/mccabe.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# pylint: disable=invalid-name,unnecessary-pass,no-else-return,useless-else-on-loop
# pylint: disable=undefined-variable,consider-using-sys-exit,unused-variable,too-many-return-statements
# pylint: disable=redefined-outer-name,using-constant-test,unused-argument
# pylint: disable=broad-except, not-context-manager, no-method-argument, unspecified-encoding
# pylint: disable=broad-except, not-context-manager, no-method-argument, unspecified-encoding, broad-exception-raised

"""Checks use of "too-complex" check"""

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/ext/typing/typing_broken_noreturn.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

If no runtime introspection is required, use string annotations instead.
"""
# pylint: disable=missing-docstring
# pylint: disable=missing-docstring, broad-exception-raised
import typing
from typing import TYPE_CHECKING, Callable, NoReturn, Union

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
With 'from __future__ import annotations', only emit errors for nodes
not in a type annotation context.
"""
# pylint: disable=missing-docstring
# pylint: disable=missing-docstring, broad-exception-raised
from __future__ import annotations

import typing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

Don't emit errors if py-version set to >= 3.7.2.
"""
# pylint: disable=missing-docstring
# pylint: disable=missing-docstring, broad-exception-raised
import typing
from typing import TYPE_CHECKING, Callable, NoReturn, Union

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/n/no/no_else_raise.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
""" Test that superfluous else raise are detected. """

# pylint:disable=invalid-name,missing-docstring,unused-variable,raise-missing-from
# pylint:disable=invalid-name,missing-docstring,unused-variable,raise-missing-from,broad-exception-raised

def foo1(x, y, z):
if x: # [no-else-raise]
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/r/regression_02/regression_5048.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Crash regression in astroid on Compare node inference
Fixed in https://github.com/PyCQA/astroid/pull/1185"""
# pylint: disable=missing-docstring
# pylint: disable=missing-docstring, broad-exception-raised


# Reported at https://github.com/PyCQA/pylint/issues/5048
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/s/stop_iteration_inside_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
Test that no StopIteration is raised inside a generator
"""
# pylint: disable=missing-docstring,invalid-name,import-error, try-except-raise, wrong-import-position,not-callable,raise-missing-from
# pylint: disable=broad-exception-raised
import asyncio


class RebornStopIteration(StopIteration):
"""
A class inheriting from StopIteration exception
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/undefined/undefined_loop_variable.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable=missing-docstring,redefined-builtin, consider-using-f-string, unnecessary-direct-lambda-call
# pylint: disable=missing-docstring,redefined-builtin, consider-using-f-string, unnecessary-direct-lambda-call, broad-exception-raised

import sys

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/unreachable.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable=missing-docstring
# pylint: disable=missing-docstring, broad-exception-raised


def func1():
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/unused/unused_variable.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, import-outside-toplevel, fixme, line-too-long
# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, import-outside-toplevel, fixme, line-too-long, broad-exception-raised

def test_regression_737():
import xml # [unused-import]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Homonym between filtered comprehension and assignment in except block."""
# pylint: disable=broad-exception-raised

def func():
"""https://github.com/PyCQA/pylint/issues/5586"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
try blocks with return statements.
See: https://github.com/PyCQA/pylint/issues/5500.
"""
# pylint: disable=inconsistent-return-statements
# pylint: disable=inconsistent-return-statements,broad-exception-raised


def function():
Expand Down
2 changes: 1 addition & 1 deletion tests/lint/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_prepare_crash_report(tmp_path: PosixPath) -> None:
with open(python_file, "w", encoding="utf8") as f:
f.write(python_content)
try:
raise Exception(exception_content)
raise Exception(exception_content) # pylint: disable=broad-exception-raised
except Exception as ex: # pylint: disable=broad-except
template_path = prepare_crash_report(
ex, str(python_file), str(tmp_path / "pylint-crash-%Y.txt")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_check_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def test_worker_initialize_pickling(self) -> None:
def test_worker_check_single_file_uninitialised(self) -> None:
pylint.lint.parallel._worker_linter = None
with pytest.raises( # Objects that do not match the linter interface will fail
Exception, match="Worker linter not yet initialised"
RuntimeError, match="Worker linter not yet initialised"
):
worker_check_single_file(_gen_file_data())

Expand Down
16 changes: 8 additions & 8 deletions tests/test_self.py
Original file line number Diff line number Diff line change
Expand Up @@ -729,14 +729,14 @@ def test_fail_under(self) -> None:
(-9, "missing-function-docstring", "fail_under_minus10.py", 22),
(-5, "missing-function-docstring", "fail_under_minus10.py", 22),
# --fail-under should guide whether error code as missing-function-docstring is not hit
(-10, "broad-except", "fail_under_plus7_5.py", 0),
(6, "broad-except", "fail_under_plus7_5.py", 0),
(7.5, "broad-except", "fail_under_plus7_5.py", 0),
(7.6, "broad-except", "fail_under_plus7_5.py", 16),
(-11, "broad-except", "fail_under_minus10.py", 0),
(-10, "broad-except", "fail_under_minus10.py", 0),
(-9, "broad-except", "fail_under_minus10.py", 22),
(-5, "broad-except", "fail_under_minus10.py", 22),
(-10, "broad-exception-caught", "fail_under_plus7_5.py", 0),
(6, "broad-exception-caught", "fail_under_plus7_5.py", 0),
(7.5, "broad-exception-caught", "fail_under_plus7_5.py", 0),
(7.6, "broad-exception-caught", "fail_under_plus7_5.py", 16),
(-11, "broad-exception-caught", "fail_under_minus10.py", 0),
(-10, "broad-exception-caught", "fail_under_minus10.py", 0),
(-9, "broad-exception-caught", "fail_under_minus10.py", 22),
(-5, "broad-exception-caught", "fail_under_minus10.py", 22),
# Enable by message id
(-10, "C0116", "fail_under_plus7_5.py", 16),
# Enable by category
Expand Down