From b764acad57cb0597319b85b02a3e850b2b05b6bb Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Mon, 29 May 2023 00:33:58 -0400 Subject: [PATCH 1/4] Implement PYI045 --- .../test/fixtures/flake8_pyi/PYI045.py | 77 +++++++++++++++++ .../test/fixtures/flake8_pyi/PYI045.pyi | 43 ++++++++++ crates/ruff/src/checkers/ast/mod.rs | 6 +- crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + .../rules/iter_method_return_iterable.rs | 86 +++++++++++++++++++ crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 4 + ...__flake8_pyi__tests__PYI045_PYI045.py.snap | 4 + ..._flake8_pyi__tests__PYI045_PYI045.pyi.snap | 45 ++++++++++ ruff.schema.json | 1 + 11 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py new file mode 100644 index 0000000000000..5ea53df34479f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py @@ -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]: + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi new file mode 100644 index 0000000000000..1e26c7192eaa9 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi @@ -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]: ... diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 1028f33de4b18..d7e97573ee25d 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -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, @@ -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); + } } } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 2b55ab6d0e36b..54e516ac5b5d7 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -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), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 1f975cff38c88..252b7b8f6e54f 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -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, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index ad62a00f8659c..69fe9fa7d8b77 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -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"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs new file mode 100644 index 0000000000000..b28ddf85d105d --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs @@ -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())); + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 0c6b55d63451c..ff93bb6cbdc4a 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -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}; @@ -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; diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap new file mode 100644 index 0000000000000..f78456b542100 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap @@ -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: + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 5bf96af3b8c82..7e036d676b65b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2206,6 +2206,7 @@ "PYI04", "PYI042", "PYI043", + "PYI045", "PYI048", "PYI05", "PYI052", From 635a41e126b75a1c3c5860c2d960407baf920122 Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Mon, 29 May 2023 00:45:02 -0400 Subject: [PATCH 2/4] add missing snapshot --- ...f__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap index f78456b542100..bcd18ba99737a 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap @@ -4,7 +4,7 @@ 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 +10 | def __iter__(self) -> typing.Iterable[int]: ... # Error: PYI045 | ^^^^^^^^^^^^^^^^^^^^ PYI045 11 | def not_iter(self) -> typing.Iterable[int]: ... | @@ -12,7 +12,7 @@ PYI045.pyi:9:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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 +14 | def __iter__(self) -> typing.Iterable: ... # Error: PYI045 | ^^^^^^^^^^^^^^^ PYI045 15 | def not_iter(self) -> typing.Iterable: ... | @@ -20,7 +20,7 @@ PYI045.pyi:13:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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 +18 | def __iter__(self) -> collections.abc.Iterable[int]: ... # Error: PYI045 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI045 19 | def not_iter(self) -> collections.abc.Iterable[int]: ... | @@ -28,7 +28,7 @@ PYI045.pyi:17:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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 +22 | def __iter__(self) -> collections.abc.Iterable: ... # Error: PYI045 | ^^^^^^^^^^^^^^^^^^^^^^^^ PYI045 23 | def not_iter(self) -> collections.abc.Iterable: ... | @@ -36,7 +36,7 @@ PYI045.pyi:21:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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 +26 | def __iter__(self) -> Iterable: ... # Error: PYI045 | ^^^^^^^^ PYI045 27 | 28 | class IteratorReturn: From ee5d85e34a39a963028ff3734d8214701e86c279 Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Mon, 29 May 2023 00:52:06 -0400 Subject: [PATCH 3/4] doc formatting --- .../rules/flake8_pyi/rules/iter_method_return_iterable.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs index b28ddf85d105d..9b0774de69ad3 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs @@ -17,13 +17,15 @@ use crate::checkers::ast::Checker; /// ## Example /// ```python /// class Foo: -/// def __iter__(self) -> collections.abc.Iterable: ... +/// def __iter__(self) -> collections.abc.Iterable: +/// ... /// ``` /// /// Use instead: /// ```python /// class Foo: -/// def __iter__(self) -> collections.abc.Iterator: ... +/// def __iter__(self) -> collections.abc.Iterator: +/// ... /// ``` #[violation] pub struct IterMethodReturnIterable; From 190f98f7dd09c4612b3df7be8601638ed5b9d492 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 29 May 2023 17:21:06 -0400 Subject: [PATCH 4/4] Include async --- .../test/fixtures/flake8_pyi/PYI045.py | 12 ++- .../test/fixtures/flake8_pyi/PYI045.pyi | 6 ++ .../rules/iter_method_return_iterable.rs | 102 ++++++++++++------ ..._flake8_pyi__tests__PYI045_PYI045.pyi.snap | 26 ++++- 4 files changed, 106 insertions(+), 40 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py index 5ea53df34479f..3bc27876b22df 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.py @@ -2,8 +2,6 @@ import typing from collections.abc import Iterator, Iterable -# All OK for PYI045 because this isn't a stub. - class NoReturn: def __iter__(self): @@ -75,3 +73,13 @@ def __iter__(self) -> collections.abc.Iterator: class CollectionsIteratorTReturn: def __iter__(self) -> collections.abc.Iterator[int]: ... + + +class TypingAsyncIterableTReturn: + def __aiter__(self) -> typing.AsyncIterable[int]: + ... + + +class TypingAsyncIterableReturn: + def __aiter__(self) -> typing.AsyncIterable: + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi index 1e26c7192eaa9..a7141baa18518 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI045.pyi @@ -41,3 +41,9 @@ class CollectionsIteratorReturn: class CollectionsIteratorTReturn: def __iter__(self) -> collections.abc.Iterator[int]: ... + +class TypingAsyncIterableTReturn: + def __aiter__(self) -> typing.AsyncIterable[int]: ... # Error: PYI045 + +class TypingAsyncIterableReturn: + def __aiter__(self) -> typing.AsyncIterable: ... # Error: PYI045 diff --git a/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs index 9b0774de69ad3..3ee4013538802 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/iter_method_return_iterable.rs @@ -9,80 +9,116 @@ 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. +/// Checks for `__iter__` methods in stubs that return `Iterable[T]` instead +/// of an `Iterator[T]`. /// /// ## Why is this bad? -/// `__iter__` should return an `Iterator`, not an `Iterable`. +/// `__iter__` methods should always should return an `Iterator` of some kind, +/// not an `Iterable`. +/// +/// In Python, an `Iterator` is an object that has a `__next__` method, which +/// provides a consistent interface for sequentially processing elements from +/// a sequence or other iterable object. Meanwhile, an `Iterable` is an object +/// with an `__iter__` method, which itself returns an `Iterator`. +/// +/// Every `Iterator` is an `Iterable`, but not every `Iterable` is an `Iterator`. +/// By returning an `Iterable` from `__iter__`, you may end up returning an +/// object that doesn't implement `__next__`, which will cause a `TypeError` +/// at runtime. For example, returning a `list` from `__iter__` will cause +/// a `TypeError` when you call `__next__` on it, as a `list` is an `Iterable`, +/// but not an `Iterator`. /// /// ## Example /// ```python -/// class Foo: -/// def __iter__(self) -> collections.abc.Iterable: +/// import collections.abc +/// +/// +/// class Class: +/// def __iter__(self) -> collections.abc.Iterable[str]: /// ... /// ``` /// /// Use instead: /// ```python -/// class Foo: -/// def __iter__(self) -> collections.abc.Iterator: +/// import collections.abc +/// +/// class Class: +/// def __iter__(self) -> collections.abc.Iterator[str]: /// ... /// ``` #[violation] -pub struct IterMethodReturnIterable; +pub struct IterMethodReturnIterable { + async_: bool, +} 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`.") + let IterMethodReturnIterable { async_ } = self; + if *async_ { + format!("`__aiter__` methods should return an `AsyncIterator`, not an `AsyncIterable`") + } else { + format!("`__iter__` methods should return an `Iterator`, not an `Iterable`") + } } } /// PYI045 pub(crate) fn iter_method_return_iterable(checker: &mut Checker, definition: &Definition) { let Definition::Member(Member { - kind: MemberKind::Method, - stmt, - .. - }) = definition else { + kind: MemberKind::Method, + stmt, + .. + }) = definition else { return; }; let Stmt::FunctionDef(ast::StmtFunctionDef { - name, - returns, - .. - }) = stmt else { + 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, + let annotation = if let Expr::Subscript(ast::ExprSubscript { value, .. }) = returns.as_ref() { + // Ex) `Iterable[T]` + value + } else { + // Ex) `Iterable`, `typing.Iterable` + returns + }; + + let async_ = match name.as_str() { + "__iter__" => false, + "__aiter__" => true, _ => return, }; if checker .semantic_model() .resolve_call_path(annotation) - .map_or(false, |cp| { - matches!( - cp.as_slice(), - &["typing", "Iterable"] | &["collections", "abc", "Iterable"] - ) + .map_or(false, |call_path| { + if async_ { + matches!( + call_path.as_slice(), + ["typing", "AsyncIterable"] | ["collections", "abc", "AsyncIterable"] + ) + } else { + matches!( + call_path.as_slice(), + ["typing", "Iterable"] | ["collections", "abc", "Iterable"] + ) + } }) { - checker - .diagnostics - .push(Diagnostic::new(IterMethodReturnIterable, returns.range())); + checker.diagnostics.push(Diagnostic::new( + IterMethodReturnIterable { async_ }, + returns.range(), + )); } } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap index bcd18ba99737a..54b29a0823cb0 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI045_PYI045.pyi.snap @@ -1,7 +1,7 @@ --- 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`. +PYI045.pyi:9:27: PYI045 `__iter__` methods should return an `Iterator`, not an `Iterable` | 9 | class TypingIterableTReturn: 10 | def __iter__(self) -> typing.Iterable[int]: ... # Error: PYI045 @@ -9,7 +9,7 @@ PYI045.pyi:9:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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`. +PYI045.pyi:13:27: PYI045 `__iter__` methods should return an `Iterator`, not an `Iterable` | 13 | class TypingIterableReturn: 14 | def __iter__(self) -> typing.Iterable: ... # Error: PYI045 @@ -17,7 +17,7 @@ PYI045.pyi:13:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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`. +PYI045.pyi:17:27: PYI045 `__iter__` methods should return an `Iterator`, not an `Iterable` | 17 | class CollectionsIterableTReturn: 18 | def __iter__(self) -> collections.abc.Iterable[int]: ... # Error: PYI045 @@ -25,7 +25,7 @@ PYI045.pyi:17:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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`. +PYI045.pyi:21:27: PYI045 `__iter__` methods should return an `Iterator`, not an `Iterable` | 21 | class CollectionsIterableReturn: 22 | def __iter__(self) -> collections.abc.Iterable: ... # Error: PYI045 @@ -33,7 +33,7 @@ PYI045.pyi:21:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 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`. +PYI045.pyi:25:27: PYI045 `__iter__` methods should return an `Iterator`, not an `Iterable` | 25 | class IterableReturn: 26 | def __iter__(self) -> Iterable: ... # Error: PYI045 @@ -42,4 +42,20 @@ PYI045.pyi:25:27: PYI045 __iter__ methods should never return ` Iterable[T]`, as 28 | class IteratorReturn: | +PYI045.pyi:46:28: PYI045 `__aiter__` methods should return an `AsyncIterator`, not an `AsyncIterable` + | +46 | class TypingAsyncIterableTReturn: +47 | def __aiter__(self) -> typing.AsyncIterable[int]: ... # Error: PYI045 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI045 +48 | +49 | class TypingAsyncIterableReturn: + | + +PYI045.pyi:49:28: PYI045 `__aiter__` methods should return an `AsyncIterator`, not an `AsyncIterable` + | +49 | class TypingAsyncIterableReturn: +50 | def __aiter__(self) -> typing.AsyncIterable: ... # Error: PYI045 + | ^^^^^^^^^^^^^^^^^^^^ PYI045 + | +