Skip to content

Commit

Permalink
Complete flake8-bugbear documentation (#5178)
Browse files Browse the repository at this point in the history
## Summary

Completes the documentation for the `flake8-bugbear` ruleset. Related to
#2646.

## Test Plan

`python scripts/check_docs_formatted.py`

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
  • Loading branch information
tjkuson and charliermarsh authored Jun 20, 2023
1 parent 7bc33a8 commit 6929fcc
Show file tree
Hide file tree
Showing 27 changed files with 803 additions and 55 deletions.
68 changes: 68 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/rules/abstract_base_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,40 @@ use ruff_python_semantic::SemanticModel;
use crate::checkers::ast::Checker;
use crate::registry::Rule;

/// ## What it does
/// Checks for abstract classes without abstract methods.
///
/// ## Why is this bad?
/// Abstract base classes are used to define interfaces. If they have no abstract
/// methods, they are not useful.
///
/// If the class is not meant to be used as an interface, it should not be an
/// abstract base class. Remove the `ABC` base class from the class definition,
/// or add an abstract method to the class.
///
/// ## Example
/// ```python
/// from abc import ABC
///
///
/// class Foo(ABC):
/// def method(self):
/// bar()
/// ```
///
/// Use instead:
/// ```python
/// from abc import ABC, abstractmethod
///
///
/// class Foo(ABC):
/// @abstractmethod
/// def method(self):
/// bar()
/// ```
///
/// ## References
/// - [Python documentation: `abc`](https://docs.python.org/3/library/abc.html)
#[violation]
pub struct AbstractBaseClassWithoutAbstractMethod {
name: String,
Expand All @@ -21,6 +55,40 @@ impl Violation for AbstractBaseClassWithoutAbstractMethod {
format!("`{name}` is an abstract base class, but it has no abstract methods")
}
}
/// ## What it does
/// Checks for empty methods in abstract base classes without an abstract
/// decorator.
///
/// ## Why is this bad?
/// Empty methods in abstract base classes without an abstract decorator are
/// indicative of unfinished code or a mistake.
///
/// Instead, add an abstract method decorated to indicate that it is abstract,
/// or implement the method.
///
/// ## Example
/// ```python
/// from abc import ABC
///
///
/// class Foo(ABC):
/// def method(self):
/// ...
/// ```
///
/// Use instead:
/// ```python
/// from abc import ABC, abstractmethod
///
///
/// class Foo(ABC):
/// @abstractmethod
/// def method(self):
/// ...
/// ```
///
/// ## References
/// - [Python documentation: abc](https://docs.python.org/3/library/abc.html)
#[violation]
pub struct EmptyMethodWithoutAbstractDecorator {
name: String,
Expand Down
22 changes: 22 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/rules/assert_false.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,28 @@ use ruff_python_ast::helpers::is_const_false;
use crate::checkers::ast::Checker;
use crate::registry::AsRule;

/// ## What it does
/// Checks for uses of `assert False`.
///
/// ## Why is this bad?
/// Python removes `assert` statements when running in optimized mode
/// (`python -O`), making `assert False` an unreliable means of
/// raising an `AssertionError`.
///
/// Instead, raise an `AssertionError` directly.
///
/// ## Example
/// ```python
/// assert False
/// ```
///
/// Use instead:
/// ```python
/// raise AssertionError
/// ```
///
/// ## References
/// - [Python documentation: `assert`](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement)
#[violation]
pub struct AssertFalse;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,39 @@ use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for assignments to `os.environ`.
///
/// ## Why is this bad?
/// In Python, `os.environ` is a mapping that represents the environment of the
/// current process.
///
/// However, reassigning to `os.environ` does not clear the environment. Instead,
/// it merely updates the `os.environ` for the current process. This can lead to
/// unexpected behavior, especially when running the program in a subprocess.
///
/// Instead, use `os.environ.clear()` to clear the environment, or use the
/// `env` argument of `subprocess.Popen` to pass a custom environment to
/// a subprocess.
///
/// ## Example
/// ```python
/// import os
///
/// os.environ = {"foo": "bar"}
/// ```
///
/// Use instead:
/// ```python
/// import os
///
/// os.environ.clear()
/// os.environ["foo"] = "bar"
/// ```
///
/// ## References
/// - [Python documentation: `os.environ`](https://docs.python.org/3/library/os.html#os.environ)
/// - [Python documentation: `subprocess.Popen`](https://docs.python.org/3/library/subprocess.html#subprocess.Popen)
#[violation]
pub struct AssignmentToOsEnviron;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,57 @@ use ruff_python_semantic::SemanticModel;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of the `functools.lru_cache` and `functools.cache`
/// decorators on methods.
///
/// ## Why is this bad?
/// Using the `functools.lru_cache` and `functools.cache` decorators on methods
/// can lead to memory leaks, as the global cache will retain a reference to
/// the instance, preventing it from being garbage collected.
///
/// Instead, refactor the method to depend only on its arguments and not on the
/// instance of the class, or use the `@lru_cache` decorator on a function
/// outside of the class.
///
/// ## Example
/// ```python
/// from functools import lru_cache
///
///
/// def square(x: int) -> int:
/// return x * x
///
///
/// class Number:
/// value: int
///
/// @lru_cache
/// def squared(self):
/// return square(self.value)
/// ```
///
/// Use instead:
/// ```python
/// from functools import lru_cache
///
///
/// @lru_cache
/// def square(x: int) -> int:
/// return x * x
///
///
/// class Number:
/// value: int
///
/// def squared(self):
/// return square(self.value)
/// ```
///
/// ## References
/// - [Python documentation: `functools.lru_cache`](https://docs.python.org/3/library/functools.html#functools.lru_cache)
/// - [Python documentation: `functools.cache`](https://docs.python.org/3/library/functools.html#functools.cache)
/// - [don't lru_cache methods!](https://www.youtube.com/watch?v=sVjtp6tGo0g)
#[violation]
pub struct CachedInstanceMethod;

Expand Down
57 changes: 57 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,33 @@ use ruff_python_ast::call_path::CallPath;
use crate::checkers::ast::Checker;
use crate::registry::{AsRule, Rule};

/// ## What it does
/// Checks for `try-except` blocks with duplicate exception handlers.
///
/// ## Why is this bad?
/// Duplicate exception handlers are redundant, as the first handler will catch
/// the exception, making the second handler unreachable.
///
/// ## Example
/// ```python
/// try:
/// ...
/// except ValueError:
/// ...
/// except ValueError:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// try:
/// ...
/// except ValueError:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
#[violation]
pub struct DuplicateTryBlockException {
name: String,
Expand All @@ -24,6 +51,36 @@ impl Violation for DuplicateTryBlockException {
format!("try-except block with duplicate exception `{name}`")
}
}

/// ## What it does
/// Checks for exception handlers that catch duplicate exceptions.
///
/// ## Why is this bad?
/// Including the same exception multiple times in the same handler is redundant,
/// as the first exception will catch the exception, making the second exception
/// unreachable. The same applies to exception hierarchies, as a handler for a
/// parent exception (like `Exception`) will also catch child exceptions (like
/// `ValueError`).
///
/// ## Example
/// ```python
/// try:
/// ...
/// except (Exception, ValueError): # `Exception` includes `ValueError`.
/// ...
/// ```
///
/// Use instead:
/// ```python
/// try:
/// ...
/// except Exception:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
/// - [Python documentation: Exception hierarchy](https://docs.python.org/3/library/exceptions.html#exception-hierarchy)
#[violation]
pub struct DuplicateHandlerException {
pub names: Vec<String>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,32 @@ use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for exception handlers that catch an empty tuple.
///
/// ## Why is this bad?
/// An exception handler that catches an empty tuple will not catch anything,
/// and is indicative of a mistake. Instead, add exceptions to the `except`
/// clause.
///
/// ## Example
/// ```python
/// try:
/// 1 / 0
/// except ():
/// ...
/// ```
///
/// Use instead:
/// ```python
/// try:
/// 1 / 0
/// except ZeroDivisionError:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
#[violation]
pub struct ExceptWithEmptyTuple;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,32 @@ use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for exception handlers that catch non-exception classes.
///
/// ## Why is this bad?
/// Catching classes that do not inherit from `BaseException` will raise a
/// `TypeError`.
///
/// ## Example
/// ```python
/// try:
/// 1 / 0
/// except 1:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// try:
/// 1 / 0
/// except ZeroDivisionError:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: `except` clause](https://docs.python.org/3/reference/compound_stmts.html#except-clause)
/// - [Python documentation: Built-in Exceptions](https://docs.python.org/3/library/exceptions.html#built-in-exceptions)
#[violation]
pub struct ExceptWithNonExceptionClasses;

Expand Down
24 changes: 24 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/rules/f_string_docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,30 @@ use ruff_python_ast::identifier::Identifier;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for docstrings that are written via f-strings.
///
/// ## Why is this bad?
/// Python will interpret the f-string as a joined string, rather than as a
/// docstring. As such, the "docstring" will not be accessible via the
/// `__doc__` attribute, nor will it be picked up by any automated
/// documentation tooling.
///
/// ## Example
/// ```python
/// def foo():
/// f"""Not a docstring."""
/// ```
///
/// Use instead:
/// ```python
/// def foo():
/// """A docstring."""
/// ```
///
/// ## References
/// - [PEP 257](https://peps.python.org/pep-0257/)
/// - [Python documentation: Formatted string literals](https://docs.python.org/3/reference/lexical_analysis.html#f-strings)
#[violation]
pub struct FStringDocstring;

Expand Down
Loading

0 comments on commit 6929fcc

Please sign in to comment.