Skip to content

Commit

Permalink
Suppress errors for unreachable branches in conditional expressions (#…
Browse files Browse the repository at this point in the history
…18295)

Fixes #4134
Fixes #9195

Suppress errors when analyzing unreachable conditional expression
branches. Same idea as what's done when analyzing the right-hand operand
of `and`/`or`:

https://github.com/python/mypy/blob/973618a6bfa88398e08dc250c8427b381b3a0fce/mypy/checkexpr.py#L4252-L4256

This PR originally added filters of the same form to the places where
`analyze_cond_branch` is called in
`ExpressionChecker.visit_conditional_expr`. However, since 5 out of the
6 `analyze_cond_branch` call sites now use `filter_errors` for the case
when `map is None`, I decided to move the error filtering logic to
inside `analyze_cond_branch`.

**Given:**
```python
from typing import TypeVar
T = TypeVar("T", int, str)

def foo(x: T) -> T:
    return x + 1 if isinstance(x, int) else x + "a"
```
**Before:**
```none
main.py:5:16: error: Unsupported operand types for + ("str" and "int")  [operator]
main.py:5:49: error: Unsupported operand types for + ("int" and "str")  [operator]
Found 2 errors in 1 file (checked 1 source file)
```
**After:**
```
Success: no issues found in 1 source file
```
  • Loading branch information
brianschubert authored Dec 20, 2024
1 parent d33cef8 commit 7959a20
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
4 changes: 3 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4793,7 +4793,9 @@ def visit_assert_stmt(self, s: AssertStmt) -> None:
# If this is asserting some isinstance check, bind that type in the following code
true_map, else_map = self.find_isinstance_check(s.expr)
if s.msg is not None:
self.expr_checker.analyze_cond_branch(else_map, s.msg, None)
self.expr_checker.analyze_cond_branch(
else_map, s.msg, None, suppress_unreachable_errors=False
)
self.push_type_map(true_map)

def visit_raise_stmt(self, s: RaiseStmt) -> None:
Expand Down
13 changes: 6 additions & 7 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4249,11 +4249,7 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type:
):
self.msg.unreachable_right_operand(e.op, e.right)

# If right_map is None then we know mypy considers the right branch
# to be unreachable and therefore any errors found in the right branch
# should be suppressed.
with self.msg.filter_errors(filter_errors=right_map is None):
right_type = self.analyze_cond_branch(right_map, e.right, expanded_left_type)
right_type = self.analyze_cond_branch(right_map, e.right, expanded_left_type)

if left_map is None and right_map is None:
return UninhabitedType()
Expand Down Expand Up @@ -5851,12 +5847,15 @@ def analyze_cond_branch(
node: Expression,
context: Type | None,
allow_none_return: bool = False,
suppress_unreachable_errors: bool = True,
) -> Type:
with self.chk.binder.frame_context(can_skip=True, fall_through=0):
if map is None:
# We still need to type check node, in case we want to
# process it for isinstance checks later
self.accept(node, type_context=context, allow_none_return=allow_none_return)
# process it for isinstance checks later. Since the branch was
# determined to be unreachable, any errors should be suppressed.
with self.msg.filter_errors(filter_errors=suppress_unreachable_errors):
self.accept(node, type_context=context, allow_none_return=allow_none_return)
return UninhabitedType()
self.chk.push_type_map(map)
return self.accept(node, type_context=context, allow_none_return=allow_none_return)
Expand Down
7 changes: 7 additions & 0 deletions test-data/unit/check-expressions.test
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,13 @@ x.append(y) if bool() else x.append(y)
z = x.append(y) if bool() else x.append(y) # E: "append" of "list" does not return a value (it only ever returns None)
[builtins fixtures/list.pyi]

[case testConditionalExpressionWithUnreachableBranches]
from typing import TypeVar
T = TypeVar("T", int, str)
def foo(x: T) -> T:
return x + 1 if isinstance(x, int) else x + "a"
[builtins fixtures/isinstancelist.pyi]

-- Special cases
-- -------------

Expand Down

0 comments on commit 7959a20

Please # to comment.