Add flag to warn about unreachable branches and exprs

This diff adds a `--disallow-inferred-unreachable` flag that
reports an error if:

1. Any branch is inferred to be unreachable
2. Any subexpression in boolean expressions, inline if statements,
   and list/set/generator/dict comprehensions are always unreachable
   or redundant.

This only takes into account the results of *type* analysis. We do not
report errors for branches that are inferred to be unreachable in the
semantic analysis phase (e.g. due to `sys.platform` checks and the
like).

Those checks are all intentional/deliberately flag certain branches
as unreachable, so error messages would be annoying in those cases.

A few additional notes:

1. I didn't spend a huge amount of time trying to come up with error
   messages. They could probably do with some polishing/rewording.

2. I thought about enabling this flag for default with mypy, but
   unfortunately that ended up producing ~40 to 50 (apparently
   legitimate) error messages.

   About a fourth of them were due to runtime checks checking to see
   if some type is None (despite not being declared to be Optional),
   about a fourth seemed to be due to mypy not quite understanding how
   to handle things like traits and Bogus[...], and about a fourth
   seemed to be legitimately unnecessary checks we could safely remove.

   The final checks were a mixture of typeshed bugs and misc errors with
   the reachability checks. (e.g. see python#7048)

3. For these reasons, I decided against adding this flag to `--strict`
   and against documenting it yet. We'll probably need to flush out
   a few bugs in mypy first/get mypy to the state where we can at least
   honestly dogfood this.

Resolves python#2395; partially
addresses python#7008.

mypy/checker.py

@@ -1777,13 +1777,36 @@ 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.disallow_inferred_unreachable and not self.is_raising_or_empty(s):
+                    self.msg.unreachable_branch(s)
                 break
             self.accept(s)
 
+    def is_raising_or_empty(self, s: Statement) -> bool:
+        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 +2979,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.

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.disallow_inferred_unreachable:
+            if left_map is None:
+                self.msg.always_same_truth_value_left_operand(e.op, e.left)
+            if right_map is None:
+                self.msg.unreachable_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,19 +3185,30 @@ 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.disallow_inferred_unreachable:
+                    if true_map is None:
+                        self.msg.comprehension_cond_always_same(False, condition)
+                    elif false_map is None:
+                        self.msg.comprehension_cond_always_same(True, condition)
+
     def visit_conditional_expr(self, e: ConditionalExpr) -> Type:
         self.accept(e.cond)
         ctx = self.type_context[-1]
 
         # 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.disallow_inferred_unreachable:
+            if if_map is None:
+                self.msg.unreachable_branch_in_inline_if('if', e.cond)
+            elif else_map is None:
+                self.msg.unreachable_branch_in_inline_if('else', e.cond)
 
         if_type = self.analyze_cond_branch(if_map, e.if_expr, context=ctx)
 

mypy/main.py

@@ -492,6 +492,10 @@ def add_invertible_flag(flag: str,
                         help="Treat imports as private unless aliased",
                         group=strictness_group)
 
+    add_invertible_flag('--disallow-inferred-unreachable', default=False, strict_flag=False,
+                        help="Disallow branches inferred to be unreachable after type analysis",
+                        group=strictness_group)
+
     incremental_group = parser.add_argument_group(
         title='Incremental mode',
         description="Adjust how mypy incrementally type checks and caches modules. "

mypy/messages.py

@@ -1228,6 +1228,30 @@ 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_branch(self, context: Context) -> None:
+        self.fail("This branch is inferred to be unreachable", context)
+
+    def always_same_truth_value_left_operand(self, op_name: str, context: Context) -> None:
+        value = 'true' if op_name == 'and' else 'false'
+        self.fail(
+            "The left operand of this '{}' expression is always {}".format(op_name, value),
+            context,
+        )
+
+    def unreachable_right_operand(self, op_name: str, context: Context) -> None:
+        self.fail(
+            "The right operand of this '{}' expression is never evaluated".format(op_name),
+            context,
+        )
+
+    def comprehension_cond_always_same(self, simplified_result: bool, context: Context) -> None:
+        template = "The conditional check in this comprehension is always {}"
+        self.fail(template.format(str(simplified_result).lower()), context)
+
+    def unreachable_branch_in_inline_if(self, sub_expr_name: str, context: Context) -> None:
+        template = "The '{}' expression in this inline if expression is never evaluated"
+        self.fail(template.format(sub_expr_name), context)
+
     def report_protocol_problems(self, subtype: Union[Instance, TupleType, TypedDictType],
                                  supertype: Instance, context: Context) -> None:
         """Report possible protocol conflicts between 'subtype' and 'supertype'.

mypy/options.py

@@ -31,6 +31,7 @@ class BuildType:
     "disallow_any_generics",
     "disallow_any_unimported",
     "disallow_incomplete_defs",
+    "disallow_inferred_unreachable",
     "disallow_subclassing_any",
     "disallow_untyped_calls",
     "disallow_untyped_decorators",
@@ -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.disallow_inferred_unreachable = False
+
         # Variable names considered True
         self.always_true = []  # type: List[str]
 

test-data/unit/check-unreachable-code.test

@@ -715,3 +715,179 @@ 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: --disallow-inferred-unreachable
+a: int
+if isinstance(a, int):
+    reveal_type(a)  # N: Revealed type is 'builtins.int'
+else:
+    reveal_type(a)  # E: This branch is inferred to be unreachable
+
+b: int
+while isinstance(b, int):
+    reveal_type(b)  # N: Revealed type is 'builtins.int'
+else:
+    reveal_type(b)  # E: This branch is inferred to be unreachable
+
+def foo(c: int) -> None:
+    reveal_type(c)  # N: Revealed type is 'builtins.int'
+    assert not isinstance(c, int)
+    reveal_type(c)  # E: This branch is inferred to be unreachable
+
+d: int
+if False:
+    reveal_type(d)  # E: This branch is inferred to be unreachable
+
+e: int
+if True:
+    reveal_type(e)  # N: Revealed type is 'builtins.int'
+else:
+    reveal_type(e)  # E: This branch is inferred to be unreachable
+
+[builtins fixtures/isinstancelist.pyi]
+
+[case testUnreachableFlagTryBlocks]
+# flags: --disallow-inferred-unreachable
+
+def foo(x: int) -> int:
+    try:
+        reveal_type(x)      # N: Revealed type is 'builtins.int'
+        return x
+        reveal_type(x)      # E: This branch is inferred to be 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: This branch is inferred to be unreachable
+
+def bar(x: int) -> int:
+    try:
+        if True:
+            raise Exception()
+        reveal_type(x)      # E: This branch is inferred to be unreachable
+    except:
+        reveal_type(x)      # N: Revealed type is 'builtins.int'
+        return x
+    else:
+        reveal_type(x)      # E: This branch is inferred to be 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: --disallow-inferred-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'
+
+if sys.platform == 'darwin':
+    reveal_type(x)
+
+if sys.version_info == (2, 7):
+    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: --disallow-inferred-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: --disallow-inferred-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: This branch is inferred to be 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: --disallow-inferred-unreachable
+def foo() -> bool: ...
+
+lst = [1, 2, 3, 4]
+
+a = True or foo()                        # E: The right operand of this 'or' expression is never evaluated
+b = False or foo()                       # E: The left operand of this 'or' expression is always false
+c = True and foo()                       # E: The left operand of this 'and' expression is always true
+d = False and foo()                      # E: The right operand of this 'and' expression is never evaluated
+e = True or (True or (True or foo()))    # E: The right operand of this 'or' expression is never evaluated
+f = (True or foo()) or (True or foo())   # E: The right operand of this 'or' expression is never evaluated
+g = 3 if True else 4                     # E: The 'else' expression in this inline if expression is never evaluated
+h = 3 if False else 4                    # E: The 'if' expression in this inline if expression is never evaluated
+i = [x for x in lst if True]             # E: The conditional check in this comprehension is always true
+j = [x for x in lst if False]            # E: The conditional check in this comprehension is always false
+
+k = [x for x in lst if isinstance(x, int) or foo()]  # E: The conditional check in this comprehension is always true \
+                                                     # E: The right operand of this 'or' expression is never evaluated
+[builtins fixtures/isinstancelist.pyi]
+
+[case testUnreachableFlagMiscTestCaseMissingMethod]
+# flags: --disallow-inferred-unreachable
+
+class Case1:
+    def test1(self) -> bool:
+        return False and self.missing()  # E: The right operand of this 'and' expression is never evaluated
+
+    def test2(self) -> bool:
+        return not self.property_decorator_missing and self.missing()  # E: The right operand of this 'and' expression is never evaluated
+
+    def property_decorator_missing(self) -> bool:
+        return True
+[builtins fixtures/bool.pyi]