Repository: quora/pyanalyze
Pull request #443 (2022-01-26 05:05:36)

Infer `NoReturn` instead of `Any` for unreachable code


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 92acce8..6043345 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Infer `NoReturn` instead of `Any` for unreachable code (#443)
 - Make `NoReturn` compatible with all other types (#442)
 - Fix treatment of walrus operator in `and`, `or`, and `if/else`
   expressions (#441)
diff --git a/pyanalyze/boolability.py b/pyanalyze/boolability.py
index e1790e9..5a85abb 100644
--- a/pyanalyze/boolability.py
+++ b/pyanalyze/boolability.py
@@ -80,10 +80,13 @@ def get_boolability(value: Value) -> Boolability:
             # If it contains both values that are always true and values that are always false,
             # it's boolable.
             return Boolability.boolable
-        else:
+        elif boolabilities:
             # This means the set contains either only truthy or only falsy options.
             # Choose the lowest-valued (and therefore weakest) one.
             return min(boolabilities, key=lambda b: b.value)
+        else:
+            # NO_RETURN_VALUE
+            return Boolability.boolable
     else:
         return _get_boolability_no_mvv(value)
 
diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py
index 54a11c5..814cede 100644
--- a/pyanalyze/name_check_visitor.py
+++ b/pyanalyze/name_check_visitor.py
@@ -937,6 +937,7 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
     current_enum_members: Optional[Dict[object, str]]
     _name_node_to_statement: Optional[Dict[ast.AST, Optional[ast.AST]]]
     import_name_to_node: Dict[str, Union[ast.Import, ast.ImportFrom]]
+    expected_return_value: Optional[Value]
 
     def __init__(
         self,
@@ -1797,8 +1798,10 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
         params: Sequence[SigParameter],
     ) -> FunctionResult:
         # Ignore generators for now.
-        if isinstance(return_set, AnyValue) or (
-            self.is_generator and info.async_kind is not AsyncFunctionKind.normal
+        if (
+            isinstance(return_set, AnyValue)
+            or return_set is NO_RETURN_VALUE
+            or (self.is_generator and info.async_kind is not AsyncFunctionKind.normal)
         ):
             has_return = True
         elif return_set is UNINITIALIZED_VALUE:
@@ -3089,11 +3092,12 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
             value = KnownNone
         else:
             value = self.visit(node.value)
-        if value is NO_RETURN_VALUE:
-            return
         self.return_values.append(value)
         self._set_name_in_scope(LEAVES_SCOPE, node, AnyValue(AnySource.marker))
-        if self.expected_return_value is NO_RETURN_VALUE:
+        if (
+            self.expected_return_value is NO_RETURN_VALUE
+            and value is not NO_RETURN_VALUE
+        ):
             self._show_error_if_checking(
                 node, error_code=ErrorCode.no_return_may_return
             )
@@ -3111,7 +3115,11 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
                     error_code=ErrorCode.incompatible_return_value,
                     detail=tv_map.display(),
                 )
-        if self.expected_return_value == KnownNone and value != KnownNone:
+        if (
+            self.expected_return_value == KnownNone
+            and value != KnownNone
+            and value is not NO_RETURN_VALUE
+        ):
             self._show_error_if_checking(
                 node,
                 "Function declared as returning None may not return a value",
@@ -3907,7 +3915,7 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
             if vals:
                 return unite_values(*vals)
             else:
-                return UNINITIALIZED_VALUE
+                return NO_RETURN_VALUE
         return local_value
 
     def visit_Attribute(self, node: ast.Attribute) -> Value:
diff --git a/pyanalyze/node_visitor.py b/pyanalyze/node_visitor.py
index 97f47ca..3ab3369 100644
--- a/pyanalyze/node_visitor.py
+++ b/pyanalyze/node_visitor.py
@@ -165,6 +165,7 @@ class BaseNodeVisitor(ast.NodeVisitor):
     default_module: Optional[ModuleType] = None  # module to run on by default
     # whether to look at FILE_ENVIRON_KEY to find files to run on
     should_check_environ_for_files: bool = True
+    caught_errors: Optional[List[Dict[str, Any]]] = None
 
     _changes_for_fixer = collections.defaultdict(list)
 
diff --git a/pyanalyze/stacked_scopes.py b/pyanalyze/stacked_scopes.py
index 295c4f8..f59df03 100644
--- a/pyanalyze/stacked_scopes.py
+++ b/pyanalyze/stacked_scopes.py
@@ -368,7 +368,7 @@ class Constraint(AbstractConstraint):
                         yield TypedValue(self.value)
                     # TODO: Technically here we should infer an intersection type:
                     # a type that is a subclass of both types. In practice currently
-                    # _constrain_values() will eventually return AnyValue.
+                    # _constrain_value() will eventually return NoReturn.
                 else:
                     if not safe_issubclass(inner_value.typ, self.value):
                         yield value
@@ -1453,20 +1453,21 @@ def uniq_chain(iterables: Iterable[Iterable[T]]) -> List[T]:
 
 
 def _constrain_value(
-    values: Iterable[Value],
+    values: Sequence[Value],
     constraints: Iterable[Constraint],
     *,
     fallback_value: Optional[Value] = None,
     simplification_limit: Optional[int] = None,
 ) -> Value:
     # Flatten MultiValuedValue so that we can apply constraints.
-    values = [val for val_or_mvv in values for val in flatten_values(val_or_mvv)]
     if not values and fallback_value is not None:
         values = list(flatten_values(fallback_value))
+    else:
+        values = [val for val_or_mvv in values for val in flatten_values(val_or_mvv)]
     for constraint in constraints:
         values = list(constraint.apply_to_values(values))
     if not values:
-        return AnyValue(AnySource.unreachable)
+        return NO_RETURN_VALUE
     if simplification_limit is not None:
         return unite_and_simplify(*values, limit=simplification_limit)
     return unite_values(*values)
diff --git a/pyanalyze/value.py b/pyanalyze/value.py
index 234de31..2d659b6 100644
--- a/pyanalyze/value.py
+++ b/pyanalyze/value.py
@@ -1332,6 +1332,8 @@ class MultiValuedValue(Value):
         if isinstance(other, TypeVarValue):
             other = other.get_fallback_value()
         if is_union(other):
+            if other is NO_RETURN_VALUE:
+                return {}
             tv_maps = []
             for val in flatten_values(other):
                 tv_map = self.can_assign(val, ctx)


Test patch:
diff --git a/pyanalyze/test_annotations.py b/pyanalyze/test_annotations.py
index 62fc973..45fb7ef 100644
--- a/pyanalyze/test_annotations.py
+++ b/pyanalyze/test_annotations.py
@@ -1210,10 +1210,10 @@ class TestCustomCheck(TestNameCheckVisitorBase):
 
             lst = []
             for x in lst:
-                assert_is_value(x, AnyValue(AnySource.unreachable))
+                assert_is_value(x, MultiValuedValue([]))
                 shallow(x)
                 deep(x)
-                none_at_all(x)  # E: incompatible_argument
+                none_at_all(x)
 
     @assert_passes()
     def test_not_none(self) -> None:
diff --git a/pyanalyze/test_boolability.py b/pyanalyze/test_boolability.py
index 53639b4..2d69682 100644
--- a/pyanalyze/test_boolability.py
+++ b/pyanalyze/test_boolability.py
@@ -4,6 +4,7 @@ from asynq.futures import FutureBase
 from .boolability import Boolability, get_boolability
 from .stacked_scopes import Composite
 from .value import (
+    NO_RETURN_VALUE,
     AnnotatedValue,
     AnySource,
     AnyValue,
@@ -114,6 +115,7 @@ def test_get_boolability() -> None:
     assert Boolability.value_always_false == get_boolability(
         KnownValue(False) | KnownValue("")
     )
+    assert Boolability.boolable == get_boolability(NO_RETURN_VALUE)
 
 
 class TestAssert(TestNameCheckVisitorBase):
diff --git a/pyanalyze/test_name_check_visitor.py b/pyanalyze/test_name_check_visitor.py
index fe24002..a577a3a 100644
--- a/pyanalyze/test_name_check_visitor.py
+++ b/pyanalyze/test_name_check_visitor.py
@@ -1972,7 +1972,7 @@ class TestUnpacking(TestNameCheckVisitorBase):
             assert_is_value(s, SequenceIncompleteValue(list, []))
 
             for sprime in []:
-                assert_is_value(sprime, AnyValue(AnySource.unreachable))
+                assert_is_value(sprime, NO_RETURN_VALUE)
 
             for t, u in []:
                 assert_is_value(t, AnyValue(AnySource.unreachable))
diff --git a/pyanalyze/test_patma.py b/pyanalyze/test_patma.py
index 25f1128..a839b48 100644
--- a/pyanalyze/test_patma.py
+++ b/pyanalyze/test_patma.py
@@ -27,9 +27,9 @@ class TestPatma(TestNameCheckVisitorBase):
             def capybara(x: int):
                 match x:
                     case None:  # E: impossible_pattern
-                        assert_is_value(x, AnyValue(AnySource.unreachable))
+                        assert_is_value(x, NO_RETURN_VALUE)
                     case "x":  # E: impossible_pattern
-                        assert_is_value(x, AnyValue(AnySource.unreachable))
+                        assert_is_value(x, NO_RETURN_VALUE)
                     case 3:
                         assert_is_value(x, KnownValue(3))
                     case _ if x == 4:
diff --git a/pyanalyze/test_stacked_scopes.py b/pyanalyze/test_stacked_scopes.py
index 27e9d4f..97fc954 100644
--- a/pyanalyze/test_stacked_scopes.py
+++ b/pyanalyze/test_stacked_scopes.py
@@ -5,6 +5,7 @@ from .stacked_scopes import ScopeType, uniq_chain
 from .test_name_check_visitor import TestNameCheckVisitorBase
 from .test_node_visitor import assert_passes
 from .value import (
+    NO_RETURN_VALUE,
     AnnotatedValue,
     AnySource,
     AnyValue,
@@ -888,8 +889,8 @@ class TestConstraints(TestNameCheckVisitorBase):
                 if isinstance(x, B):
                     assert_is_value(x, TypedValue(B))
                     if isinstance(x, C):
-                        # Incompatible constraints result in Any.
-                        assert_is_value(x, AnyValue(AnySource.unreachable))
+                        # Incompatible constraints result in NoReturn.
+                        assert_is_value(x, NO_RETURN_VALUE)
             if isinstance(x, B):
                 assert_is_value(x, TypedValue(B))
                 if isinstance(x, A):
@@ -1647,6 +1648,19 @@ class TestComposite(TestNameCheckVisitorBase):
                 x = self.x
                 assert_is_value(x, TypedValue(int))
 
+    @assert_passes()
+    def test_attribute_to_never(self):
+        from typing import Union
+
+        class TypedValue:
+            typ: Union[type, str]
+
+            def get_generic_args_for_type(self) -> object:
+                if isinstance(self.typ, super):
+                    return self.typ.__self_class__
+                else:
+                    assert False
+
     @assert_passes()
     def test_constraint(self):
         class Capybara(object):