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.

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

[flake8-pyi] Implement PYI045 #4700

Merged
merged 5 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
Implement PYI045
  • Loading branch information
density committed May 29, 2023
commit b764acad57cb0597319b85b02a3e850b2b05b6bb
77 changes: 77 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import collections.abc
import typing
from collections.abc import Iterator, Iterable

# All OK for PYI045 because this isn't a stub.


class NoReturn:
def __iter__(self):
...


class TypingIterableTReturn:
def __iter__(self) -> typing.Iterable[int]:
...

def not_iter(self) -> typing.Iterable[int]:
...


class TypingIterableReturn:
def __iter__(self) -> typing.Iterable:
...

def not_iter(self) -> typing.Iterable:
...


class CollectionsIterableTReturn:
def __iter__(self) -> collections.abc.Iterable[int]:
...

def not_iter(self) -> collections.abc.Iterable[int]:
...


class CollectionsIterableReturn:
def __iter__(self) -> collections.abc.Iterable:
...

def not_iter(self) -> collections.abc.Iterable:
...


class IterableReturn:
def __iter__(self) -> Iterable:
...


class IteratorReturn:
def __iter__(self) -> Iterator:
...


class IteratorTReturn:
def __iter__(self) -> Iterator[int]:
...


class TypingIteratorReturn:
def __iter__(self) -> typing.Iterator:
...


class TypingIteratorTReturn:
def __iter__(self) -> typing.Iterator[int]:
...


class CollectionsIteratorReturn:
def __iter__(self) -> collections.abc.Iterator:
...


class CollectionsIteratorTReturn:
def __iter__(self) -> collections.abc.Iterator[int]:
...
43 changes: 43 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import collections.abc
import typing
from collections.abc import Iterator, Iterable

class NoReturn:
def __iter__(self): ...

class TypingIterableTReturn:
def __iter__(self) -> typing.Iterable[int]: ... # Error: PYI045
def not_iter(self) -> typing.Iterable[int]: ...

class TypingIterableReturn:
def __iter__(self) -> typing.Iterable: ... # Error: PYI045
def not_iter(self) -> typing.Iterable: ...

class CollectionsIterableTReturn:
def __iter__(self) -> collections.abc.Iterable[int]: ... # Error: PYI045
def not_iter(self) -> collections.abc.Iterable[int]: ...

class CollectionsIterableReturn:
def __iter__(self) -> collections.abc.Iterable: ... # Error: PYI045
def not_iter(self) -> collections.abc.Iterable: ...

class IterableReturn:
def __iter__(self) -> Iterable: ... # Error: PYI045

class IteratorReturn:
def __iter__(self) -> Iterator: ...

class IteratorTReturn:
def __iter__(self) -> Iterator[int]: ...

class TypingIteratorReturn:
def __iter__(self) -> typing.Iterator: ...

class TypingIteratorTReturn:
def __iter__(self) -> typing.Iterator[int]: ...

class CollectionsIteratorReturn:
def __iter__(self) -> collections.abc.Iterator: ...

class CollectionsIteratorTReturn:
def __iter__(self) -> collections.abc.Iterator[int]: ...
6 changes: 5 additions & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5321,7 +5321,8 @@ impl<'a> Checker<'a> {
Rule::MissingReturnTypeClassMethod,
Rule::AnyType,
]);
let enforce_stubs = self.is_stub && self.any_enabled(&[Rule::DocstringInStub]);
let enforce_stubs = self.is_stub
&& self.any_enabled(&[Rule::DocstringInStub, Rule::IterMethodReturnIterable]);
let enforce_docstrings = self.any_enabled(&[
Rule::UndocumentedPublicModule,
Rule::UndocumentedPublicClass,
Expand Down Expand Up @@ -5425,6 +5426,9 @@ impl<'a> Checker<'a> {
if self.enabled(Rule::DocstringInStub) {
flake8_pyi::rules::docstring_in_stubs(self, docstring);
}
if self.enabled(Rule::IterMethodReturnIterable) {
flake8_pyi::rules::iter_method_return_iterable(self, definition);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "033") => (RuleGroup::Unspecified, Rule::TypeCommentInStub),
(Flake8Pyi, "042") => (RuleGroup::Unspecified, Rule::SnakeCaseTypeAlias),
(Flake8Pyi, "043") => (RuleGroup::Unspecified, Rule::TSuffixedTypeAlias),
(Flake8Pyi, "045") => (RuleGroup::Unspecified, Rule::IterMethodReturnIterable),
(Flake8Pyi, "048") => (RuleGroup::Unspecified, Rule::StubBodyMultipleStatements),
(Flake8Pyi, "052") => (RuleGroup::Unspecified, Rule::UnannotatedAssignmentInStub),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ ruff_macros::register_rules!(
rules::flake8_pyi::rules::AssignmentDefaultInStub,
rules::flake8_pyi::rules::BadVersionInfoComparison,
rules::flake8_pyi::rules::DocstringInStub,
rules::flake8_pyi::rules::IterMethodReturnIterable,
rules::flake8_pyi::rules::DuplicateUnionMember,
rules::flake8_pyi::rules::EllipsisInNonEmptyClassBody,
rules::flake8_pyi::rules::NonEmptyStubBody,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ mod tests {
#[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))]
#[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.py"))]
#[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.pyi"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.py"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.pyi"))]
#[test_case(Rule::PassInClassBody, Path::new("PYI012.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use rustpython_parser::ast;
use rustpython_parser::ast::{Ranged, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::prelude::Expr;
use ruff_python_semantic::definition::{Definition, Member, MemberKind};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__iter__` methods in stubs with the wrong return type annotation.
///
/// ## Why is this bad?
/// `__iter__` should return an `Iterator`, not an `Iterable`.
///
/// ## Example
/// ```python
/// class Foo:
/// def __iter__(self) -> collections.abc.Iterable: ...
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __iter__(self) -> collections.abc.Iterator: ...
/// ```
#[violation]
pub struct IterMethodReturnIterable;

impl Violation for IterMethodReturnIterable {
#[derive_message_formats]
fn message(&self) -> String {
format!("__iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.")
}
}

/// PYI045
pub(crate) fn iter_method_return_iterable(checker: &mut Checker, definition: &Definition) {
let Definition::Member(Member {
kind: MemberKind::Method,
stmt,
..
}) = definition else {
return;
};

let Stmt::FunctionDef(ast::StmtFunctionDef {
name,
returns,
..
}) = stmt else {
return;
};

if name != "__iter__" {
return;
}

let Some(returns) = returns else {
return;
};

let annotation = match returns.as_ref() {
// e.g., Iterable[T]
Expr::Subscript(ast::ExprSubscript { value, .. }) => value.as_ref(),
// e.g., typing.Iterable, Iterable
ann_expr @ (Expr::Name(_) | Expr::Attribute(_)) => ann_expr,
_ => return,
};

if checker
.semantic_model()
.resolve_call_path(annotation)
.map_or(false, |cp| {
matches!(
cp.as_slice(),
&["typing", "Iterable"] | &["collections", "abc", "Iterable"]
)
})
{
checker
.diagnostics
.push(Diagnostic::new(IterMethodReturnIterable, returns.range()));
}
}
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ pub(crate) use duplicate_union_member::{duplicate_union_member, DuplicateUnionMe
pub(crate) use ellipsis_in_non_empty_class_body::{
ellipsis_in_non_empty_class_body, EllipsisInNonEmptyClassBody,
};
pub(crate) use iter_method_return_iterable::{
iter_method_return_iterable, IterMethodReturnIterable,
};
pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
pub(crate) use pass_in_class_body::{pass_in_class_body, PassInClassBody};
pub(crate) use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
Expand All @@ -31,6 +34,7 @@ mod bad_version_info_comparison;
mod docstring_in_stubs;
mod duplicate_union_member;
mod ellipsis_in_non_empty_class_body;
mod iter_method_return_iterable;
mod non_empty_stub_body;
mod pass_in_class_body;
mod pass_statement_stub_body;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI045.pyi:9:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.
|
9 | class TypingIterableTReturn:
10 | def __iter__(self) -> typing.Iterable[int]: ... # Error: PYI045
| ^^^^^^^^^^^^^^^^^^^^ PYI045
11 | def not_iter(self) -> typing.Iterable[int]: ...
|

PYI045.pyi:13:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.
|
13 | class TypingIterableReturn:
14 | def __iter__(self) -> typing.Iterable: ... # Error: PYI045
| ^^^^^^^^^^^^^^^ PYI045
15 | def not_iter(self) -> typing.Iterable: ...
|

PYI045.pyi:17:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.
|
17 | class CollectionsIterableTReturn:
18 | def __iter__(self) -> collections.abc.Iterable[int]: ... # Error: PYI045
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI045
19 | def not_iter(self) -> collections.abc.Iterable[int]: ...
|

PYI045.pyi:21:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.
|
21 | class CollectionsIterableReturn:
22 | def __iter__(self) -> collections.abc.Iterable: ... # Error: PYI045
| ^^^^^^^^^^^^^^^^^^^^^^^^ PYI045
23 | def not_iter(self) -> collections.abc.Iterable: ...
|

PYI045.pyi:25:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as they should always return some kind of `Iterator`.
|
25 | class IterableReturn:
26 | def __iter__(self) -> Iterable: ... # Error: PYI045
| ^^^^^^^^ PYI045
27 |
28 | class IteratorReturn:
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading