Add broken NoReturn check

[cdce8p]

Type of Changes

	Type
✓	✨ New feature

Description

Response to: pylint-dev/astroid#1239

Add a new check to detect broken uses for typing.NoReturn in 3.7.0 / 3.7.1.
I'm open to changing the checker name if something better than broken-noreturn is found :)

[Pierre-Sassoulas]

Shouldn't this be in flake8-typing-imports instead ? We were going to add it to our own pre-commit configuration in #5070 (and astroid too)

[cdce8p]

Shouldn't this be in flake8-typing-imports instead ? We were going to add it to our own pre-commit configuration in #5070 (and astroid too)

Not sure Anthony will add it ;)
Technically, NoReturn can be imported and even used, it's just broken in these special cases. So I'm not sure what flake8-typing-import should emit. Importing it from typing_extensions doesn't fix this issue either.

[Pierre-Sassoulas]

Technically, NoReturn can be imported and even used, it's just broken in these special cases.

Ok, that makes sense. Trying not to add anything in 2.12 so we can release though :)

Not sure Anthony will add it ;)

Yeah, it's often a struggle.

[coveralls]

Pull Request Test Coverage Report for Build 1963926454

• 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.006%) to 94.006%

---

Totals
Change from base Build 1963138182:	0.006%
Covered Lines:	14961
Relevant Lines:	15915

---

💛 - Coveralls

[Pierre-Sassoulas]

I'm open to changing the checker name if something better than broken-noreturn is found :)

invalid-no-return-as-typing-argument ? It's a little lengthy.

[cdce8p]

I'm open to changing the checker name if something better than broken-noreturn is found :)

invalid-no-return-as-typing-argument ? It's a little lengthy.

Doesn't really fit IMO. NoReturn isn't invalid itself. It's just the specific usage that breaks.

[Pierre-Sassoulas]

It's just the specific usage that breaks.

I thought this usage where it's not valid was when it's used as a typing argument ?

[cdce8p]

You can still use it like this

def func() -> NoReturn:
    ...

Alias = Union[None, 'NoReturn']  # note the use of ' '

at least if you don't need to call typing.get_type_annotations.

[cdce8p]

It's just the specific usage that breaks.

I thought this usage where it's not valid was when it's used as a typing argument ?

The uses are valid. It just breaks because of https://bugs.python.org/issue34921

[DanielNoord]

@cdce8p Is this ready for review (barring the merge conflicts)?

[cdce8p]

@cdce8p Is this ready for review (barring the merge conflicts)?

Yes. I just resolved the merged conflicts.

[DanielNoord]

One little question. Rest looks good!

[Pierre-Sassoulas]

LGTM !

[cdce8p]

Need to do one more change. So far I've ignored from __future__ import annotations here.

Update: cc7fb08