Add docs for tryceratops rules (#4042)

This commit is contained in:
Tom Kuson
2023-04-23 05:35:56 +01:00
committed by GitHub
parent 260138b427
commit f5cd659292
7 changed files with 225 additions and 0 deletions

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;