Skip to content

Commit

Permalink
Spruce up docs for flake8-pyi rules (part 2) (#10494)
Browse files Browse the repository at this point in the history
- Improve clarity over the motivation for some rules
- Improve links to external references. In particular, reduce links to PEPs, as PEPs are generally historical documents rather than pieces of living documentation. Where possible, it's better to link to the official typing spec, the other docs at typing.readthedocs.io/en/latest, or the docs at docs.python.org/3/library/typing.html.
- Use more concise language in a few places
  • Loading branch information
AlexWaygood authored Mar 21, 2024
1 parent d9ac170 commit ac150b9
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 38 deletions.
26 changes: 12 additions & 14 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,30 @@ use crate::rules::flake8_pyi::rules::TypingModule;
use crate::settings::types::PythonVersion;

/// ## What it does
/// Checks for typed function arguments in stubs with default values that
/// are not "simple" /// (i.e., `int`, `float`, `complex`, `bytes`, `str`,
/// `bool`, `None`, `...`, or simple container literals).
/// Checks for typed function arguments in stubs with complex default values.
///
/// ## Why is this bad?
/// Stub (`.pyi`) files exist to define type hints, and are not evaluated at
/// runtime. As such, function arguments in stub files should not have default
/// values, as they are ignored by type checkers.
///
/// However, the use of default values may be useful for IDEs and other
/// consumers of stub files, and so "simple" values may be worth including and
/// are permitted by this rule.
/// Stub (`.pyi`) files exist as "data files" for static analysis tools, and
/// are not evaluated at runtime. While simple default values may be useful for
/// some tools that consume stubs, such as IDEs, they are ignored by type
/// checkers.
///
/// Instead of including and reproducing a complex value, use `...` to indicate
/// that the assignment has a default value, but that the value is non-simple
/// or varies according to the current platform or Python version.
/// that the assignment has a default value, but that the value is "complex" or
/// varies according to the current platform or Python version. For the
/// purposes of this rule, any default value counts as "complex" unless it is
/// a literal `int`, `float`, `complex`, `bytes`, `str`, `bool`, `None`, `...`,
/// or a simple container literal.
///
/// ## Example
/// ```python
/// def foo(arg: List[int] = []) -> None:
/// def foo(arg: list[int] = list(range(10_000))) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def foo(arg: List[int] = ...) -> None:
/// def foo(arg: list[int] = ...) -> None:
/// ...
/// ```
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ use crate::checkers::ast::Checker;
/// in stub (`.pyi`) files.
///
/// ## Why is this bad?
/// If a function has a default value where the string or bytes 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.
/// If a function or variable has a default value where the string or bytes
/// representation is greater than 50 characters long, it 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 (`...`).
/// Although IDEs may find them useful, default values are ignored by type
/// checkers, the primary consumers of stub files. Replace very long constants
/// with ellipses (`...`) to simplify the stub.
///
/// ## Example
/// ```python
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ impl Violation for SnakeCaseTypeAlias {
///
/// _MyType: TypeAlias = int
/// ```
///
/// ## References
/// - [PEP 484: Type Aliases](https://peps.python.org/pep-0484/#type-aliases)
#[violation]
pub struct TSuffixedTypeAlias {
name: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,25 @@ use crate::checkers::ast::Checker;
/// Checks for the presence of multiple literal types in a union.
///
/// ## Why is this bad?
/// Literal types accept multiple arguments, and it is clearer to specify them
/// as a single literal.
/// `Literal["foo", 42]` has identical semantics to
/// `Literal["foo"] | Literal[42]`, but is clearer and more concise.
///
/// ## Example
/// ```python
/// from typing import Literal
///
/// field: Literal[1] | Literal[2]
/// field: Literal[1] | Literal[2] | str
/// ```
///
/// Use instead:
/// ```python
/// from typing import Literal
///
/// field: Literal[1, 2]
/// field: Literal[1, 2] | str
/// ```
///
/// ## References
/// - [Python documentation: `typing.Literal`](https://docs.python.org/3/library/typing.html#typing.Literal)
#[violation]
pub struct UnnecessaryLiteralUnion {
members: Vec<String>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ use crate::checkers::ast::Checker;
/// Checks for the presence of multiple `type`s in a union.
///
/// ## Why is this bad?
/// The `type` built-in function accepts unions, and it is clearer to
/// explicitly specify them as a single `type`.
/// `type[T | S]` has identical semantics to `type[T] | type[S]` in a type
/// annotation, but is cleaner and more concise.
///
/// ## Example
/// ```python
/// field: type[int] | type[float]
/// field: type[int] | type[float] | str
/// ```
///
/// Use instead:
/// ```python
/// field: type[int | float]
/// field: type[int | float] | str
/// ```
#[violation]
pub struct UnnecessaryTypeUnion {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use crate::registry::Rule;
///
/// ## Why is this bad?
/// Some `sys.platform` checks are too complex for type checkers to
/// understand, and thus result in false positives. `sys.platform` checks
/// should be simple string comparisons, like `sys.platform == "linux"`.
/// understand, and thus result in incorrect inferences by these tools.
/// `sys.platform` checks should be simple string comparisons, like
/// `if sys.platform == "linux"`.
///
/// ## Example
/// ```python
Expand All @@ -39,7 +40,7 @@ use crate::registry::Rule;
/// ```
///
/// ## References
/// - [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
/// - [Typing stubs documentation: Version and Platform Checks](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct UnrecognizedPlatformCheck;

Expand Down Expand Up @@ -75,7 +76,7 @@ impl Violation for UnrecognizedPlatformCheck {
/// ```
///
/// ## References
/// - [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
/// - [Typing stubs documentation: Version and Platform Checks](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct UnrecognizedPlatformName {
platform: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ use crate::registry::Rule;
/// if sys.version_info[0] == 2:
/// ...
/// ```
///
/// ## References
/// - [Typing stubs documentation: Version and Platform Checks](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct UnrecognizedVersionInfoCheck;

Expand Down Expand Up @@ -69,6 +72,9 @@ impl Violation for UnrecognizedVersionInfoCheck {
/// if sys.version_info >= (3, 4):
/// ...
/// ```
///
/// ## References
/// - [Typing stubs documentation: Version and Platform Checks](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct PatchVersionComparison;

Expand Down Expand Up @@ -104,6 +110,9 @@ impl Violation for PatchVersionComparison {
/// if sys.version_info[0] == 3:
/// ...
/// ```
///
/// ## References
/// - [Typing stubs documentation: Version and Platform Checks](https://typing.readthedocs.io/en/latest/source/stubs.html#version-and-platform-checks)
#[violation]
pub struct WrongTupleLengthVersionComparison {
expected_length: usize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,28 @@ use crate::checkers::ast::Checker;
///
/// ## Example
/// ```python
/// __all__ = ["A"]
/// __all__.append("B")
/// import sys
///
/// __all__ = ["A", "B"]
///
/// if sys.version_info >= (3, 10):
/// __all__.append("C")
///
/// if sys.version_info >= (3, 11):
/// __all__.remove("B")
/// ```
///
/// Use instead:
/// ```python
/// import sys
///
/// __all__ = ["A"]
/// __all__ += ["B"]
///
/// if sys.version_info < (3, 11):
/// __all__ += ["B"]
///
/// if sys.version_info >= (3, 10):
/// __all__ += ["C"]
/// ```
#[violation]
pub struct UnsupportedMethodCallOnAll {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Violation for UnusedPrivateTypeVar {
///
/// ## Why is this bad?
/// A private `typing.Protocol` that is defined but not used is likely a
/// mistake, and should either be used, made public, or removed to avoid
/// mistake. It should either be used, made public, or removed to avoid
/// confusion.
///
/// ## Example
Expand Down Expand Up @@ -83,11 +83,11 @@ impl Violation for UnusedPrivateProtocol {
}

/// ## What it does
/// Checks for the presence of unused private `typing.TypeAlias` definitions.
/// Checks for the presence of unused private type aliases.
///
/// ## Why is this bad?
/// A private `typing.TypeAlias` that is defined but not used is likely a
/// mistake, and should either be used, made public, or removed to avoid
/// A private type alias that is defined but not used is likely a
/// mistake. It should either be used, made public, or removed to avoid
/// confusion.
///
/// ## Example
Expand Down Expand Up @@ -125,7 +125,7 @@ impl Violation for UnusedPrivateTypeAlias {
///
/// ## Why is this bad?
/// A private `typing.TypedDict` that is defined but not used is likely a
/// mistake, and should either be used, made public, or removed to avoid
/// mistake. It should either be used, made public, or removed to avoid
/// confusion.
///
/// ## Example
Expand Down

0 comments on commit ac150b9

Please sign in to comment.