The previous example is taken from the Python documentation that states that generators that only yield values may be anotated “as having a return type of either Iterable[YieldType] or Iterator[YieldType]” instead of Generator[YieldType, None, None].
Indeed, S5886 should not raise any issue in this case. I’m also not able to reproduce any false positive with the example you gave.
However, I see we do have a false positive in the following example (taken from the same documentation page), due to the fact that the yield statement is part of an assignment statement.
def echo_round() -> Generator[int, float, str]:
sent = yield 0
while sent >= 0:
sent = yield round(sent)
return 'Done' # FP S5886 here
Or, in this slightly modified reproducer:
def echo_round() -> Generator[int, float, str]:
sent = 0
while sent < 3:
sent += 1
x = yield sent
return 'Done' # FP S5886 here
To address this, I created the following ticket. Can you confirm the issue you’re facing is similar? If not, can you please give me a reproducer that raises a false positive?
I am honestly surprised the example I gave did not reproduce. My issue arise when the returned type is annotated with Iterator[XXX] (e.g. Iterator[str]). It happens quite a lot, since I find it more readable than Generator[str, None, None] so I use it a lot in my code.
Thanks to the the SonarCloud example you gave, I managed to reproduce the false positive:
import sys
if sys.version_info < (3, 9):
from typing import Iterator
else:
from collections.abc import Iterator
def my_iterator() -> Iterator[str]:
yield "hello" # FP S5886
The issue is raised because of the 2 imports of Iterator. The analyzer somehow fails to disambiguate them and to understand that both underlying symbols are valid return types for a generator.
I have found the same issue with a simpler reproducer that may indicate this is not a type disambiguation problem.
from __future__ import annotations
from collections.abc import Iterable
def foo() -> Iterable[str]:
yield ""
Given the error of
annotate function “run” with “typing.Generator” or one of its supertypes
I suspect the underlying problem is that typing.Generator is not being abstract/erased to collections.abc.Generator, and so collections.abc.Iterable is not considered a supertype.
Indeed, it seems that the rule strictly expects something annotated as typing.Iterator, typing.Iterable, typing.Generator or their async alternatives. And this flew under our radar for quite some time now…
I’ve updated the ticket accordingly and I’ve bumped its priority so that we make sure to address this.
Thanks for your feedback, and sorry we didn’t respond on this for quite some time.