From ae0ff9b0295e72bcbd9a7fc126afbcaab2bbcc4e Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 18 Mar 2024 18:03:32 +0000 Subject: [PATCH] Spruce up docs for flake8-pyi rules (#10422) --- .../flake8_pyi/rules/any_eq_ne_annotation.rs | 22 ++++++---- .../rules/collections_named_tuple.rs | 11 ++--- .../rules/complex_assignment_in_stub.rs | 16 +++++-- .../rules/complex_if_statement_in_stub.rs | 15 ++++--- .../flake8_pyi/rules/exit_annotations.rs | 13 ++++-- .../rules/future_annotations_in_stub.rs | 7 +-- .../rules/iter_method_return_iterable.rs | 44 ++++++++++++++----- .../rules/no_return_argument_annotation.rs | 18 +++++--- .../flake8_pyi/rules/non_empty_stub_body.rs | 9 ++-- .../flake8_pyi/rules/non_self_return_type.rs | 26 +++++------ .../rules/numeric_literal_too_long.rs | 9 ++-- .../flake8_pyi/rules/pass_in_class_body.rs | 18 +++----- .../rules/pass_statement_stub_body.rs | 14 +++--- .../flake8_pyi/rules/prefix_type_params.rs | 8 ++-- .../rules/quoted_annotation_in_stub.rs | 11 +++-- .../rules/redundant_literal_union.rs | 18 ++++---- .../rules/redundant_numeric_union.rs | 29 +++++++----- 17 files changed, 173 insertions(+), 115 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs index a193732226..ebc52fcfc4 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/collections_named_tuple.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/collections_named_tuple.rs index ea873ebf95..e0939087f3 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/collections_named_tuple.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/collections_named_tuple.rs @@ -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"]) /// ``` /// diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_assignment_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_assignment_in_stub.rs index 122ec0b44a..7111b1212f 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_assignment_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_assignment_in_stub.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_if_statement_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_if_statement_in_stub.rs index 4c5471e7c6..6a8fe34dfa 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_if_statement_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/complex_if_statement_in_stub.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs index 19af0bd6f5..3d27a44853 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs @@ -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: /// ... /// ``` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs index 41ea12a377..9d02dedb3d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs @@ -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) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs index aec883d862..017b3947a8 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs @@ -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 "", line 1, in +/// 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]: /// ... /// ``` diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs index 01350b9501..a09872435c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs @@ -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, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs index 2eae1f7b75..14d117791b 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs index c946eba080..739d4ac611 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_self_return_type.rs @@ -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, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs index 64092f3035..97759b3f77 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs @@ -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: /// ... /// ``` /// diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs index 1e26b76c6e..b6c2fbc365 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs index 3e1a0d3a8d..940b6afca3 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/prefix_type_params.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/prefix_type_params.rs index e4c237c3cb..1770ecf2eb 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/prefix_type_params.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/prefix_type_params.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/quoted_annotation_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/quoted_annotation_in_stub.rs index 96330f7573..f79a0103ca 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/quoted_annotation_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/quoted_annotation_in_stub.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs index 57f08abb05..56808290f4 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs @@ -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 { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs index 672e4c40f4..746fd8c1fe 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -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". +/// > Python’s 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,