8000 Check expr that are implicitly true for lack dunder bool/len by ikonst · Pull Request #10666 · python/mypy · GitHub
[go: up one dir, main page]

Skip to content

Check expr that are implicitly true for lack dunder bool/len #10666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 33 commits into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
1bc2b9d
wip
ikonst May 29, 2021
b3b301f
fix fixtures and tests
ikonst Jun 18, 2021
76bd3e2
Undo unneeded test change
ikonst Jun 25, 2021
801f274
Let’s try to just warn in boolean context (no tests yet)
ikonst Jun 25, 2021
cbcd9d9
disable for non-strict optional + message
ikonst Jun 25, 2021
c58bb6a
futile attempt to improve _format_expr_name
ikonst Jun 25, 2021
6e497bb
kick CI
ikonst Jun 25, 2021
f0a5081
even more futile attempts on _format_expr_name
ikonst Jun 25, 2021
12278df
add error code
ikonst Jun 25, 2021
bcf8706
docs
ikonst Jun 25, 2021
050bd67
fix type_guard support
ikonst Jun 25, 2021
d2851a3
fix _format_expr_name sig
ikonst Jun 25, 2021
6f0951d
revert change to testConditionalBoolLiteralUnionNarrowing
ikonst Jun 25, 2021
dc70b7c
handle FunctionLikes + re-add test
ikonst Jun 25, 2021
c68df2f
fix list fixture
ikonst Jun 25, 2021
f346448
fix typing
ikonst Jun 26, 2021
903d72b
Merge remote-tracking branch 'origin/master' into check-always-true
ikonst Jun 30, 2021
d36a54c
fix type errors
ikonst Jun 30, 2021
b9dd548
fix TypeFixture
ikonst Jun 30, 2021
212af09
fix sphinx syntax
ikonst Jun 30, 2021
246d065
restructure _check_for_truthy_type
ikonst Jun 30, 2021
9e19e85
Rewrite implicit_bool short description
ikonst Jun 30, 2021
1753f2c
Remove unused _is_truthy_instance
ikonst Jul 1, 2021
e60082d
why did I ever change this code?
ikonst Jul 1, 2021
250c19b
testImplicitBool: clean up newlines
ikonst Aug 5, 2021
7166b54
Revert most of the changes to test fixtures
ikonst Aug 5, 2021
9349eef
Merge remote-tracking branch 'origin/master' into check-always-true
ikonst Aug 5, 2021
d432220
confused but removing timedelta.__bool__
ikonst Aug 5, 2021
6267456
Apply suggestions from code review
hauntsaninja Aug 5, 2021
31977ab
Update test-data/unit/check-errorcodes.test
hauntsaninja Aug 5, 2021
5c6d763
more copy changes
Aug 8, 2021
13af1eb
add a test
Aug 8, 2021
338e35b
Try to provide more color in the docs
ikonst Sep 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,45 @@ mypy generates an error if it thinks that an expression is redundant.

# Error: If condition in comprehension is always true [redundant-expr]
[i for i in range(x) if isinstance(i, int)]


Check that expression is not implicitly true in boolean context [truthy-bool]
-----------------------------------------------------------------------------

Warn when an expression whose type does not implement ``__bool__`` or ``__len__`` is used in boolean context,
since unless implemented by a sub-type, the expression will always evaluate to true.

.. code-block:: python

# mypy: enable-error-code truthy-bool

class Foo:
pass
foo = Foo()
# Error: "foo" has type "Foo" which does not implement __bool__ or __len__ so it could always be true in boolean context
if foo:
...


This check might falsely imply an error. For example, ``typing.Iterable`` does not implement
``__len__`` and so this code will be flagged:

.. code-block:: python

# mypy: enable-error-code truthy-bool

def transform(items: Iterable[int]) -> List[int]:
# Error: "items" has type "typing.Iterable[int]" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool]
if not items:
return [42]
return [x + 1 for x in items]



If called as ``transform((int(s) for s in []))``, this function would not return ``[42]]`` unlike what the author
might have intended. Of course it's possible that ``transform`` is only passed ``list`` objects, and so there is
no error in practice. In such case, it might be prudent to annotate ``items: typing.Sequence[int]``.

This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``typing.Sized``),
except that attempting to invoke an undefined method (e.g. ``__len__``) results in an error,
while attempting to evaluate an object in boolean context without a concrete implementation results in a truthy value.
59 changes: 59 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3987,6 +3987,63 @@ def conditional_callable_type_map(self, expr: Expression,

return None, {}

def _is_truthy_type(self, t: ProperType) -> bool:
return (
(
isinstance(t, Instance) and
bool(t.type) and
not t.type.has_readable_member('__bool__') and
not t.type.has_readable_member('__len__')
)
or isinstance(t, FunctionLike)
or (
isinstance(t, UnionType) and
all(self._is_truthy_type(t) for t in get_proper_types(t.items))
)
)

def _check_for_truthy_type(self, t: Type, expr: Expression) -> None:
if not state.strict_optional:
return # if everything can be None, all bets are off

t = get_proper_type(t)
if not self._is_truthy_type(t):
return

def format_expr_type() -> str:
if isinstance(expr, MemberExpr):
return f'Member "{expr.name}" has type "{t}"'
elif isinstance(expr, RefExpr) and expr.fullname:
return f'"{expr.fullname}" has type "{t}"'
elif isinstance(expr, CallExpr):
if isinstance(expr.callee, MemberExpr):
return f'"{expr.callee.name}" returns "{t}"'
elif isinstance(expr.callee, RefExpr) and expr.callee.fullname:
return f'"{expr.callee.fullname}" returns "{t}"'
return f'Call returns "{t}"'
else:
return f'Expression has type "{t}"'

if isinstance(t, FunctionLike):
self.msg.fail(
f'Function "{t}" could always be true in boolean context', expr,
code=codes.TRUTHY_BOOL,
)
elif isinstance(t, UnionType):
self.msg.fail(
f"{format_expr_type()} of which no members implement __bool__ or __len__ "
"so it could always be true in boolean context",
expr,
code=codes.TRUTHY_BOOL,
)
else:
self.msg.fail(
f'{format_expr_type()} which does not implement __bool__ or __len__ '
'so it could always be true in boolean context',
expr,
code=codes.TRUTHY_BOOL,
)

def find_type_equals_check(self, node: ComparisonExpr, expr_indices: List[int]
) -> Tuple[TypeMap, TypeMap]:
"""Narrow types based on any checks of the type ``type(x) == T``
Expand Down Expand Up @@ -4089,6 +4146,7 @@ def find_isinstance_check_helper(self, node: Expression) -> Tuple[TypeMap, TypeM
elif is_false_literal(node):
return None, {}
elif isinstance(node, CallExpr):
self._check_for_truthy_type(type_map[node], node)
if len(node.args) == 0:
return {}, {}
expr = collapse_walrus(node.args[0])
Expand Down Expand Up @@ -4271,6 +4329,7 @@ def has_no_custom_eq_checks(t: Type) -> bool:
# Restrict the type of the variable to True-ish/False-ish in the if and else branches
# respectively
vartype = type_map[node]
self._check_for_truthy_type(vartype, node)
if_type: Type = true_only(vartype)
else_type: Type = false_only(vartype)
ref: Expression = node
Expand Down
6 changes: 6 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ def __str__(self) -> str:
REDUNDANT_EXPR: Final = ErrorCode(
"redundant-expr", "Warn about redundant expressions", "General", default_enabled=False
)
TRUTHY_BOOL: Final = ErrorCode(
'truthy-bool',
"Warn about expressions that could always evaluate to true in boolean contexts",
'General',
default_enabled=False
)
NAME_MATCH: Final = ErrorCode(
"name-match", "Check that type definition has consistent naming", "General"
)
Expand Down
15 changes: 13 additions & 2 deletions mypy/test/typefixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@

from typing import List, Optional, Tuple

from mypy.semanal_shared import set_callable_name
from mypy.types import (
Type, TypeVarType, AnyType, NoneType, Instance, CallableType, TypeVarDef, TypeType,
UninhabitedType, TypeOfAny, TypeAliasType, UnionType, LiteralType
)
from mypy.nodes import (
TypeInfo, ClassDef, Block, ARG_POS, ARG_OPT, ARG_STAR, SymbolTable,
COVARIANT, TypeAlias
TypeInfo, ClassDef, FuncDef, Block, ARG_POS, ARG_OPT, ARG_STAR, SymbolTable,
COVARIANT, TypeAlias, SymbolTableNode, MDEF,
)


Expand Down Expand Up @@ -62,6 +63,7 @@ def make_type_var(name: str, id: int, values: List[Type], upper_bound: Type,
typevars=['T'],
variances=[COVARIANT]) # class tuple
self.type_typei = self.make_type_info('builtins.type') # class type
self.bool_type_info = self.make_type_info('builtins.bool')
self.functioni = self.make_type_info('builtins.function') # function TODO
self.ai = self.make_type_info('A', mro=[self.oi]) # class A
self.bi = self.make_type_info('B', mro=[self.ai, self.oi]) # class B(A)
Expand Down Expand Up @@ -165,6 +167,15 @@ def make_type_var(name: str, id: int, values: List[Type], upper_bound: Type,
self.type_t = TypeType.make_normalized(self.t)
self.type_any = TypeType.make_normalized(self.anyt)

self._add_bool_dunder(self.bool_type_info)
self._add_bool_dunder(self.ai)

def _add_bool_dunder(self, type_info: TypeInfo) -> None:
signature = CallableType([], [], [], Instance(self.bool_type_info, []), self.function)
bool_func = FuncDef('__bool__', [], Block([]))
bool_func.type = set_callable_name(signature, bool_func)
type_info.names[bool_func.name] = SymbolTableNode(MDEF, bool_func)

# Helper methods

def callable(self, *a: Type) -> CallableType:
Expand Down
51 changes: 51 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -807,3 +807,54 @@ 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 testTruthyBool]
# flags: --enable-error-code truthy-bool
from typing import List, Union
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could be good to reduce the whitespace in this test


class Foo:
pass

foo = Foo()
if foo: # E: "__main__.foo" has type "__main__.Foo" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-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
if not good_union:
pass

bad_union: Union[Foo, object] = Foo()
if bad_union: # E: "__main__.bad_union" has type "Union[__main__.Foo, builtins.object]" of which no members implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool]
pass
if not bad_union: # E: "__main__.bad_union" has type "builtins.object" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool]
pass

def f():
pass
if f: # E: Function "def () -> Any" could always be true in boolean context [truthy-bool]
pass
if not f: # E: Function "def () -> Any" could always be true in boolean context [truthy-bool]
pass
conditional_result = 'foo' if f else 'bar' # E: Function "def () -> Any" could always be true in boolean context [truthy-bool]

lst: List[int] = []
if lst:
pass
[builtins fixtures/list.pyi]
10 changes: 7 additions & 3 deletions test-data/unit/fixtures/list.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class list(Sequence[T]):
@overload
def __init__(self, x: Iterable[T]) -> None: pass
def __iter__(self) -> Iterator[T]: pass
def __len__(self) -> int: pass
def __contains__(self, item: object) -> bool: pass
def __add__(self, x: list[T]) -> list[T]: pass
def __mul__(self, x: int) -> list[T]: pass
Expand All @@ -26,9 +27,12 @@ class list(Sequence[T]):

class tuple(Generic[T]): pass
class function: pass
class int: pass
class float: pass
class str: pass
class int:
def __bool__(self) -> bool: pass
class float:
def __bool__(self) -> bool: pass
class str:
def __len__(self) -> bool: pass
class bool(int): pass

property = object() # Dummy definition.
0