diff --git a/mypy/binder.py b/mypy/binder.py index 9cb9aa433de1..e8ad27aed3e9 100644 --- a/mypy/binder.py +++ b/mypy/binder.py @@ -32,6 +32,14 @@ def __init__(self) -> None: self.types = {} # type: Dict[Key, Type] self.unreachable = False + # Should be set only if we're entering a frame where it's not + # possible to accurately determine whether or not contained + # statements will be unreachable or not. + # + # Long-term, we should improve mypy to the point where we no longer + # need this field. + self.suppress_unreachable_warnings = False + Assigns = DefaultDict[Expression, List[Tuple[Type, Optional[Type]]]] @@ -131,6 +139,9 @@ def put(self, expr: Expression, typ: Type) -> None: def unreachable(self) -> None: self.frames[-1].unreachable = True + def suppress_unreachable_warnings(self) -> None: + self.frames[-1].suppress_unreachable_warnings = True + def get(self, expr: Expression) -> Optional[Type]: key = literal_hash(expr) assert key is not None, 'Internal error: binder tried to get non-literal' @@ -141,6 +152,10 @@ def is_unreachable(self) -> bool: # this traversal on every statement? return any(f.unreachable for f in self.frames) + def is_unreachable_warning_suppressed(self) -> bool: + # TODO: See todo in 'is_unreachable' + return any(f.suppress_unreachable_warnings for f in self.frames) + def cleanse(self, expr: Expression) -> None: """Remove all references to a Node from the binder.""" key = literal_hash(expr) diff --git a/mypy/checker.py b/mypy/checker.py index 8720b4995f36..1db7acdcf44f 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -783,7 +783,8 @@ def enter_attribute_inference_context(self) -> Iterator[None]: def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str]) -> None: """Type check a function definition.""" # Expand type variables with value restrictions to ordinary types. - for item, typ in self.expand_typevars(defn, typ): + expanded = self.expand_typevars(defn, typ) + for item, typ in expanded: old_binder = self.binder self.binder = ConditionalTypeBinder() with self.binder.top_frame_context(): @@ -932,6 +933,14 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str]) # Type check body in a new scope. with self.binder.top_frame_context(): with self.scope.push_function(defn): + # We suppress reachability warnings when we use TypeVars with value + # restrictions: we only want to report a warning if a certain statement is + # marked as being suppressed in *all* of the expansions, but we currently + # have no good way of doing this. + # + # TODO: Find a way of working around this limitation + if len(expanded) >= 2: + self.binder.suppress_unreachable_warnings() self.accept(item.body) unreachable = self.binder.is_unreachable() @@ -1777,13 +1786,46 @@ def check_import(self, node: ImportBase) -> None: def visit_block(self, b: Block) -> None: if b.is_unreachable: + # This block was marked as being unreachable during semantic analysis. + # It turns out any blocks marked in this way are *intentionally* marked + # as unreachable -- so we don't display an error. self.binder.unreachable() return for s in b.body: if self.binder.is_unreachable(): + if (self.options.warn_unreachable + and not self.binder.is_unreachable_warning_suppressed() + and not self.is_raising_or_empty(s)): + self.msg.unreachable_statement(s) break self.accept(s) + def is_raising_or_empty(self, s: Statement) -> bool: + """Returns 'true' if the given statement either throws an error of some kind + or is a no-op. + + We use this function mostly while handling the '--warn-unreachable' flag. When + that flag is present, we normally report an error on any unreachable statement. + But if that statement is just something like a 'pass' or a just-in-case 'assert False', + reporting an error would be annoying. + """ + if isinstance(s, AssertStmt) and is_false_literal(s.expr): + return True + elif isinstance(s, (RaiseStmt, PassStmt)): + return True + elif isinstance(s, ExpressionStmt): + if isinstance(s.expr, EllipsisExpr): + return True + elif isinstance(s.expr, CallExpr): + self.expr_checker.msg.disable_errors() + typ = self.expr_checker.accept( + s.expr, allow_none_return=True, always_allow_any=True) + self.expr_checker.msg.enable_errors() + + if isinstance(typ, UninhabitedType): + return True + return False + def visit_assignment_stmt(self, s: AssignmentStmt) -> None: """Type check an assignment statement. @@ -2956,7 +2998,8 @@ def visit_try_stmt(self, s: TryStmt) -> None: # type checks in both contexts, but only the resulting types # from the latter context affect the type state in the code # that follows the try statement.) - self.accept(s.finally_body) + if not self.binder.is_unreachable(): + self.accept(s.finally_body) def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None: """Type check a try statement, ignoring the finally block. diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 3c2306de6fdc..54fdbca7b227 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -2367,14 +2367,25 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: restricted_left_type = true_only(left_type) result_is_left = not left_type.can_be_false + # 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. + # + # Note that we perform these checks *before* we take into account + # the analysis from the semanal phase below. We assume that nodes + # marked as unreachable during semantic analysis were done so intentionally. + # So, we shouldn't report an error. + if self.chk.options.warn_unreachable: + if left_map is None: + self.msg.redundant_left_operand(e.op, e.left) + if right_map is None: + self.msg.redundant_right_operand(e.op, e.right) + if e.right_unreachable: right_map = None elif e.right_always: left_map = None - # 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. if right_map is None: self.msg.disable_errors() try: @@ -3174,12 +3185,18 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No self.accept(condition) # values are only part of the comprehension when all conditions are true - true_map, _ = self.chk.find_isinstance_check(condition) + true_map, false_map = self.chk.find_isinstance_check(condition) if true_map: for var, type in true_map.items(): self.chk.binder.put(var, type) + if self.chk.options.warn_unreachable: + if true_map is None: + self.msg.redundant_condition_in_comprehension(False, condition) + elif false_map is None: + self.msg.redundant_condition_in_comprehension(True, condition) + def visit_conditional_expr(self, e: ConditionalExpr) -> Type: self.accept(e.cond) ctx = self.type_context[-1] @@ -3187,6 +3204,11 @@ def visit_conditional_expr(self, e: ConditionalExpr) -> Type: # Gain type information from isinstance if it is there # but only for the current expression if_map, else_map = self.chk.find_isinstance_check(e.cond) + if self.chk.options.warn_unreachable: + if if_map is None: + self.msg.redundant_condition_in_if(False, e.cond) + elif else_map is None: + self.msg.redundant_condition_in_if(True, e.cond) if_type = self.analyze_cond_branch(if_map, e.if_expr, context=ctx) diff --git a/mypy/main.py b/mypy/main.py index 14fbee845516..c16d32c9333d 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -463,6 +463,10 @@ def add_invertible_flag(flag: str, help="Warn about returning values of type Any" " from non-Any typed functions", group=lint_group) + add_invertible_flag('--warn-unreachable', default=False, strict_flag=False, + help="Warn about statements or expressions inferred to be" + " unreachable or redundant", + group=lint_group) # Note: this group is intentionally added here even though we don't add # --strict to this group near the end. diff --git a/mypy/messages.py b/mypy/messages.py index 08812acaabcc..9cb279a2d7d0 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1228,6 +1228,35 @@ def note_call(self, subtype: Type, call: Type, context: Context) -> None: self.note('"{}.__call__" has type {}'.format(self.format_bare(subtype), self.format(call, verbosity=1)), context) + def unreachable_statement(self, context: Context) -> None: + self.fail("Statement is unreachable", context) + + def redundant_left_operand(self, op_name: str, context: Context) -> None: + """Indicates that the left operand of a boolean expression is redundant: + it does not change the truth value of the entire condition as a whole. + 'op_name' should either be the string "and" or the string "or". + """ + self.redundant_expr("Left operand of '{}'".format(op_name), op_name == 'and', context) + + def redundant_right_operand(self, op_name: str, context: Context) -> None: + """Indicates that the right operand of a boolean expression is redundant: + it does not change the truth value of the entire condition as a whole. + 'op_name' should either be the string "and" or the string "or". + """ + self.fail("Right operand of '{}' is never evaluated".format(op_name), context) + + def redundant_condition_in_comprehension(self, truthiness: bool, context: Context) -> None: + self.redundant_expr("If condition in comprehension", truthiness, context) + + def redundant_condition_in_if(self, truthiness: bool, context: Context) -> None: + self.redundant_expr("If condition", truthiness, context) + + def redundant_condition_in_assert(self, truthiness: bool, context: Context) -> None: + self.redundant_expr("Condition in assert", truthiness, context) + + def redundant_expr(self, description: str, truthiness: bool, context: Context) -> None: + self.fail("{} is always {}".format(description, str(truthiness).lower()), context) + def report_protocol_problems(self, subtype: Union[Instance, TupleType, TypedDictType], supertype: Instance, context: Context) -> None: """Report possible protocol conflicts between 'subtype' and 'supertype'. diff --git a/mypy/options.py b/mypy/options.py index 68cf3446d5da..6cafca7aa62e 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -48,6 +48,7 @@ class BuildType: "strict_optional_whitelist", "warn_no_return", "warn_return_any", + "warn_unreachable", "warn_unused_ignores", } # type: Final @@ -164,6 +165,10 @@ def __init__(self) -> None: # This makes 1 == '1', 1 in ['1'], and 1 is '1' errors. self.strict_equality = False + # Report an error for any branches inferred to be unreachable as a result of + # type analysis. + self.warn_unreachable = False + # Variable names considered True self.always_true = [] # type: List[str] diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index 593f24523242..e1e99419dc26 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -715,3 +715,251 @@ if sys.version_info[0] >= 2: reveal_type('') # N: Revealed type is 'builtins.str' reveal_type('') # N: Revealed type is 'builtins.str' [builtins fixtures/ops.pyi] + +[case testUnreachableFlagWithBadControlFlow] +# flags: --warn-unreachable +a: int +if isinstance(a, int): + reveal_type(a) # N: Revealed type is 'builtins.int' +else: + reveal_type(a) # E: Statement is unreachable + +b: int +while isinstance(b, int): + reveal_type(b) # N: Revealed type is 'builtins.int' +else: + reveal_type(b) # E: Statement is unreachable + +def foo(c: int) -> None: + reveal_type(c) # N: Revealed type is 'builtins.int' + assert not isinstance(c, int) + reveal_type(c) # E: Statement is unreachable + +d: int +if False: + reveal_type(d) # E: Statement is unreachable + +e: int +if True: + reveal_type(e) # N: Revealed type is 'builtins.int' +else: + reveal_type(e) # E: Statement is unreachable + +[builtins fixtures/isinstancelist.pyi] + +[case testUnreachableFlagStatementAfterReturn] +# flags: --warn-unreachable +def foo(x: int) -> None: + reveal_type(x) # N: Revealed type is 'builtins.int' + return + reveal_type(x) # E: Statement is unreachable + +[case testUnreachableFlagTryBlocks] +# flags: --warn-unreachable + +def foo(x: int) -> int: + try: + reveal_type(x) # N: Revealed type is 'builtins.int' + return x + reveal_type(x) # E: Statement is unreachable + finally: + reveal_type(x) # N: Revealed type is 'builtins.int' + if True: + reveal_type(x) # N: Revealed type is 'builtins.int' + else: + reveal_type(x) # E: Statement is unreachable + +def bar(x: int) -> int: + try: + if True: + raise Exception() + reveal_type(x) # E: Statement is unreachable + except: + reveal_type(x) # N: Revealed type is 'builtins.int' + return x + else: + reveal_type(x) # E: Statement is unreachable + +def baz(x: int) -> int: + try: + reveal_type(x) # N: Revealed type is 'builtins.int' + except: + # Mypy assumes all lines could throw an exception + reveal_type(x) # N: Revealed type is 'builtins.int' + return x + else: + reveal_type(x) # N: Revealed type is 'builtins.int' + return x +[builtins fixtures/exception.pyi] + +[case testUnreachableFlagIgnoresSemanticAnalysisUnreachable] +# flags: --warn-unreachable --python-version 3.7 --platform win32 --always-false FOOBAR +import sys +from typing import TYPE_CHECKING + +x: int +if TYPE_CHECKING: + reveal_type(x) # N: Revealed type is 'builtins.int' +else: + reveal_type(x) + +if not TYPE_CHECKING: + reveal_type(x) +else: + reveal_type(x) # N: Revealed type is 'builtins.int' + +if sys.platform == 'darwin': + reveal_type(x) +else: + reveal_type(x) # N: Revealed type is 'builtins.int' + +if sys.platform == 'win32': + reveal_type(x) # N: Revealed type is 'builtins.int' +else: + reveal_type(x) + +if sys.version_info == (2, 7): + reveal_type(x) +else: + reveal_type(x) # N: Revealed type is 'builtins.int' + +if sys.version_info == (3, 7): + reveal_type(x) # N: Revealed type is 'builtins.int' +else: + reveal_type(x) + +FOOBAR = "" +if FOOBAR: + reveal_type(x) +else: + reveal_type(x) # N: Revealed type is 'builtins.int' +[builtins fixtures/ops.pyi] + +[case testUnreachableFlagIgnoresSemanticAnalysisExprUnreachable] +# flags: --warn-unreachable --always-false FOOBAR +import sys +from typing import TYPE_CHECKING + +FOOBAR = "" + +def foo() -> bool: ... + +lst = [1, 2, 3] + +a = FOOBAR and foo() +b = (not FOOBAR) or foo() +c = 1 if FOOBAR else 2 +d = [x for x in lst if FOOBAR] +[builtins fixtures/list.pyi] + +[case testUnreachableFlagOkWithDeadStatements] +# flags: --warn-unreachable +from typing import NoReturn +def assert_never(x: NoReturn) -> NoReturn: + assert False + +def nonthrowing_assert_never(x: NoReturn) -> None: ... + +def expect_str(x: str) -> str: pass + +x: int +if False: + assert False + reveal_type(x) + +if False: + raise Exception() + reveal_type(x) + +if False: + assert_never(x) + reveal_type(x) + +if False: + nonthrowing_assert_never(x) # E: Statement is unreachable + reveal_type(x) + +if False: + # Ignore obvious type errors + assert_never(expect_str(x)) + reveal_type(x) +[builtins fixtures/exception.pyi] + +[case testUnreachableFlagExpressions] +# flags: --warn-unreachable +def foo() -> bool: ... + +lst = [1, 2, 3, 4] + +a = True or foo() # E: Right operand of 'or' is never evaluated +b = False or foo() # E: Left operand of 'or' is always false +c = True and foo() # E: Left operand of 'and' is always true +d = False and foo() # E: Right operand of 'and' is never evaluated +e = True or (True or (True or foo())) # E: Right operand of 'or' is never evaluated +f = (True or foo()) or (True or foo()) # E: Right operand of 'or' is never evaluated +g = 3 if True else 4 # E: If condition is always true +h = 3 if False else 4 # E: If condition is always false +i = [x for x in lst if True] # E: If condition in comprehension is always true +j = [x for x in lst if False] # E: If condition in comprehension is always false + +k = [x for x in lst if isinstance(x, int) or foo()] # E: If condition in comprehension is always true \ + # E: Right operand of 'or' is never evaluated +[builtins fixtures/isinstancelist.pyi] + +[case testUnreachableFlagMiscTestCaseMissingMethod] +# flags: --warn-unreachable + +class Case1: + def test1(self) -> bool: + return False and self.missing() # E: Right operand of 'and' is never evaluated + + def test2(self) -> bool: + return not self.property_decorator_missing and self.missing() # E: Right operand of 'and' is never evaluated + + def property_decorator_missing(self) -> bool: + return True +[builtins fixtures/bool.pyi] + +[case testUnreachableFlagWithGenerics] +# flags: --warn-unreachable +from typing import TypeVar, Generic + +T1 = TypeVar('T1', bound=int) +T2 = TypeVar('T2', int, str) + +def test1(x: T1) -> T1: + if isinstance(x, int): + reveal_type(x) # N: Revealed type is 'T1`-1' + else: + reveal_type(x) # E: Statement is unreachable + return x + +def test2(x: T2) -> T2: + if isinstance(x, int): + reveal_type(x) # N: Revealed type is 'builtins.int*' + else: + reveal_type(x) # N: Revealed type is 'builtins.str*' + + if False: + # This is unreachable, but we don't report an error, unfortunately. + # The presence of the TypeVar with values unfortunately currently shuts + # down type-checking for this entire function. + # TODO: Find a way of removing this limitation + reveal_type(x) + + return x + +class Test3(Generic[T2]): + x: T2 + + def func(self) -> None: + if isinstance(self.x, int): + reveal_type(self.x) # N: Revealed type is 'builtins.int*' + else: + reveal_type(self.x) # N: Revealed type is 'builtins.str*' + + if False: + # Same issue as above + reveal_type(self.x) + +[builtins fixtures/isinstancelist.pyi]