Fail when conditioning on implicit bool

Author: ikonst

docs/source/error_code_list.rst

@@ -665,6 +665,24 @@ consistently when using the call-based syntax. Example:
     # Error: First argument to namedtuple() should be "Point2D", not "Point"
     Point2D = NamedTuple("Point", [("x", int), ("y", int)])
 
+
+
+Implicitly converting to bool always evaluates as True [implicit-bool]
+----------------------------------------------------------------------
+
+In contexts where an expression has to be converted to a bool, an object
+which does not implement __bool__ nor __len__ will always evaluate to True.
+
+.. code-block:: python
+
+    class Foo:
+      pass
+
+    foo = Foo()
+    # Error: "{}" does not implement __bool__ or __len__ so the expression is always truthy
+    if foo:
+       ...
+
 Report syntax errors [syntax]
 -----------------------------
 

mypy/checker.py

@@ -51,7 +51,7 @@
     try_getting_str_literals_from_type, try_getting_int_literals_from_type,
     tuple_fallback, is_singleton_type, try_expanding_enum_to_union,
     true_only, false_only, function_type, get_type_vars, custom_special_method,
-    is_literal_type_like,
+    is_literal_type_like, type_has_explicit_conversion_to_bool,
 )
 from mypy import message_registry
 from mypy.subtypes import (
@@ -3240,6 +3240,14 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
                 if self.in_checked_function():
                     self.fail(message_registry.RETURN_VALUE_EXPECTED, s, code=codes.RETURN_VALUE)
 
+    def _type_has_explicit_conversion_to_bool(self, t: Type) -> bool:
+        if isinstance(t, Instance):
+            return t.type.has_readable_member('__bool__') or t.type.has_readable_member('__len__')
+        elif isinstance(t, FunctionLike):
+            return False
+        else:
+            return True
+
     def visit_if_stmt(self, s: IfStmt) -> None:
         """Type check an if statement."""
         # This frame records the knowledge from previous if/elif clauses not being taken.
@@ -3251,6 +3259,14 @@ def visit_if_stmt(self, s: IfStmt) -> None:
                 if isinstance(t, DeletedType):
                     self.msg.deleted_as_rvalue(t, s)
 
+                if codes.IMPLICIT_BOOL in self.options.enabled_error_codes:
+                    if isinstance(t, (Instance, FunctionLike)):
+                        if not type_has_explicit_conversion_to_bool(t):
+                            self.msg.type_converts_to_bool_implicitly(t, e)
+                    elif isinstance(t, UnionType):
+                        if not any(type_has_explicit_conversion_to_bool(item) for item in t.items):
+                            self.msg.type_union_converts_to_bool_implicitly(t, e)
+
                 if_map, else_map = self.find_isinstance_check(e)
 
                 # XXX Issue a warning if condition is always False?

mypy/checkexpr.py

@@ -64,7 +64,7 @@
 from mypy.typeops import (
     tuple_fallback, make_simplified_union, true_only, false_only, erase_to_union_or_bound,
     function_type, callable_type, try_getting_str_literals, custom_special_method,
-    is_literal_type_like,
+    is_literal_type_like, type_has_explicit_conversion_to_bool,
 )
 import mypy.errorcodes as codes
 
@@ -3819,7 +3819,7 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No
                         self.msg.redundant_condition_in_comprehension(True, condition)
 
     def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = False) -> Type:
-        self.accept(e.cond)
+        cond_type = self.accept(e.cond)
         ctx = self.type_context[-1]
 
         # Gain type information from isinstance if it is there
@@ -3831,6 +3831,16 @@ def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = F
             elif else_map is None:
                 self.msg.redundant_condition_in_if(True, e.cond)
 
+        if codes.IMPLICIT_BOOL in self.chk.options.enabled_error_codes:
+            cond_proper_type = get_proper_type(cond_type)
+
+            if isinstance(cond_proper_type, (Instance, FunctionLike)):
+                if not type_has_explicit_conversion_to_bool(cond_proper_type):
+                    self.msg.type_converts_to_bool_implicitly(cond_proper_type, e.cond)
+            elif isinstance(cond_proper_type, UnionType):
+                if not any(type_has_explicit_conversion_to_bool(item) for item in cond_proper_type.items):
+                    self.msg.type_union_converts_to_bool_implicitly(cond_proper_type, e.cond)
+
         if_type = self.analyze_cond_branch(if_map, e.if_expr, context=ctx,
                                            allow_none_return=allow_none_return)
 

mypy/errorcodes.py

@@ -92,6 +92,10 @@ def __str__(self) -> str:
     'General')  # type: Final
 EXIT_RETURN = ErrorCode(
     'exit-return', "Warn about too general return type for '__exit__'", 'General')  # type: Final
+NAME_MATCH = ErrorCode(
+    'name-match', "Check that type definition has consistent naming", 'General')  # type: Final
+IMPLICIT_BOOL = ErrorCode(
+    'implicit-bool', "Implicitly converting to bool always evaluates as True", 'General')  # type: Final
 
 # These error codes aren't enabled by default.
 NO_UNTYPED_DEF = ErrorCode(
@@ -117,8 +121,6 @@ def __str__(self) -> str:
     "Warn about redundant expressions",
     'General',
     default_enabled=False)  # type: Final
-NAME_MATCH = ErrorCode(
-    'name-match', "Check that type definition has consistent naming", 'General')  # type: Final
 
 
 # Syntax errors are often blocking.

mypy/message_registry.py

@@ -84,6 +84,8 @@
 DESCRIPTOR_SET_NOT_CALLABLE = "{}.__set__ is not callable"  # type: Final
 DESCRIPTOR_GET_NOT_CALLABLE = "{}.__get__ is not callable"  # type: Final
 MODULE_LEVEL_GETATTRIBUTE = '__getattribute__ is not valid at the module level'  # type: Final
+TYPE_CONVERTS_TO_BOOL_IMPLICITLY = '"{}" does not implement __bool__ or __len__ so the expression is always truthy'  # type: Final
+ALL_TYPES_IN_UNION_CONVERT_TO_BOOL_IMPLICITLY = 'Neither of {} implements __bool__ or __len__ so the expression is always truthy'  # type: Final
 
 # Generic
 GENERIC_INSTANCE_VAR_CLASS_ACCESS = \

mypy/messages.py

@@ -718,6 +718,12 @@ def deleted_as_lvalue(self, typ: DeletedType, context: Context) -> None:
             s = ' "{}"'.format(typ.source)
         self.fail('Assignment to variable{} outside except: block'.format(s), context)
 
+    def type_converts_to_bool_implicitly(self, typ: Type, context: Context):
+        return self.fail(message_registry.TYPE_CONVERTS_TO_BOOL_IMPLICITLY.format(typ), context, code=codes.IMPLICIT_BOOL)
+
+    def type_union_converts_to_bool_implicitly(self, typ: UnionType, context: Context):
+        return self.fail(message_registry.ALL_TYPES_IN_UNION_CONVERT_TO_BOOL_IMPLICITLY.format(typ), context, code=codes.IMPLICIT_BOOL)
+
     def no_variant_matches_arguments(self,
                                      plausible_targets: List[CallableType],
                                      overload: Overloaded,

mypy/test/helpers.py

@@ -8,6 +8,7 @@
 from typing import List, Iterable, Dict, Tuple, Callable, Any, Optional, Iterator
 
 from mypy import defaults
+from mypy import errorcodes as codes
 import mypy.api as api
 
 import pytest
@@ -372,16 +373,19 @@ def assert_type(typ: type, value: object) -> None:
 def parse_options(program_text: str, testcase: DataDrivenTestCase,
                   incremental_step: int) -> Options:
     """Parse comments like '# flags: --foo' in a test case."""
-    options = Options()
     flags = re.search('# flags: (.*)$', program_text, flags=re.MULTILINE)
     if incremental_step > 1:
         flags2 = re.search('# flags{}: (.*)$'.format(incremental_step), program_text,
                            flags=re.MULTILINE)
         if flags2:
             flags = flags2
 
+    disabled_error_codes = {codes.IMPLICIT_BOOL}
+
     if flags:
         flag_list = flags.group(1).split()
+        for code in disabled_error_codes:
+            flag_list.extend(['--disable-error-code', code.code])
         flag_list.append('--no-site-packages')  # the tests shouldn't need an installed Python
         targets, options = process_options(flag_list, require_targets=False)
         if targets:
@@ -393,6 +397,7 @@ def parse_options(program_text: str, testcase: DataDrivenTestCase,
         # TODO: Enable strict optional in test cases by default (requires *many* test case changes)
         options.strict_optional = False
         options.error_summary = False
+        options.disabled_error_codes = disabled_error_codes
 
     # Allow custom python version to override testcase_pyversion.
     if all(flag.split('=')[0] not in ['--python-version', '-2', '--py2'] for flag in flag_list):

mypy/typeops.py

@@ -642,6 +642,15 @@ def get_enum_values(typ: Instance) -> List[str]:
     return [name for name, sym in typ.type.names.items() if isinstance(sym.node, Var)]
 
 
+def type_has_explicit_conversion_to_bool(typ: Type) -> bool:
+    if isinstance(typ, Instance):
+        return typ.type.has_readable_member('__bool__') or typ.type.has_readable_member('__len__')
+    elif isinstance(typ, FunctionLike):
+        return False
+    else:
+        return True
+
+
 def is_singleton_type(typ: Type) -> bool:
     """Returns 'true' if this type is a "singleton type" -- if there exists
     exactly only one runtime value associated with this type.

test-data/unit/check-errorcodes.test

@@ -807,3 +807,61 @@ from typing_extensions import TypedDict
 
 Foo = TypedDict("Bar", {})  # E: First argument "Bar" to TypedDict() does not match variable name "Foo"  [name-match]
 [builtins fixtures/dict.pyi]
+[case testImplicitBool]
+# flags: --enable-error-code implicit-bool
+from typing import List, Union
+
+class Foo:
+    pass
+
+
+foo = Foo()
+if foo:  # E: "__main__.Foo" does not implement __bool__ or __len__ so the expression is always truthy  [implicit-bool]
+    pass
+
+zero = 0
+if zero:
+    pass
+
+
+false = False
+if false:
+    pass
+
+
+null = None
+if null:
+    pass
+
+
+s = ''
+if s:
+    pass
+
+
+good_union: Union[str, int] = 5
+if good_union:
+    pass
+
+
+bad_union: Union[Foo, object] = Foo()
+if bad_union:  # E: Neither of Union[__main__.Foo, builtins.object] implements __bool__ or __len__ so the expression is always truthy  [implicit-bool]
+    pass
+
+
+def f():
+    pass
+
+
+if f:  # E: "def () -> Any" does not implement __bool__ or __len__ so the expression is always truthy  [implicit-bool]
+    pass
+
+
+lst: List[int] = []
+if lst:
+    pass
+
+
+conditional_result = 'foo' if f else 'bar'  # E: "def () -> Any" does not implement __bool__ or __len__ so the expression is always truthy  [implicit-bool]
+
+[builtins fixtures/implicit_bool.pyi]

test-data/unit/fixtures/implicit_bool.pyi

@@ -0,0 +1,16 @@
+from typing import Generic, Sequence, TypeVar
+
+_T = TypeVar('_T')
+
+
+class object:
+    def __init__(self): pass
+
+class type: pass
+class int:
+    def __bool__(self) -> bool: pass
+class bool(int): pass
+class list(Generic[_T], Sequence[_T]):
+    def __len__(self) -> int: pass
+class str:
+    def __len__(self) -> int: pass