Skip to content

Commit

Permalink
Broaden appropriate flake8-pyi rules to check non-stub code too (#6297)
Browse files Browse the repository at this point in the history
Of the rules that flake8-pyi enforces for `.pyi` type stubs, many of
them equally make sense to check in normal runtime code with type
annotations. Broaden these rules to check all files:

PYI013 ellipsis-in-non-empty-class-body
PYI016 duplicate-union-member
PYI018 unused-private-type-var
PYI019 custom-type-var-return-type
PYI024 collections-named-tuple
PYI025 unaliased-collections-abc-set-import
PYI030 unnecessary-literal-union
PYI032 any-eq-ne-annotation
PYI034 non-self-return-type
PYI036 bad-exit-annotation
PYI041 redundant-numeric-union
PYI042 snake-case-type-alias
PYI043 t-suffixed-type-alias
PYI045 iter-method-return-iterable
PYI046 unused-private-protocol
PYI047 unused-private-type-alias
PYI049 unused-private-typed-dict
PYI050 no-return-argument-annotation-in-stub (Python ≥ 3.11)
PYI051 redundant-literal-union
PYI056 unsupported-method-call-on-all

The other rules are stub-specific and remain enabled only in `.pyi`
files.

PYI001 unprefixed-type-param
PYI002 complex-if-statement-in-stub
PYI003 unrecognized-version-info-check
PYI004 patch-version-comparison
PYI005 wrong-tuple-length-version-comparison (could make sense to
broaden, see
#6297 (comment))
PYI006 bad-version-info-comparison (same)
PYI007 unrecognized-platform-check
PYI008 unrecognized-platform-name
PYI009 pass-statement-stub-body
PYI010 non-empty-stub-body
PYI011 typed-argument-default-in-stub
PYI012 pass-in-class-body
PYI014 argument-default-in-stub
PYI015 assignment-default-in-stub
PYI017 complex-assignment-in-stub
PYI020 quoted-annotation-in-stub
PYI021 docstring-in-stub
PYI026 type-alias-without-annotation (could make sense to broaden, but
gives many false positives on runtime code as currently implemented)
PYI029 str-or-repr-defined-in-stub
PYI033 type-comment-in-stub
PYI035 unassigned-special-variable-in-stub
PYI044 future-annotations-in-stub
PYI048 stub-body-multiple-statements
PYI052 unannotated-assignment-in-stub
PYI053 string-or-bytes-too-long
PYI054 numeric-literal-too-long

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
  • Loading branch information
andersk committed Aug 3, 2023
1 parent 30c2e94 commit 7c8bced
Show file tree
Hide file tree
Showing 30 changed files with 1,274 additions and 137 deletions.
10 changes: 5 additions & 5 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
_S2 = TypeVar("_S2", BadClass, GoodClass)

class BadClass:
def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # Ok
def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019


def bad_instance_method(self: _S, arg: bytes) -> _S: ... # Ok
def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019


@classmethod
def bad_class_method(cls: type[_S], arg: int) -> _S: ... # Ok
def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019


@classmethod
Expand All @@ -32,10 +32,10 @@ def static_method(arg1: _S) -> _S: ...

# Python > 3.12
class PEP695BadDunderNew[T]:
def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok
def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019


def generic_instance_method[S](self: S) -> S: ... # Ok
def generic_instance_method[S](self: S) -> S: ... # PYI019


class PEP695GoodDunderNew[T]:
Expand Down
8 changes: 5 additions & 3 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI024.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import collections

person: collections.namedtuple # OK
person: collections.namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple"

from collections import namedtuple

person: namedtuple # OK
person: namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple"

person = namedtuple("Person", ["name", "age"]) # OK
person = namedtuple(
"Person", ["name", "age"]
) # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple"
42 changes: 28 additions & 14 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI030.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,38 @@
import typing
import typing_extensions
from typing import Literal
# Shouldn't emit for any cases in the non-stub file for compatibility with flake8-pyi.
# Note that this rule could be applied here in the future.

# Shouldn't affect non-union field types.
field1: Literal[1] # OK
field2: Literal[1] | Literal[2] # OK

def func1(arg1: Literal[1] | Literal[2]): # OK
# Should emit for duplicate field types.
field2: Literal[1] | Literal[2] # Error

# Should emit for union types in arguments.
def func1(arg1: Literal[1] | Literal[2]): # Error
print(arg1)


def func2() -> Literal[1] | Literal[2]: # OK
# Should emit for unions in return types.
def func2() -> Literal[1] | Literal[2]: # Error
return "my Literal[1]ing"


field3: Literal[1] | Literal[2] | str # OK
field4: str | Literal[1] | Literal[2] # OK
field5: Literal[1] | str | Literal[2] # OK
field6: Literal[1] | bool | Literal[2] | str # OK
field7 = Literal[1] | Literal[2] # OK
field8: Literal[1] | (Literal[2] | str) # OK
field9: Literal[1] | (Literal[2] | str) # OK
field10: (Literal[1] | str) | Literal[2] # OK
field11: dict[Literal[1] | Literal[2], str] # OK
# Should emit in longer unions, even if not directly adjacent.
field3: Literal[1] | Literal[2] | str # Error
field4: str | Literal[1] | Literal[2] # Error
field5: Literal[1] | str | Literal[2] # Error
field6: Literal[1] | bool | Literal[2] | str # Error

# Should emit for non-type unions.
field7 = Literal[1] | Literal[2] # Error

# Should emit for parenthesized unions.
field8: Literal[1] | (Literal[2] | str) # Error

# Should handle user parentheses when fixing.
field9: Literal[1] | (Literal[2] | str) # Error
field10: (Literal[1] | str) | Literal[2] # Error

# Should emit for union in generic parent type.
field11: dict[Literal[1] | Literal[2], str] # Error
4 changes: 2 additions & 2 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI032.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@


class Bad:
def __eq__(self, other: Any) -> bool: ... # Fine because not a stub file
def __ne__(self, other: typing.Any) -> typing.Any: ... # Fine because not a stub file
def __eq__(self, other: Any) -> bool: ... # Y032
def __ne__(self, other: typing.Any) -> typing.Any: ... # Y032


class Good:
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI042.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@

just_literals_pipe_union: TypeAlias = (
Literal[True] | Literal["idk"]
) # not PYI042 (not a stubfile)
) # PYI042, since not camel case
PublicAliasT: TypeAlias = str | int
PublicAliasT2: TypeAlias = Union[str, bytes]
_ABCDEFGHIJKLMNOPQRST: TypeAlias = typing.Any
_PrivateAliasS: TypeAlias = Literal["I", "guess", "this", "is", "okay"]
_PrivateAliasS2: TypeAlias = Annotated[str, "also okay"]

snake_case_alias1: TypeAlias = str | int # not PYI042 (not a stubfile)
_snake_case_alias2: TypeAlias = Literal["whatever"] # not PYI042 (not a stubfile)
Snake_case_alias: TypeAlias = int | float # not PYI042 (not a stubfile)
snake_case_alias1: TypeAlias = str | int # PYI042, since not camel case
_snake_case_alias2: TypeAlias = Literal["whatever"] # PYI042, since not camel case
Snake_case_alias: TypeAlias = int | float # PYI042, since not camel case

# check that this edge case doesn't crash
_: TypeAlias = str | int
6 changes: 3 additions & 3 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI043.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
Literal,
)

_PrivateAliasT: TypeAlias = str | int # not PYI043 (not a stubfile)
_PrivateAliasT2: TypeAlias = typing.Any # not PYI043 (not a stubfile)
_PrivateAliasT: TypeAlias = str | int # PYI043, since this ends in a T
_PrivateAliasT2: TypeAlias = typing.Any # PYI043, since this ends in a T
_PrivateAliasT3: TypeAlias = Literal[
"not", "a", "chance"
] # not PYI043 (not a stubfile)
] # PYI043, since this ends in a T
just_literals_pipe_union: TypeAlias = Literal[True] | Literal["idk"]
PublicAliasT: TypeAlias = str | int
PublicAliasT2: TypeAlias = Union[str, bytes]
Expand Down
12 changes: 5 additions & 7 deletions crates/ruff/src/checkers/ast/analyze/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,11 @@ pub(crate) fn bindings(checker: &mut Checker) {
checker.diagnostics.push(diagnostic);
}
}
if checker.is_stub {
if checker.enabled(Rule::UnaliasedCollectionsAbcSetImport) {
if let Some(diagnostic) =
flake8_pyi::rules::unaliased_collections_abc_set_import(checker, binding)
{
checker.diagnostics.push(diagnostic);
}
if checker.enabled(Rule::UnaliasedCollectionsAbcSetImport) {
if let Some(diagnostic) =
flake8_pyi::rules::unaliased_collections_abc_set_import(checker, binding)
{
checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
24 changes: 11 additions & 13 deletions crates/ruff/src/checkers/ast/analyze/deferred_scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,17 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
}
}

if checker.is_stub {
if checker.enabled(Rule::UnusedPrivateTypeVar) {
flake8_pyi::rules::unused_private_type_var(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateProtocol) {
flake8_pyi::rules::unused_private_protocol(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateTypeAlias) {
flake8_pyi::rules::unused_private_type_alias(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateTypedDict) {
flake8_pyi::rules::unused_private_typed_dict(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateTypeVar) {
flake8_pyi::rules::unused_private_type_var(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateProtocol) {
flake8_pyi::rules::unused_private_protocol(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateTypeAlias) {
flake8_pyi::rules::unused_private_type_alias(checker, scope, &mut diagnostics);
}
if checker.enabled(Rule::UnusedPrivateTypedDict) {
flake8_pyi::rules::unused_private_typed_dict(checker, scope, &mut diagnostics);
}

if matches!(
Expand Down
8 changes: 5 additions & 3 deletions crates/ruff/src/checkers/ast/analyze/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub(crate) fn definitions(checker: &mut Checker) {
Rule::MissingTypeKwargs,
Rule::MissingTypeSelf,
]);
let enforce_stubs = checker.is_stub
&& checker.any_enabled(&[Rule::DocstringInStub, Rule::IterMethodReturnIterable]);
let enforce_stubs = checker.is_stub && checker.enabled(Rule::DocstringInStub);
let enforce_stubs_and_runtime = checker.enabled(Rule::IterMethodReturnIterable);
let enforce_docstrings = checker.any_enabled(&[
Rule::BlankLineAfterLastSection,
Rule::BlankLineAfterSummary,
Expand Down Expand Up @@ -81,7 +81,7 @@ pub(crate) fn definitions(checker: &mut Checker) {
Rule::UndocumentedPublicPackage,
]);

if !enforce_annotations && !enforce_docstrings && !enforce_stubs {
if !enforce_annotations && !enforce_docstrings && !enforce_stubs && !enforce_stubs_and_runtime {
return;
}

Expand Down Expand Up @@ -141,6 +141,8 @@ pub(crate) fn definitions(checker: &mut Checker) {
if checker.enabled(Rule::DocstringInStub) {
flake8_pyi::rules::docstring_in_stubs(checker, docstring);
}
}
if enforce_stubs_and_runtime {
if checker.enabled(Rule::IterMethodReturnIterable) {
flake8_pyi::rules::iter_method_return_iterable(checker, definition);
}
Expand Down
70 changes: 32 additions & 38 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::NumpyDeprecatedFunction) {
numpy::rules::deprecated_function(checker, expr);
}
if checker.is_stub {
if checker.enabled(Rule::CollectionsNamedTuple) {
flake8_pyi::rules::collections_named_tuple(checker, expr);
}
if checker.enabled(Rule::CollectionsNamedTuple) {
flake8_pyi::rules::collections_named_tuple(checker, expr);
}

// Ex) List[...]
Expand Down Expand Up @@ -323,10 +321,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::PrivateMemberAccess) {
flake8_self::rules::private_member_access(checker, expr);
}
if checker.is_stub {
if checker.enabled(Rule::CollectionsNamedTuple) {
flake8_pyi::rules::collections_named_tuple(checker, expr);
}
if checker.enabled(Rule::CollectionsNamedTuple) {
flake8_pyi::rules::collections_named_tuple(checker, expr);
}
pandas_vet::rules::attr(checker, attr, value, expr);
}
Expand Down Expand Up @@ -868,7 +864,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::DjangoLocalsInRenderFunction) {
flake8_django::rules::locals_in_render_function(checker, call);
}
if checker.is_stub && checker.enabled(Rule::UnsupportedMethodCallOnAll) {
if checker.enabled(Rule::UnsupportedMethodCallOnAll) {
flake8_pyi::rules::unsupported_method_call_on_all(checker, func);
}
}
Expand Down Expand Up @@ -1079,35 +1075,33 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
);
}
}
if checker.is_stub {
if checker.enabled(Rule::DuplicateUnionMember)
&& checker.semantic.in_type_definition()
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}
if checker.enabled(Rule::UnnecessaryLiteralUnion)
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
if checker.enabled(Rule::RedundantLiteralUnion)
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::redundant_literal_union(checker, expr);
}
if checker.enabled(Rule::DuplicateUnionMember)
&& checker.semantic.in_type_definition()
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}
if checker.enabled(Rule::UnnecessaryLiteralUnion)
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
if checker.enabled(Rule::RedundantLiteralUnion)
// Avoid duplicate checks if the parent is an `|`
&& !matches!(
checker.semantic.expr_parent(),
Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..}))
)
{
flake8_pyi::rules::redundant_literal_union(checker, expr);
}
}
Expr::UnaryOp(ast::ExprUnaryOp {
Expand Down
Loading

0 comments on commit 7c8bced

Please sign in to comment.