track `if typing.TYPE_CHECKING` to warn about non runtime bindings

[terencehonles]

When importing or defining values in if typing.TYPE_CHECKING blocks the bound names will not be available at runtime and may cause errors when used in the following way:

import typing

if typing.TYPE_CHECKING:
    from module import Type  # some slow import or circular reference

def method(value) -> Type:  # the import is needed by the type checker
    assert isinstance(value, Type)  # this is a runtime error

This change allows pyflakes to track what names are bound for runtime use, and allows it to warn when a non runtime name is used in a runtime context.

[asottile]

#530 predates this and has similar goals / problems -- perhaps collaborate with that PR?

[terencehonles]

#530 predates this and has similar goals / problems -- perhaps collaborate with that PR?

@asottile It looks like I actually ended up writing about the same code :sweat_smile: however there are a few things I have which the other one doesn't:

1. The logic handles the non runtime bindings in either the body or the else of the if/else expression
2. The runtime attribute defaults to True (it generally will be) which needs less changes in other parts of the codebase.
3. The lines https://github.com/PyCQA/pyflakes/pull/530/files#diff-5df44d79406311387e0056faae7a8092d1ce9cc85c7d15420186e45a0c712283R1178-R1181 continues looking up the stack for a runtime value, but it's more likely that the non runtime value will be declared at a higher scope and therefore the code I wrote warns and aborts immediately.

It also looks like the code was waiting review for almost a year. Is there something you wanted changed from that PR? I'm willing to work with @PetterS, but I didn't realize there was an PR that old for this already opened.

[asottile]

ah shoot, that's probably on me -- I might've missed the last round of reviews on there and then it lapsed (and now conflicts)

[PetterS]

I can revive that PR if we agree that it it is likely to be merged.

This is a problem that has been observed for real code, so would be good to fix.

[terencehonles]

I can revive that PR if we agree that it it is likely to be merged.

This is a problem that has been observed for real code, so would be good to fix.

It should be pretty easy to xref this PR in order to see what might also need to change since I based my PR off the most recent release (I was not aware of your PR when starting). We're 99% inline with each other's implementations.

[PetterS]

ah shoot, that's probably on me -- I might've missed the last round of reviews on there and then it lapsed (and now conflicts)

Merged it with master again now.

[terencehonles]

#530 predates this and has similar goals / problems -- perhaps collaborate with that PR?

@asottile since the PR in comment above is not going to be updated by its author

https://github.com/PyCQA/pyflakes/pull/530#issuecomment-1427121941

any chance you'd be willing to review this PR?

As mentioned above https://github.com/PyCQA/pyflakes/pull/622#issuecomment-805307482 it had used negated logic compared to the other PR in order to minimize the number of changes to the code base, and I've continued to both use and update this PR for changes on the development branch.

I can seek a 2nd approver if we can move forward with this PR.

[terencehonles]

Thanks for the review, I'll see when I have time to address it.