Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mypy does not check the code after the line with lambda function that calls NoReturn function #17254

Closed
ravilnicki opened this issue May 16, 2024 · 8 comments · Fixed by #17287
Labels
bug mypy got something wrong

Comments

@ravilnicki
Copy link

ravilnicki commented May 16, 2024

Bug Report
mypy does not check the code after the line with lambda function that calls NoReturn function

To Reproduce
Check the code below. I also attached a mypy Playground link.

from typing import NoReturn


def foo() -> NoReturn:
    raise


f = lambda _: foo()  # when this line is commented mypy works as expected
bar: int = "baz"

https://mypy-play.net/?mypy=latest&python=3.12&gist=a924d0f423440c8a1a48348f49edae4e

Expected Behavior
mypy checks the code after line with lambda function that calls NoReturn function, not only the code before that line

Actual Behavior
mypy does not check the code after the line with lambda function that calls NoReturn function

Environment

  • Mypy version used: mypy 1.5.0-1.10.0
  • Python version used: Python 3.11.2

Edit
I edited the used mypy version because I noticed the same bug in many versions.

@ravilnicki ravilnicki added the bug mypy got something wrong label May 16, 2024
@ravilnicki
Copy link
Author

It looks like this bug was introduced in version 1.5.0. I tested previous versions and version 1.4.1 is the last one that works as expected.

@ravilnicki
Copy link
Author

I've found the culprit - 3bf8521.

@ravilnicki
Copy link
Author

ravilnicki commented May 17, 2024

@ikonst please take a look

file mypy/checker.py line 467-

-    if (
-        self.binder.is_unreachable()
-        and self.should_report_unreachable_issues()
-        and not self.is_raising_or_empty(d)
-     ):
-        self.msg.unreachable_statement(d)
-        break
-    self.accept(d)

+    if self.binder.is_unreachable():
+        if not self.should_report_unreachable_issues():
+             break
+         if not self.is_noop_for_reachability(d):
+             self.msg.unreachable_statement(d)
+             break
+    else:
+        self.accept(d)

In the previous version it was possible for self.accept(d) to run even if the self.binder.is_unreachable() was True (eg when self.should_report_unreachable_issues() was False). After your changes when self.binder.is_unreachable() is True there is no possibility to run self.accept(d).

In the example I provided (the one with lambda function) the line after lambda is marked as unreachable which I think is also not right. Nevertheless in versions before 1.5.0 this is not an issue and the next lines are checked.

@ikonst
Copy link
Contributor

ikonst commented May 17, 2024

You can see me discussing in the PR that maybe the right thing to do is not to avoid type checking unreachable code. I just don't think I ever got to make that change since it seemed larger.

@ravilnicki
Copy link
Author

How do you think we should proceed? I think that mypy falsely marks the statement after this lambda expression as an unreachable code. So that is the actual bug. In the older versions it went unnoticed though and after your change it stopped checking the presumably unreachable code. I think it shouldn’t be marked as u reachable in the first place. What do you think @ikonst?

@ikonst
Copy link
Contributor

ikonst commented May 18, 2024

Yeah, fixing the false unreachable is probably the way to go.

@andersk
Copy link
Contributor

andersk commented May 24, 2024

By moving the code into a function (and adjusting for syntax modernization), I can reproduce the same bug all the way back to the original introduction of NoReturn in 9406886 = v0.500~45.

from mypy_extensions import NoReturn

def foo():
    # type: () -> NoReturn
    raise

def main():
    # type: () -> None
    f = lambda _: foo()  # when this line is commented mypy works as expected
    bar = "baz"  # type: int

@andersk
Copy link
Contributor

andersk commented May 24, 2024

It looks like the problem is that ExpressionChecker.visit_lambda_expr never pushes a new Frame onto self.chk.binder, and unreachability is associated with a Frame so it leaks outside the lambda body.

Indeed, this effect is noted in a comment there, although the chosen solution only works around unreachability from the implicit return statement and not other kinds of unreachability:

mypy/mypy/checkexpr.py

Lines 5227 to 5235 in 43a605f

# Lambdas can have more than one element in body,
# when we add "fictional" AssigmentStatement nodes, like in:
# `lambda (a, b): a`
for stmt in e.body.body[:-1]:
stmt.accept(self.chk)
# Only type check the return expression, not the return statement.
# This is important as otherwise the following statements would be
# considered unreachable. There's no useful type context.
ret_type = self.accept(e.expr(), allow_none_return=True)

andersk added a commit to andersk/mypy that referenced this issue May 24, 2024
Fixes python#17254.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/mypy that referenced this issue May 24, 2024
Fixes python#17254.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
hauntsaninja pushed a commit that referenced this issue May 26, 2024
Fixes #17254

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants