Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broaden appropriate flake8-pyi rules to check non-stub code too #6297

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Broaden appropriate flake8-pyi rules to check non-stub code too #6297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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