Honor NoReturn as __setitem__ return type to mark unreachable code.

[sterliakov]

Description

In general code that follows any calls to methods annotated with NoReturn is considered unreachable. However calls like variable['foo'] = 'foo' are not treated this way. Moreover, variable.__setitem__('foo', 'foo') is interpreted properly, so this behavior is inconsistent. After this change both variants result in marking remaining part of the branch as unreachable.

Test Plan

Added test case where code after indexed assignment should be reported is unreachable:

[case testSetitemNoReturn]
# flags: --warn-unreachable
from typing import NoReturn
class Foo:
    def __setitem__(self, key: str, value: str) -> NoReturn:
        raise Exception
Foo()['a'] = 'a'
x = 0  # E: Statement is unreachable
[builtins fixtures/exception.pyi]

[hauntsaninja]

Thanks, this looks great!

One small adjustment, could you apply this diff:

diff --git a/mypy/checker.py b/mypy/checker.py
index 3340cb4b4..bf9905e7d 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -4055,7 +4055,8 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
             [nodes.ARG_POS, nodes.ARG_POS],
             context,
         )
-        if isinstance(get_proper_type(res_type), UninhabitedType):
+        res_type = get_proper_type(res_type)
+        if isinstance(res_type, UninhabitedType) and not res_type.ambiguous:
             self.binder.unreachable()
 
     def try_infer_partial_type_from_indexed_assignment(

And add a test case like:

from typing import NoReturn, TypeVar

T = TypeVar("T")

class Foo:
    def __setitem__(self, key: str, value: str) -> T:
        raise Exception

def f() -> None:
    Foo()['a'] = 'a'
    x = 0

[sterliakov]

I'm happy to apply the diff (PR updated), but the proposed test case looks weird (unbound TypeVar), and I expect only one error ("a function returning TypeVar...", if I recall properly) unrelated to NoReturn. Could you clarify your intent here? T should be resolved to Any here and not trigger unreachable warning.

[hauntsaninja]

It was meant to be a test for the additional diff I proposed. I think on the original PR you'd get an incorrect unreachable warning on that test.

(I can double check when I'm at a computer)

[hauntsaninja]

Yeah, confirmed that on 8c09147 you get a spurious error on my test case test.py:11: error: Statement is unreachable that is fixed by applying the diff in 8717196

[sterliakov]

Sorry, haven't noticed that. Updated the PR with this test.

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[hauntsaninja]

Thank you!