diff --git a/crates/ruff/src/rules/tryceratops/rules/error_instead_of_exception.rs b/crates/ruff/src/rules/tryceratops/rules/error_instead_of_exception.rs index 97b3e1537a639..abd471a49111c 100644 --- a/crates/ruff/src/rules/tryceratops/rules/error_instead_of_exception.rs +++ b/crates/ruff/src/rules/tryceratops/rules/error_instead_of_exception.rs @@ -8,6 +8,41 @@ use ruff_python_ast::visitor::Visitor; use crate::checkers::ast::Checker; use crate::rules::tryceratops::helpers::LoggerCandidateVisitor; +/// ## What it does +/// Checks for uses of `logging.error` instead of `logging.exception` when +/// logging an exception. +/// +/// ## Why is this bad? +/// `logging.exception` logs the exception and the traceback, while +/// `logging.error` only logs the exception. The former is more appropriate +/// when logging an exception, as the traceback is often useful for debugging. +/// +/// ## Example +/// ```python +/// import logging +/// +/// +/// def foo(): +/// try: +/// raise NotImplementedError +/// except NotImplementedError: +/// logging.error("Exception occurred") +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// +/// def foo(): +/// try: +/// raise NotImplementedError +/// except NotImplementedError as exc: +/// logging.exception("Exception occurred") +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/logging.html#logging.exception) #[violation] pub struct ErrorInsteadOfException; diff --git a/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs b/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs index 58af731e1d28e..008c92810947f 100644 --- a/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs +++ b/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs @@ -6,6 +6,40 @@ use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for long exception messages that are not defined in the exception +/// class itself. +/// +/// ## Why is this bad? +/// By formatting an exception message at the `raise` site, the exception class +/// becomes less reusable, and may now raise inconsistent messages depending on +/// where it is raised. +/// +/// If the exception message is instead defined within the exception class, it +/// will be consistent across all `raise` invocations. +/// +/// ## Example +/// ```python +/// class CantBeNegative(Exception): +/// pass +/// +/// +/// def foo(x): +/// if x < 0: +/// raise CantBeNegative(f"{x} is negative") +/// ``` +/// +/// Use instead: +/// ```python +/// class CantBeNegative(Exception): +/// def __init__(self, number): +/// super().__init__(f"{number} is negative") +/// +/// +/// def foo(x): +/// if x < 0: +/// raise CantBeNegative(x) +/// ``` #[violation] pub struct RaiseVanillaArgs; diff --git a/crates/ruff/src/rules/tryceratops/rules/raise_within_try.rs b/crates/ruff/src/rules/tryceratops/rules/raise_within_try.rs index d22a537fde16a..15931a68a050c 100644 --- a/crates/ruff/src/rules/tryceratops/rules/raise_within_try.rs +++ b/crates/ruff/src/rules/tryceratops/rules/raise_within_try.rs @@ -7,6 +7,43 @@ use ruff_python_ast::visitor::{self, Visitor}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for `raise` statements within `try` blocks. +/// +/// ## Why is this bad? +/// Raising and catching exceptions within the same `try` block is redundant, +/// as the code can be refactored to avoid the `try` block entirely. +/// +/// Alternatively, the `raise` can be moved within an inner function, making +/// the exception reusable across multiple call sites. +/// +/// ## Example +/// ```python +/// def bar(): +/// pass +/// +/// +/// def foo(): +/// try: +/// a = bar() +/// if not a: +/// raise ValueError +/// except ValueError: +/// raise +/// ``` +/// +/// Use instead: +/// ```python +/// def bar(): +/// raise ValueError +/// +/// +/// def foo(): +/// try: +/// a = bar() # refactored `bar` to raise `ValueError` +/// except ValueError: +/// raise +/// ``` #[violation] pub struct RaiseWithinTry; diff --git a/crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs b/crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs index f3c0648f3ebed..c18b01fb9af79 100644 --- a/crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs +++ b/crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs @@ -7,6 +7,35 @@ use ruff_python_ast::visitor::Visitor; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for exceptions that are re-raised without specifying the cause via +/// the `from` keyword. +/// +/// ## Why is this bad? +/// The `from` keyword sets the `__cause__` attribute of the exception, which +/// stores the "cause" of the exception. The availability of an exception +/// "cause" is useful for debugging. +/// +/// ## Example +/// ```python +/// def reciprocal(n): +/// try: +/// return 1 / n +/// except ZeroDivisionError: +/// raise ValueError +/// ``` +/// +/// Use instead: +/// ```python +/// def reciprocal(n): +/// try: +/// return 1 / n +/// except ZeroDivisionError as exc: +/// raise ValueError from exc +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/exceptions.html#exception-context) #[violation] pub struct ReraiseNoCause; diff --git a/crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs b/crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs index d29d1d2b0019f..b3e3643879cd0 100644 --- a/crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs +++ b/crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs @@ -7,6 +7,45 @@ use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for `return` statements in `try` blocks. +/// +/// ## Why is this bad? +/// The `try`-`except` statement has an `else` clause for code that should +/// run _only_ if no exceptions were raised. Using the `else` clause is more +/// explicit than using a `return` statement inside of a `try` block. +/// +/// ## Example +/// ```python +/// import logging +/// +/// +/// def reciprocal(n): +/// try: +/// rec = 1 / n +/// print(f"reciprocal of {n} is {rec}") +/// return rec +/// except ZeroDivisionError as exc: +/// logging.exception("Exception occurred") +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// +/// def reciprocal(n): +/// try: +/// rec = 1 / n +/// except ZeroDivisionError as exc: +/// logging.exception("Exception occurred") +/// else: +/// print(f"reciprocal of {n} is {rec}") +/// return rec +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/tutorial/errors.html) #[violation] pub struct TryConsiderElse; diff --git a/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs b/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs index e4891262a6e00..c9e2213e4425a 100644 --- a/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs +++ b/crates/ruff/src/rules/tryceratops/rules/type_check_without_type_error.rs @@ -8,6 +8,33 @@ use ruff_python_ast::visitor::Visitor; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for type checks that do not raise `TypeError`. +/// +/// ## Why is this bad? +/// The Python documentation states that `TypeError` should be raised upon +/// encountering an inappropriate type. +/// +/// ## Example +/// ```python +/// def foo(n: int): +/// if isinstance(n, int): +/// pass +/// else: +/// raise ValueError("n must be an integer") +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(n: int): +/// if isinstance(n, int): +/// pass +/// else: +/// raise TypeError("n must be an integer") +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/exceptions.html#TypeError) #[violation] pub struct TypeCheckWithoutTypeError; diff --git a/crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs b/crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs index 1b0daf2a21383..4833feceb28d1 100644 --- a/crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs +++ b/crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs @@ -8,6 +8,30 @@ use ruff_python_ast::visitor::Visitor; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for needless exception names in `raise` statements. +/// +/// ## Why is this bad? +/// It's redundant to specify the exception name in a `raise` statement if the +/// exception is being re-raised. +/// +/// ## Example +/// ```python +/// def foo(): +/// try: +/// ... +/// except ValueError as exc: +/// raise exc +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// try: +/// ... +/// except ValueError: +/// raise +/// ``` #[violation] pub struct VerboseRaise;