Skip to content

Commit

Permalink
Make Y060 more aggressive (#432)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood authored Nov 6, 2023
1 parent 7840da6 commit 5bc395d
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 25 deletions.
2 changes: 1 addition & 1 deletion ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ The following warnings are currently emitted by default:
| Y057 | Do not use `typing.ByteString` or `collections.abc.ByteString`. These types have unclear semantics, and are deprecated; use `typing_extensions.Buffer` or a union such as `bytes \| bytearray \| memoryview` instead. See [PEP 688](https://peps.python.org/pep-0688/) for more details.
| Y058 | Use `Iterator` rather than `Generator` as the return value for simple `__iter__` methods, and `AsyncIterator` rather than `AsyncGenerator` as the return value for simple `__aiter__` methods. Using `(Async)Iterator` for these methods is simpler and more elegant, and reflects the fact that the precise kind of iterator returned from an `__iter__` method is usually an implementation detail that could change at any time, and should not be relied upon.
| Y059 | `Generic[]` should always be the last base class, if it is present in a class's bases tuple. At runtime, if `Generic[]` is not the final class in a the bases tuple, this [can cause the class creation to fail](https://github.com/python/cpython/issues/106102). In a stub file, however, this rule is enforced purely for stylistic consistency.
| Y060 | Redundant inheritance from `Generic[]`. For example, `class Foo(Iterable[_T], Generic[_T]): ...` can be written more simply as `class Foo(Iterable[_T]): ...`.<br><br>To avoid false-positive errors, and to avoid complexity in the implementation, this check is deliberately conservative: it only looks at classes that have exactly two bases.
| Y060 | Redundant inheritance from `Generic[]`. For example, `class Foo(Iterable[_T], Generic[_T]): ...` can be written more simply as `class Foo(Iterable[_T]): ...`.<br><br>To avoid false-positive errors, and to avoid complexity in the implementation, this check is deliberately conservative: it only flags classes where all subscripted bases have identical code inside their subscript slices.
| Y061 | Do not use `None` inside a `Literal[]` slice. For example, use `Literal["foo"] \| None` instead of `Literal["foo", None]`. While both are legal according to [PEP 586](https://peps.python.org/pep-0586/), the former is preferred for stylistic consistency.

## Warnings disabled by default
Expand Down
40 changes: 25 additions & 15 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from copy import deepcopy
from dataclasses import dataclass
from functools import partial
from itertools import chain, zip_longest
from itertools import chain, groupby, zip_longest
from keyword import iskeyword
from typing import TYPE_CHECKING, Any, ClassVar, NamedTuple, Union

Expand Down Expand Up @@ -62,6 +62,15 @@ class TypeVarInfo(NamedTuple):
name: str


def all_equal(iterable: Iterable[object]) -> bool:
"""Returns True if all the elements are equal to each other.
Adapted from the CPython itertools documentation."""
g = groupby(iterable)
next(g, True)
return not next(g, False)


_MAPPING_SLICE = "KeyType, ValueType"

# Y022: Use stdlib imports instead of aliases from typing/typing_extensions
Expand Down Expand Up @@ -1570,26 +1579,27 @@ def _check_class_bases(self, bases: list[ast.expr]) -> None:
Y040_encountered = False
Y059_encountered = False
Generic_basenode: ast.Subscript | None = None
subscript_bases: list[ast.Subscript] = []
num_bases = len(bases)

for i, base_node in enumerate(bases, start=1):
if not Y040_encountered and _is_builtins_object(base_node):
self.error(base_node, Y040)
Y040_encountered = True
if isinstance(base_node, ast.Subscript) and _is_Generic(base_node.value):
Generic_basenode = base_node
if i < num_bases and not Y059_encountered:
Y059_encountered = True
self.error(base_node, Y059)

if (
Generic_basenode is not None
and num_bases == 2
and isinstance(bases[0], ast.Subscript)
and isinstance(bases[1], ast.Subscript)
and ast.dump(bases[0].slice) == ast.dump(bases[1].slice)
):
self.error(Generic_basenode, Y060)
if isinstance(base_node, ast.Subscript):
subscript_bases.append(base_node)
if _is_Generic(base_node.value):
Generic_basenode = base_node
if i < num_bases and not Y059_encountered:
Y059_encountered = True
self.error(base_node, Y059)

if Generic_basenode is not None:
assert subscript_bases
if len(subscript_bases) > 1 and all_equal(
ast.dump(subscript_base.slice) for subscript_base in subscript_bases
):
self.error(Generic_basenode, Y060)

def visit_ClassDef(self, node: ast.ClassDef) -> None:
if node.name.startswith("_") and not self.in_class.active:
Expand Down
36 changes: 27 additions & 9 deletions tests/classdefs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ from collections.abc import (
Generator,
Iterable,
Iterator,
Mapping,
)
from enum import EnumMeta
from typing import Any, Generic, TypeVar, overload
Expand Down Expand Up @@ -190,22 +191,39 @@ def __imul__(self, other: Any) -> list[str]: ...
_S = TypeVar("_S")
_T = TypeVar("_T")

class BadGeneric(Generic[_T], int): ... # Y059 "Generic[]" should always be the last base class
class GoodGeneric(Generic[_T]): ...
class GoodGeneric2(int, typing.Generic[_T]): ...
class BreaksAtRuntimeButWhatever(int, str, typing_extensions.Generic[_T]): ...
class DoesntTypeCheckButWhatever(Iterable[int], str, Generic[_T]): ...

class BadGeneric(Generic[_T], int): ... # Y059 "Generic[]" should always be the last base class
class BadGeneric2(int, typing.Generic[_T], str): ... # Y059 "Generic[]" should always be the last base class
class GoodGeneric2(int, typing.Generic[_T]): ...

class BadGeneric3(typing_extensions.Generic[_T], int, str): ... # Y059 "Generic[]" should always be the last base class
class GoodGeneric3(int, str, typing_extensions.Generic[_T]): ...

class BadGeneric4(Generic[_T], Iterable[int], str): ... # Y059 "Generic[]" should always be the last base class
class GoodGeneric4(Iterable[int], str, Generic[_T]): ...

class RedundantGeneric1(Iterable[_T], Generic[_T]): ... # Y060 Redundant inheritance from "Generic[]"; class would be inferred as generic anyway
class Corrected1(Iterable[_T]): ...

class RedundantGeneric2(Generic[_S], GoodGeneric[_S]): ... # Y059 "Generic[]" should always be the last base class # Y060 Redundant inheritance from "Generic[]"; class would be inferred as generic anyway
class Corrected2(GoodGeneric[_S]): ...

class RedundantGeneric3(int, Iterator[_T], str, float, memoryview, bytes, Generic[_T]): ... # Y060 Redundant inheritance from "Generic[]"; class would be inferred as generic anyway
class Corrected3(int, Iterator[_T], str, float, memoryview, bytes): ...

class RedundantGeneric4(Iterable[_T], Iterator[_T], Generic[_T]): ... # Y060 Redundant inheritance from "Generic[]"; class would be inferred as generic anyway
class Corrected4(Iterable[_T], Iterator[_T]): ...

# Strictly speaking these inheritances from Generic are "redundant",
class BadAndRedundantGeneric(object, Generic[_T], Container[_T]): ... # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 # Y059 "Generic[]" should always be the last base class # Y060 Redundant inheritance from "Generic[]"; class would be inferred as generic anyway
class Corrected5(Container[_T]): ...

# Strictly speaking this inheritance from Generic is "redundant",
# but people may consider it more readable to explicitly inherit from Generic,
# so we deliberately don't flag them with Y060
class GoodGeneric3(Container[_S], Iterator[_T], Generic[_S, _T]): ...
class GoodGeneric4(int, Iterator[_T], str, Generic[_T]): ...
class BadGeneric5(object, Generic[_T], Container[_T]): ... # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 # Y059 "Generic[]" should always be the last base class
# so we deliberately don't flag it with Y060
class GoodGeneric5(Container[_S], Iterator[_T], Generic[_S, _T]): ...

# And these definitely arent't redundant,
# since the order of the type variables is changed via the inheritance from Generic:
class GoodGeneric6(Container[_S], Iterator[_T], Generic[_T, _S]): ...
class GoodGeneric7(Mapping[_S, _T], Generic[_T, _S]): ...

0 comments on commit 5bc395d

Please sign in to comment.