diff --git a/mypy/checker.py b/mypy/checker.py index b6a9bb3b22cd..26bf836a29f8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -152,6 +152,7 @@ from mypy.patterns import AsPattern, StarredPattern from mypy.plugin import Plugin from mypy.plugins import dataclasses as dataclasses_plugin +from mypy.reachability import MYPY_FALSE, MYPY_TRUE, infer_condition_value from mypy.scope import Scope from mypy.semanal import is_trivial_body, refers_to_fullname, set_callable_name from mypy.semanal_enum import ENUM_BASES, ENUM_SPECIAL_PROPS @@ -5021,21 +5022,61 @@ def visit_if_stmt(self, s: IfStmt) -> None: if_map, else_map = self.find_isinstance_check(e) - # 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) self.accept(b) + self._visit_if_stmt_redundant_expr_helper( + s, e, b, if_map, else_map, self.binder.frames[-1].unreachable + ) - # XXX Issue a warning if condition is always True? self.push_type_map(else_map, from_assignment=False) with self.binder.frame_context(can_skip=False, fall_through=2): if s.else_body: self.accept(s.else_body) + def _visit_if_stmt_redundant_expr_helper( + self, + stmt: IfStmt, + expr: Expression, + body: Block, + if_map: TypeMap, + else_map: TypeMap, + else_reachable: bool, + ) -> None: + """Emits `redundant-expr` errors for if statements that are always true or always false. + + We try to avoid emitting such errors if the redundancy seems to be intended as part of + dynamic type or exhaustiveness checking (risking to miss some unintended redundant if + statements). + """ + + if codes.REDUNDANT_EXPR not in self.options.enabled_error_codes: + return + if infer_condition_value(expr, self.options) in (MYPY_FALSE, MYPY_TRUE): + return + + def _filter(body: Block | None) -> bool: + if body is None: + return False + return all(self.is_noop_for_reachability(s) for s in body.body) + + if if_map is None: + if stmt.while_stmt: + self.msg.redundant_condition_in_while(False, expr) + elif not _filter(body): + self.msg.redundant_condition_in_if(False, expr) + + if else_map is None: + if stmt.while_stmt: + if not is_true_literal(expr): + self.msg.redundant_condition_in_while(True, expr) + elif not (else_reachable or _filter(stmt.else_body)): + self.msg.redundant_condition_in_if(True, expr) + def visit_while_stmt(self, s: WhileStmt) -> None: """Type check a while statement.""" - if_stmt = IfStmt([s.expr], [s.body], None) + if_stmt = IfStmt([s.expr], [s.body], None, while_stmt=True) if_stmt.set_line(s) self.accept_loop(if_stmt, s.else_body, exit_condition=s.expr) diff --git a/mypy/messages.py b/mypy/messages.py index c6378c264757..8b1e40fad9a7 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -2091,6 +2091,9 @@ def redundant_condition_in_comprehension(self, truthiness: bool, context: Contex def redundant_condition_in_if(self, truthiness: bool, context: Context) -> None: self.redundant_expr("If condition", truthiness, context) + def redundant_condition_in_while(self, truthiness: bool, context: Context) -> None: + self.redundant_expr("While condition", truthiness, context) + def redundant_expr(self, description: str, truthiness: bool, context: Context) -> None: self.fail( f"{description} is always {str(truthiness).lower()}", diff --git a/mypy/nodes.py b/mypy/nodes.py index 040f3fc28dce..931805dae0ff 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -1792,19 +1792,28 @@ def accept(self, visitor: StatementVisitor[T]) -> T: class IfStmt(Statement): - __slots__ = ("expr", "body", "else_body") + __slots__ = ("expr", "body", "else_body", "while_stmt") - __match_args__ = ("expr", "body", "else_body") + __match_args__ = ("expr", "body", "else_body", "while_stmt") expr: list[Expression] body: list[Block] else_body: Block | None + while_stmt: bool # Is the if statement a converted while statement? - def __init__(self, expr: list[Expression], body: list[Block], else_body: Block | None) -> None: + def __init__( + self, + expr: list[Expression], + body: list[Block], + else_body: Block | None, + *, + while_stmt: bool = False, + ) -> None: super().__init__() self.expr = expr self.body = body self.else_body = else_body + self.while_stmt = while_stmt def accept(self, visitor: StatementVisitor[T]) -> T: return visitor.visit_if_stmt(self) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 5043d5422108..e64f9163c4b8 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -3033,3 +3033,131 @@ if isinstance(a, B): c = a [builtins fixtures/isinstance.pyi] + +[case testRedundantExpressionWarningsForIfStatements] +# flags: --enable-error-code=redundant-expr +from typing import Literal + +if True: # E: If condition is always true + x = 1 + +if False: # E: If condition is always false + x = 1 + +class W: + ... +if W(): + x = 1 + +class X: + def __bool__(self) -> bool: ... +if X(): + x = 1 + +class Y: + def __bool__(self) -> Literal[True]: ... +if Y(): # E: If condition is always true + x = 1 + +class Z: + def __bool__(self) -> Literal[False]: ... +if Z(): # E: If condition is always false + x = 1 + +[builtins fixtures/bool.pyi] + +[case testRedundantExpressionWarningsForWhileStatements] +# flags: --enable-error-code=redundant-expr + +x = None +while True: + if x: + break + x = True + +while False: # E: While condition is always false + ... + +y = None +while y is None: # E: While condition is always true + ... + +[builtins fixtures/bool.pyi] + +[case testNoRedundantExpressionWarningsForUsagesOfTYPECHECKING] +# flags: --enable-error-code=redundant-expr +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + ... + +if not TYPE_CHECKING: + ... + +[case testNoRedundantExpressionWarningsForExhaustivenessChecks] +# flags: --enable-error-code=redundant-expr +from typing import Literal, Never, NoReturn + +def assert_never(x: Never) -> NoReturn: + ... +def f1(x: Literal[1, 2]) -> str: + if x == 1: + y = "a" + elif x == 2: + y = "b" + else: + assert_never(x) + return y + +class ValueError: ... +def f2(x: Literal[1]) -> str: + if x == 1: + y = "a" + else: + raise ValueError + return y + +def f3(x: Literal[1]) -> str: + if x == 1: + y = "a" + else: + assert False + return y + +def f4(x: Literal[1, 2]) -> int: + if x == 1: + return 1 + if x == 2: + return 2 + +def f5(x: Literal[1, 2]) -> int: + if x == 1: + return 1 + if x == 2: + if bool(): + return 2 + else: + return 3 + +def f6(x: Literal[1]) -> None: + if x != 1: + raise ValueError + +def f7(x: Literal[1, 2]) -> None: + if x == 1: + y = "a" + elif x == 2: + y = "b" + else: + pass + +def f8(x: Literal[1, 2]) -> None: + if x == 1: + y = "a" + elif x == 2: # E: If condition is always true + y = "b" + else: + pass + y = "c" + +[builtins fixtures/bool.pyi]