Looks good! However, I'm afraid that NoReturn in third position is not actually enforced by any means for user-defined generator subclasses.

from typing import *

class G(Generator[int, None, Never]):
    def __iter__(self) -> 'G':
        return self
    
    def __next__(self) -> int:
        raise StopIteration
    
    def send(self, value: None, /) -> int:
        assert False, "unused"
        
    def throw(
        self, typ: object, val: object = None, tb: object | None = None, /
    ) -> int:
        assert False, "unused"
    

def check() -> str:
    for _ in G():
        return ""
    print("Reached")

check()

This (untested, sorry for any typos) should print "Reached", and this PR should mark that line unreachable. It's a separate problem, of course, but I'm not sure this can be merged until we can somehow enforce Never vs None distinction there.

I'm not sure it's possible to enforce any distinction there. We could enforce that raise StopIteration only happens in __iter__ but that seems overly limiting. Otherwise we would need some sort of checked exceptions... Any ideas? I guess we could do some best effort thing (check StopIteration in __iter__, throw up our arms and give up. Or we could require GeneratorType? That's final so there's no possible subtype with weird behavior... We could even enforce some back-compat by mutating generator function return types to replace Generator with GeneratorType? Last part seems a bit much.)

I'll think about this tomorrow. __next__ is the right place for raise StopIteration, restricting it to __iter__ would be weird.

The problem stems from the fact that _ReturnT is only referenced in close method that is not abstract and __iter__ method that can return self, so the default implementation cannot be safe for arbitrary type _ReturnT. I can suggest to go as far as prohibiting non-None third parameter when subclassing Generator without overriding close, that would be sufficient to enforce consistency. My example above could have been class G(Generator[int, None, str]) just as well, and type checkers won't mark that as error.

Sorry yes that's what I meant re: __next__. OK that actually makes sense -- I agree. I guess close can't be abstract because it isn't at runtime (only send and throw are).

I'm not convinced that subclassing a subclass will work correctly for close detection... Would be nice if there were a conditional way to define abstract methods!

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Hm, okay, I revisited this once again, maybe it's not really a blocker and we should file a separate feature request to check third type argument of Generator more strictly.

I don't think we need to support initial definition in subclasses. This should be an error even if some subclasses define close with correct signature, as G itself violates its contract:

class G(Generator[int, None, str]):
    def __iter__(self) -> 'G':
        return self
    
    def __next__(self) -> int:
        raise StopIteration
    
    def send(self, value: None, /) -> int:
        assert False, "unused"
        
    def throw(
        self, typ: object, val: object = None, tb: object | None = None, /
    ) -> int:
        assert False, "unused"

On the other hand, if the direct subclass of Generator was confirmed to be correct, we'll get checking close in future subclasses for free - that's just plain override compatibility. If I'm not missing anything, we only need one special case - immediate subclasses of Generator without close definition can only inherit from Generator[..., ..., None]). For the third argument to be non-None, explicit close must be provided (only provided, the rest should be checked by existing infrastructure).

I only envision this as a sanity check specifically for Generator. Since it can't be safely implemented (close must return some unknown _ReturnT, not its default, so must be abstract), no general support is necessary - only a patch to handle this specific typeshed definition.

@sterliakov sorry I haven't actually thought about this more until now. I think pushing off doing this properly is fine. I think doing this properly could be what you talk about, or it could be something like:

• update close to have a self-type of Generator[Any, Any, None] in typeshed?
• fix self annotation not checked in __index__? #19341

EDIT: nevermind, I thought a bit more and the second approach wouldn't work (because we need to say there's a close that returns the last type always, it's just not defined by default for anything other than None...). Unless we allow overloading with abstractmethod, but that's sounds complex.

Fixes #7374?

Good catch, I'll separate out the part of that issue that this doesn't fix into another issue.