Spruce up docs for flake8-pyi rules (#10422)

This commit is contained in:
Alex Waygood 2024-03-18 18:03:32 +00:00 committed by GitHub
parent 162d2eb723
commit ae0ff9b029
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 173 additions and 115 deletions

View file

@ -8,28 +8,34 @@ use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for `__eq__` and `__ne__` implementations that use `typing.Any` as
/// the type annotation for the `obj` parameter.
/// the type annotation for their second parameter.
///
/// ## Why is this bad?
/// The Python documentation recommends the use of `object` to "indicate that a
/// value could be any type in a typesafe manner", while `Any` should be used to
/// "indicate that a value is dynamically typed."
/// value could be any type in a typesafe manner". `Any`, on the other hand,
/// should be seen as an "escape hatch when you need to mix dynamically and
/// statically typed code". Since using `Any` allows you to write highly unsafe
/// code, you should generally only use `Any` when the semantics of your code
/// would otherwise be inexpressible to the type checker.
///
/// The semantics of `__eq__` and `__ne__` are such that the `obj` parameter
/// should be any type, as opposed to a dynamically typed value. Therefore, the
/// `object` type annotation is more appropriate.
/// The expectation in Python is that a comparison of two arbitrary objects
/// using `==` or `!=` should never raise an exception. This contract can be
/// fully expressed in the type system and does not involve requesting unsound
/// behaviour from a type checker. As such, `object` is a more appropriate
/// annotation than `Any` for the second parameter of the methods implementing
/// these comparison operators -- `__eq__` and `__ne__`.
///
/// ## Example
/// ```python
/// class Foo:
/// def __eq__(self, obj: typing.Any):
/// def __eq__(self, obj: typing.Any) -> bool:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __eq__(self, obj: object):
/// def __eq__(self, obj: object) -> bool:
/// ...
/// ```
/// ## References

View file

@ -13,16 +13,17 @@ use crate::checkers::ast::Checker;
/// ## Why is this bad?
/// `typing.NamedTuple` is the "typed version" of `collections.namedtuple`.
///
/// The class generated by subclassing `typing.NamedTuple` is equivalent to
/// `collections.namedtuple`, with the exception that `typing.NamedTuple`
/// includes an `__annotations__` attribute, which allows type checkers to
/// infer the types of the fields.
/// Inheriting from `typing.NamedTuple` creates a custom `tuple` subclass in
/// the same way as using the `collections.namedtuple` factory function.
/// However, using `typing.NamedTuple` allows you to provide a type annotation
/// for each field in the class. This means that type checkers will have more
/// information to work with, and will be able to analyze your code more
/// precisely.
///
/// ## Example
/// ```python
/// from collections import namedtuple
///
///
/// person = namedtuple("Person", ["name", "age"])
/// ```
///

View file

@ -20,18 +20,28 @@ use crate::checkers::ast::Checker;
///
/// ## Example
/// ```python
/// from typing import TypeAlias
///
/// a = b = int
/// a.b = int
///
///
/// class Klass:
/// ...
///
///
/// Klass.X: TypeAlias = int
/// ```
///
/// Use instead:
/// ```python
/// from typing import TypeAlias
///
/// a: TypeAlias = int
/// b: TypeAlias = int
///
///
/// class a:
/// b: int
/// class Klass:
/// X: TypeAlias = int
/// ```
#[violation]
pub struct ComplexAssignmentInStub;

View file

@ -10,16 +10,16 @@ use crate::checkers::ast::Checker;
/// Checks for `if` statements with complex conditionals in stubs.
///
/// ## Why is this bad?
/// Stub files support simple conditionals to test for differences in Python
/// versions and platforms. However, type checkers only understand a limited
/// subset of these conditionals; complex conditionals may result in false
/// positives or false negatives.
/// Type checkers understand simple conditionals to express variations between
/// different Python versions and platforms. However, complex tests may not be
/// understood by a type checker, leading to incorrect inferences when they
/// analyze your code.
///
/// ## Example
/// ```python
/// import sys
///
/// if (2, 7) < sys.version_info < (3, 5):
/// if (3, 10) <= sys.version_info < (3, 12):
/// ...
/// ```
///
@ -27,9 +27,12 @@ use crate::checkers::ast::Checker;
/// ```python
/// import sys
///
/// if sys.version_info < (3, 5):
/// if sys.version_info >= (3, 10) and sys.version_info < (3, 12):
/// ...
/// ```
///
/// ## References
/// The [typing documentation on stub files](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct ComplexIfStatementInStub;

View file

@ -19,25 +19,32 @@ use crate::checkers::ast::Checker;
/// methods.
///
/// ## Why is this bad?
/// Improperly-annotated `__exit__` and `__aexit__` methods can cause
/// Improperly annotated `__exit__` and `__aexit__` methods can cause
/// unexpected behavior when interacting with type checkers.
///
/// ## Example
/// ```python
/// from types import TracebackType
///
///
/// class Foo:
/// def __exit__(self, typ, exc, tb, extra_arg) -> None:
/// def __exit__(
/// self, typ: BaseException, exc: BaseException, tb: TracebackType
/// ) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// from types import TracebackType
///
///
/// class Foo:
/// def __exit__(
/// self,
/// typ: type[BaseException] | None,
/// exc: BaseException | None,
/// tb: TracebackType | None,
/// extra_arg: int = 0,
/// ) -> None:
/// ...
/// ```

View file

@ -10,9 +10,10 @@ use crate::checkers::ast::Checker;
/// statement in stub files.
///
/// ## Why is this bad?
/// Stub files are already evaluated under `annotations` semantics. As such,
/// the `from __future__ import annotations` import statement has no effect
/// and should be omitted.
/// Stub files natively support forward references in all contexts, as stubs are
/// never executed at runtime. (They should be thought of as "data files" for
/// type checkers.) As such, the `from __future__ import annotations` import
/// statement has no effect and should be omitted.
///
/// ## References
/// - [Static Typing with Python: Type Stubs](https://typing.readthedocs.io/en/latest/source/stubs.html)

View file

@ -15,24 +15,46 @@ use crate::checkers::ast::Checker;
/// `__iter__` methods should always should return an `Iterator` of some kind,
/// not an `Iterable`.
///
/// In Python, an `Iterator` is an object that has a `__next__` method, which
/// provides a consistent interface for sequentially processing elements from
/// a sequence or other iterable object. Meanwhile, an `Iterable` is an object
/// with an `__iter__` method, which itself returns an `Iterator`.
/// In Python, an `Iterable` is an object that has an `__iter__` method; an
/// `Iterator` is an object that has `__iter__` and `__next__` methods. All
/// `__iter__` methods are expected to return `Iterator`s. Type checkers may
/// not always recognize an object as being iterable if its `__iter__` method
/// does not return an `Iterator`.
///
/// Every `Iterator` is an `Iterable`, but not every `Iterable` is an `Iterator`.
/// By returning an `Iterable` from `__iter__`, you may end up returning an
/// object that doesn't implement `__next__`, which will cause a `TypeError`
/// at runtime. For example, returning a `list` from `__iter__` will cause
/// a `TypeError` when you call `__next__` on it, as a `list` is an `Iterable`,
/// but not an `Iterator`.
/// For example, `list` is an `Iterable`, but not an `Iterator`; you can obtain
/// an iterator over a list's elements by passing the list to `iter()`:
///
/// ```pycon
/// >>> import collections.abc
/// >>> x = [42]
/// >>> isinstance(x, collections.abc.Iterable)
/// True
/// >>> isinstance(x, collections.abc.Iterator)
/// False
/// >>> next(x)
/// Traceback (most recent call last):
/// File "<stdin>", line 1, in <module>
/// TypeError: 'list' object is not an iterator
/// >>> y = iter(x)
/// >>> isinstance(y, collections.abc.Iterable)
/// True
/// >>> isinstance(y, collections.abc.Iterator)
/// True
/// >>> next(y)
/// 42
/// ```
///
/// Using `Iterable` rather than `Iterator` as a return type for an `__iter__`
/// methods would imply that you would not necessarily be able to call `next()`
/// on the returned object, violating the expectations of the interface.
///
/// ## Example
/// ```python
/// import collections.abc
///
///
/// class Class:
/// class Klass:
/// def __iter__(self) -> collections.abc.Iterable[str]:
/// ...
/// ```
@ -42,7 +64,7 @@ use crate::checkers::ast::Checker;
/// import collections.abc
///
///
/// class Class:
/// class Klass:
/// def __iter__(self) -> collections.abc.Iterator[str]:
/// ...
/// ```

View file

@ -9,19 +9,22 @@ use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion::Py311;
/// ## What it does
/// Checks for uses of `typing.NoReturn` (and `typing_extensions.NoReturn`) in
/// stubs.
/// Checks for uses of `typing.NoReturn` (and `typing_extensions.NoReturn`) for
/// parameter annotations.
///
/// ## Why is this bad?
/// Prefer `typing.Never` (or `typing_extensions.Never`) over `typing.NoReturn`,
/// as the former is more explicit about the intent of the annotation. This is
/// a purely stylistic choice, as the two are semantically equivalent.
/// Prefer `Never` over `NoReturn` for parameter annotations. `Never` has a
/// clearer name in these contexts, since it makes little sense to talk about a
/// parameter annotation "not returning".
///
/// This is a purely stylistic lint: the two types have identical semantics for
/// type checkers. Both represent Python's "[bottom type]" (a type that has no
/// members).
///
/// ## Example
/// ```python
/// from typing import NoReturn
///
///
/// def foo(x: NoReturn): ...
/// ```
///
@ -29,13 +32,14 @@ use crate::settings::types::PythonVersion::Py311;
/// ```python
/// from typing import Never
///
///
/// def foo(x: Never): ...
/// ```
///
/// ## References
/// - [Python documentation: `typing.Never`](https://docs.python.org/3/library/typing.html#typing.Never)
/// - [Python documentation: `typing.NoReturn`](https://docs.python.org/3/library/typing.html#typing.NoReturn)
///
/// [bottom type]: https://en.wikipedia.org/wiki/Bottom_type
#[violation]
pub struct NoReturnArgumentAnnotationInStub {
module: TypingModule,

View file

@ -10,9 +10,9 @@ use crate::checkers::ast::Checker;
/// Checks for non-empty function stub bodies.
///
/// ## Why is this bad?
/// Stub files are meant to be used as a reference for the interface of a
/// module, and should not contain any implementation details. Thus, the
/// body of a stub function should be empty.
/// Stub files are never executed at runtime; they should be thought of as
/// "data files" for type checkers or IDEs. Function bodies are redundant
/// for this purpose.
///
/// ## Example
/// ```python
@ -26,7 +26,8 @@ use crate::checkers::ast::Checker;
/// ```
///
/// ## References
/// - [PEP 484 Type Hints: Stub Files](https://www.python.org/dev/peps/pep-0484/#stub-files)
/// - [The recommended style for stub functions and methods](https://typing.readthedocs.io/en/latest/source/stubs.html#id6)
/// in the typing docs.
#[violation]
pub struct NonEmptyStubBody;

View file

@ -10,13 +10,13 @@ use ruff_python_semantic::{ScopeKind, SemanticModel};
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for methods that are annotated with a fixed return type, which
/// should instead be returning `self`.
/// Checks for methods that are annotated with a fixed return type which
/// should instead be returning `Self`.
///
/// ## Why is this bad?
/// If methods like `__new__` or `__enter__` are annotated with a fixed return
/// type, and the class is subclassed, type checkers will not be able to infer
/// the correct return type.
/// If methods that generally return `self` at runtime are annotated with a
/// fixed return type, and the class is subclassed, type checkers will not be
/// able to infer the correct return type.
///
/// For example:
/// ```python
@ -30,7 +30,7 @@ use crate::checkers::ast::Checker;
/// self.radius = radius
/// return self
///
/// # This returns `Shape`, not `Circle`.
/// # Type checker infers return type as `Shape`, not `Circle`.
/// Circle().set_scale(0.5)
///
/// # Thus, this expression is invalid, as `Shape` has no attribute `set_radius`.
@ -40,7 +40,7 @@ use crate::checkers::ast::Checker;
/// Specifically, this check enforces that the return type of the following
/// methods is `Self`:
///
/// 1. In-place binary operations, like `__iadd__`, `__imul__`, etc.
/// 1. In-place binary-operation dunder methods, like `__iadd__`, `__imul__`, etc.
/// 1. `__new__`, `__enter__`, and `__aenter__`, if those methods return the
/// class name.
/// 1. `__iter__` methods that return `Iterator`, despite the class inheriting
@ -51,16 +51,16 @@ use crate::checkers::ast::Checker;
/// ## Example
/// ```python
/// class Foo:
/// def __new__(cls, *args: Any, **kwargs: Any) -> Bad:
/// def __new__(cls, *args: Any, **kwargs: Any) -> Foo:
/// ...
///
/// def __enter__(self) -> Bad:
/// def __enter__(self) -> Foo:
/// ...
///
/// async def __aenter__(self) -> Bad:
/// async def __aenter__(self) -> Foo:
/// ...
///
/// def __iadd__(self, other: Bad) -> Bad:
/// def __iadd__(self, other: Foo) -> Foo:
/// ...
/// ```
///
@ -79,11 +79,11 @@ use crate::checkers::ast::Checker;
/// async def __aenter__(self) -> Self:
/// ...
///
/// def __iadd__(self, other: Bad) -> Self:
/// def __iadd__(self, other: Foo) -> Self:
/// ...
/// ```
/// ## References
/// - [PEP 673](https://peps.python.org/pep-0673/)
/// - [`typing.Self` documentation](https://docs.python.org/3/library/typing.html#typing.Self)
#[violation]
pub struct NonSelfReturnType {
class_name: String,

View file

@ -12,14 +12,15 @@ use crate::checkers::ast::Checker;
///
/// ## Why is this bad?
/// If a function has a default value where the literal representation is
/// greater than 50 characters, it is likely to be an implementation detail or
/// a constant that varies depending on the system you're running on.
/// greater than 50 characters, the value is likely to be an implementation
/// detail or a constant that varies depending on the system you're running on.
///
/// Consider replacing such constants with ellipses (`...`).
/// Default values like these should generally be omitted from stubs. Use
/// ellipses (`...`) instead.
///
/// ## Example
/// ```python
/// def foo(arg: int = 12345678901) -> None:
/// def foo(arg: int = 693568516352839939918568862861217771399698285293568) -> None:
/// ...
/// ```
///

View file

@ -7,31 +7,25 @@ use crate::checkers::ast::Checker;
use crate::fix;
/// ## What it does
/// Checks for the presence of the `pass` statement within a class body
/// in a stub (`.pyi`) file.
/// Checks for the presence of the `pass` statement in non-empty class bodies
/// in `.pyi` files.
///
/// ## Why is this bad?
/// In stub files, class definitions are intended to provide type hints, but
/// are never actually evaluated. As such, it's unnecessary to include a `pass`
/// statement in a class body, since it has no effect.
///
/// Instead of `pass`, prefer `...` to indicate that the class body is empty
/// and adhere to common stub file conventions.
/// The `pass` statement is always unnecessary in non-empty class bodies in
/// stubs.
///
/// ## Example
/// ```python
/// class MyClass:
/// x: int
/// pass
/// ```
///
/// Use instead:
/// ```python
/// class MyClass:
/// ...
/// x: int
/// ```
///
/// ## References
/// - [Mypy documentation: Stub files](https://mypy.readthedocs.io/en/stable/stubs.html)
#[violation]
pub struct PassInClassBody;

View file

@ -9,22 +9,22 @@ use crate::checkers::ast::Checker;
/// Checks for `pass` statements in empty stub bodies.
///
/// ## Why is this bad?
/// For consistency, empty stub bodies should contain `...` instead of `pass`.
///
/// Additionally, an ellipsis better conveys the intent of the stub body (that
/// the body has been implemented, but has been intentionally left blank to
/// document the interface).
/// For stylistic consistency, `...` should always be used rather than `pass`
/// in stub files.
///
/// ## Example
/// ```python
/// def foo(bar: int) -> list[int]:
/// pass
/// def foo(bar: int) -> list[int]: pass
/// ```
///
/// Use instead:
/// ```python
/// def foo(bar: int) -> list[int]: ...
/// ```
///
/// ## References
/// The [recommended style for functions and methods](https://typing.readthedocs.io/en/latest/source/stubs.html#functions-and-methods)
/// in the typing docs.
#[violation]
pub struct PassStatementStubBody;

View file

@ -25,12 +25,12 @@ impl fmt::Display for VarKind {
}
/// ## What it does
/// Checks that type `TypeVar`, `ParamSpec`, and `TypeVarTuple` definitions in
/// stubs are prefixed with `_`.
/// Checks that type `TypeVar`s, `ParamSpec`s, and `TypeVarTuple`s in stubs
/// have names prefixed with `_`.
///
/// ## Why is this bad?
/// By prefixing type parameters with `_`, we can avoid accidentally exposing
/// names internal to the stub.
/// Prefixing type parameters with `_` avoids accidentally exposing names
/// internal to the stub.
///
/// ## Example
/// ```python

View file

@ -9,10 +9,10 @@ use crate::checkers::ast::Checker;
/// Checks for quoted type annotations in stub (`.pyi`) files, which should be avoided.
///
/// ## Why is this bad?
/// Stub files are evaluated using `annotations` semantics, as if
/// `from __future__ import annotations` were included in the file. As such,
/// quotes are never required for type annotations in stub files, and should be
/// omitted.
/// Stub files natively support forward references in all contexts, as stubs
/// are never executed at runtime. (They should be thought of as "data files"
/// for type checkers and IDEs.) As such, quotes are never required for type
/// annotations in stub files, and should be omitted.
///
/// ## Example
/// ```python
@ -25,6 +25,9 @@ use crate::checkers::ast::Checker;
/// def function() -> int:
/// ...
/// ```
///
/// ## References
/// - [Static Typing with Python: Type Stubs](https://typing.readthedocs.io/en/latest/source/stubs.html)
#[violation]
pub struct QuotedAnnotationInStub;

View file

@ -13,30 +13,28 @@ use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
/// ## What it does
/// Checks for the presence of redundant `Literal` types and builtin super
/// types in an union.
/// Checks for redundant unions between a `Literal` and a builtin supertype of
/// that `Literal`.
///
/// ## Why is this bad?
/// The use of `Literal` types in a union with the builtin super type of one of
/// its literal members is redundant, as the super type is strictly more
/// general than the `Literal` type.
///
/// Using a `Literal` type in a union with its builtin supertype is redundant,
/// as the supertype will be strictly more general than the `Literal` type.
/// For example, `Literal["A"] | str` is equivalent to `str`, and
/// `Literal[1] | int` is equivalent to `int`, as `str` and `int` are the super
/// types of `"A"` and `1` respectively.
/// `Literal[1] | int` is equivalent to `int`, as `str` and `int` are the
/// supertypes of `"A"` and `1` respectively.
///
/// ## Example
/// ```python
/// from typing import Literal
///
/// A: Literal["A"] | str
/// x: Literal["A", b"B"] | str
/// ```
///
/// Use instead:
/// ```python
/// from typing import Literal
///
/// A: Literal["A"]
/// x: Literal[b"B"] | str
/// ```
#[violation]
pub struct RedundantLiteralUnion {

View file

@ -7,34 +7,41 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for union annotations that contain redundant numeric types (e.g.,
/// `int | float`).
/// Checks for parameter annotations that contain redundant unions between
/// builtin numeric types (e.g., `int | float`).
///
/// ## Why is this bad?
/// In Python, `int` is a subtype of `float`, and `float` is a subtype of
/// `complex`. As such, a union that includes both `int` and `float` is
/// redundant, as it is equivalent to a union that only includes `float`.
/// The [typing specification] states:
///
/// For more, see [PEP 3141], which defines Python's "numeric tower".
/// > Pythons numeric types `complex`, `float` and `int` are not subtypes of
/// > each other, but to support common use cases, the type system contains a
/// > straightforward shortcut: when an argument is annotated as having type
/// > `float`, an argument of type `int` is acceptable; similar, for an
/// > argument annotated as having type `complex`, arguments of type `float` or
/// > `int` are acceptable.
///
/// Unions with redundant elements are less readable than unions without them.
/// As such, a union that includes both `int` and `float` is redundant in the
/// specific context of a parameter annotation, as it is equivalent to a union
/// that only includes `float`. For readability and clarity, unions should omit
/// redundant elements.
///
/// ## Example
/// ```python
/// def foo(x: float | int) -> None:
/// def foo(x: float | int | str) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def foo(x: float) -> None:
/// def foo(x: float | str) -> None:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: The numeric tower](https://docs.python.org/3/library/numbers.html#the-numeric-tower)
/// - [The typing specification](https://docs.python.org/3/library/numbers.html#the-numeric-tower)
/// - [PEP 484: The numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower)
///
/// [PEP 3141]: https://peps.python.org/pep-3141/
/// [typing specification]: https://typing.readthedocs.io/en/latest/spec/special-types.html#special-cases-for-float-and-complex
#[violation]
pub struct RedundantNumericUnion {
redundancy: Redundancy,