Skip to content

Commit

Permalink
Fail when conditioning on implicit bool
Browse files Browse the repository at this point in the history
  • Loading branch information
ikonst committed May 30, 2021
1 parent 61c3462 commit e96ce45
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 6 deletions.
18 changes: 18 additions & 0 deletions docs/source/error_code_list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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]
-----------------------------

Expand Down
18 changes: 17 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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.
Expand All @@ -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?
Expand Down
15 changes: 13 additions & 2 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -3831,6 +3831,17 @@ 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)

Expand Down
7 changes: 5 additions & 2 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ 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(
Expand All @@ -117,8 +122,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.
Expand Down
5 changes: 5 additions & 0 deletions mypy/message_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@
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 = \
Expand Down
8 changes: 8 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,14 @@ 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,
Expand Down
7 changes: 6 additions & 1 deletion mypy/test/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down
9 changes: 9 additions & 0 deletions mypy/typeops.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
58 changes: 58 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
16 changes: 16 additions & 0 deletions test-data/unit/fixtures/implicit_bool.pyi
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e96ce45

Please sign in to comment.