Skip to content

Avoid false possibly-undefined errors due to omitted unrequired else statements. #20149

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+36 −6
Open

Avoid false possibly-undefined errors due to omitted unrequired else statements. #20149

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1ed9c3
Avoid false `possibly-undefined` errors due to omitted unrequired els…
tyralla Oct 30, 2025
7e9b592
else_irrelevant_for_possibly_undefined -> unreachable_else
tyralla Oct 31, 2025
b9c59be
else_irrelevant_for_possibly_undefined -> unreachable_else
tyralla Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5036,6 +5036,8 @@ def visit_if_stmt(self, s: IfStmt) -> None:

if_map, else_map = self.find_isinstance_check(e)

s.else_irrelevant_for_possibly_undefined = else_map is None
Copy link
Collaborator

@sterliakov sterliakov Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So else_irrelevant_for_possibly_undefined is essentially equivalent to else_unreachable or is_exhaustive? I'm a bit confused by this naming

Copy link
Collaborator Author

@tyralla tyralla Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I should have changed this. When I started working on this PR, I thought that the assert_never cases and similar ones would also need similar treatment. So else_irrelevant_for_possibly_undefined meant "unreachable or does not return" to me. I will change it later. Thanks!

Copy link
Collaborator Author

@tyralla tyralla Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


# XXX Issue a warning if condition is always False?
with self.binder.frame_context(can_skip=True, fall_through=2):
self.push_type_map(if_map, from_assignment=False)
Expand Down
6 changes: 4 additions & 2 deletions mypy/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1792,19 +1792,21 @@ def accept(self, visitor: StatementVisitor[T]) -> T:


class IfStmt(Statement):
__slots__ = ("expr", "body", "else_body")
__slots__ = ("expr", "body", "else_body", "else_irrelevant_for_possibly_undefined")

__match_args__ = ("expr", "body", "else_body")
__match_args__ = ("expr", "body", "else_body", "else_body_irrelevant_for_possibly_undefined")

expr: list[Expression]
body: list[Block]
else_body: Block | None
else_irrelevant_for_possibly_undefined: bool

def __init__(self, expr: list[Expression], body: list[Block], else_body: Block | None) -> None:
super().__init__()
self.expr = expr
self.body = body
self.else_body = else_body
self.else_irrelevant_for_possibly_undefined = False

def accept(self, visitor: StatementVisitor[T]) -> T:
return visitor.visit_if_stmt(self)
Expand Down
10 changes: 6 additions & 4 deletions mypy/partially_defined.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,13 @@ def visit_if_stmt(self, o: IfStmt) -> None:
continue
b.accept(self)
self.tracker.next_branch()
if o.else_body:
if not o.else_body.is_unreachable:
o.else_body.accept(self)
else:
if o.else_irrelevant_for_possibly_undefined:
self.tracker.skip_branch()
elif o.else_body:
if o.else_body.is_unreachable:
self.tracker.skip_branch()
else:
o.else_body.accept(self)
self.tracker.end_branch_statement()

def visit_match_stmt(self, o: MatchStmt) -> None:
Expand Down
24 changes: 24 additions & 0 deletions test-data/unit/check-possibly-undefined.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,3 +1043,27 @@ def foo(x: Union[int, str]) -> None:
assert_never(x)
f # OK
[builtins fixtures/tuple.pyi]

[case testOmittedUnrequiredElse]
# flags: --enable-error-code possibly-undefined
from typing import Literal

a: Literal[True]
if a:
w = 1
w + 1

b: bool
if b:
x = 1
elif not b:
x = 2
x + 1

if b:
y = 1
elif not b:
if a:
z = 2
y = z
y + 1
Loading