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

Add docs for tryceratops rules #4042

Merged
merged 3 commits into from
Apr 23, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
34 changes: 34 additions & 0 deletions crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
37 changes: 37 additions & 0 deletions crates/ruff/src/rules/tryceratops/rules/raise_within_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
29 changes: 29 additions & 0 deletions crates/ruff/src/rules/tryceratops/rules/reraise_no_cause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
39 changes: 39 additions & 0 deletions crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
24 changes: 24 additions & 0 deletions crates/ruff/src/rules/tryceratops/rules/verbose_raise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading