From 23d5401c96aead22d7ce8f46096917d321ab97c1 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 29 Sep 2022 04:26:43 +0200 Subject: [PATCH 1/3] Add error-code for truthy-iterable --- docs/source/error_code_list2.rst | 28 ++++++++++++------------- mypy/checker.py | 8 +++++++ mypy/errorcodes.py | 5 +++++ mypy/message_registry.py | 4 ++++ mypy/semanal.py | 6 ++++-- mypyc/test-data/irbuild-statements.test | 4 ++-- test-data/unit/check-errorcodes.test | 7 +++++++ 7 files changed, 43 insertions(+), 19 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index cac19e705361..d0b4611b0e30 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -231,31 +231,29 @@ since unless implemented by a sub-type, the expression will always evaluate to t if foo: ... +This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``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. -This check might falsely imply an error. For example, ``Iterable`` does not implement -``__len__`` and so this code will be flagged: -.. code-block:: python +Check that iterable is not implicitly true in boolean context [truthy-iterable] +------------------------------------------------------------------------------- + +``Iterable`` does not implement ``__len__`` and so this code will be flagged: - # Use "mypy -enable-error-code truthy-bool ..." +.. code-block:: python from typing import Iterable - def transform(items: Iterable[int]) -> Iterable[int]: - # Error: "items" has type "Iterable[int]" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool] + def transform(items: Iterable[int]) -> list[int]: + # Error :"items" has type "Iterable[int]" which can always be true in boolean context. Consider using "Collection[int]" instead. [truthy-iterable] 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: Sequence[int]``. - -This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``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. +If called with a ``Generator`` like ``int(x) for x 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 is recommended to annotate ``items: Collection[int]``. Check that function isn't used in boolean context [truthy-function] diff --git a/mypy/checker.py b/mypy/checker.py index 67d132afe2c7..694b0b25c9b0 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5088,6 +5088,14 @@ def format_expr_type() -> str: self.fail(message_registry.FUNCTION_ALWAYS_TRUE.format(format_type(t)), expr) elif isinstance(t, UnionType): self.fail(message_registry.TYPE_ALWAYS_TRUE_UNIONTYPE.format(format_expr_type()), expr) + elif isinstance(t, Instance) and t.type.fullname == "typing.Iterable": + _, info = self.make_fake_typeinfo("typing", "Collection", "Collection", []) + self.fail( + message_registry.ITERABLE_ALWAYS_TRUE.format( + format_expr_type(), format_type(Instance(info, t.args)) + ), + expr, + ) else: self.fail(message_registry.TYPE_ALWAYS_TRUE.format(format_expr_type()), expr) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index f2a74c332b2e..e9b828d826c2 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -160,6 +160,11 @@ def __str__(self) -> str: "Warn about function that always evaluate to true in boolean contexts", "General", ) +TRUTHY_ITERABLE: Final[ErrorCode] = ErrorCode( + "truthy-iterable", + "Warn about Iterable expressions that could always evaluate to true in boolean contexts", + "General", +) NAME_MATCH: Final = ErrorCode( "name-match", "Check that type definition has consistent naming", "General" ) diff --git a/mypy/message_registry.py b/mypy/message_registry.py index 18acb2cd7a71..219c445497e9 100644 --- a/mypy/message_registry.py +++ b/mypy/message_registry.py @@ -153,6 +153,10 @@ def with_additional_msg(self, info: str) -> ErrorMessage: FUNCTION_ALWAYS_TRUE: Final = ErrorMessage( "Function {} could always be true in boolean context", code=codes.TRUTHY_FUNCTION ) +ITERABLE_ALWAYS_TRUE: Final = ErrorMessage( + "{} which can always be true in boolean context. Consider using {} instead.", + code=codes.TRUTHY_ITERABLE, +) NOT_CALLABLE: Final = "{} not callable" TYPE_MUST_BE_USED: Final = "Value of type {} must be used" diff --git a/mypy/semanal.py b/mypy/semanal.py index 77555648ba7e..ce88d033e01c 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -51,7 +51,7 @@ from __future__ import annotations from contextlib import contextmanager -from typing import Any, Callable, Iterable, Iterator, List, TypeVar, cast +from typing import Any, Callable, Collection, Iterable, Iterator, List, TypeVar, cast from typing_extensions import Final, TypeAlias as _TypeAlias from mypy import errorcodes as codes, message_registry @@ -6202,7 +6202,9 @@ def add_plugin_dependency(self, trigger: str, target: str | None = None) -> None target = self.scope.current_target() self.cur_mod_node.plugin_deps.setdefault(trigger, set()).add(target) - def add_type_alias_deps(self, aliases_used: Iterable[str], target: str | None = None) -> None: + def add_type_alias_deps( + self, aliases_used: Collection[str], target: str | None = None + ) -> None: """Add full names of type aliases on which the current node depends. This is used by fine-grained incremental mode to re-check the corresponding nodes. diff --git a/mypyc/test-data/irbuild-statements.test b/mypyc/test-data/irbuild-statements.test index ab947c956b74..090c7ed9f3df 100644 --- a/mypyc/test-data/irbuild-statements.test +++ b/mypyc/test-data/irbuild-statements.test @@ -1006,9 +1006,9 @@ L5: return 1 [case testForZip] -from typing import List, Iterable +from typing import List, Iterable, Sequence -def f(a: List[int], b: Iterable[bool]) -> None: +def f(a: List[int], b: Sequence[bool]) -> None: for x, y in zip(a, b): if b: x = 1 diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 81b8948be14a..4b18896b8fd6 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -857,6 +857,13 @@ if not f: # E: Function "Callable[[], Any]" could always be true in boolean con pass conditional_result = 'foo' if f else 'bar' # E: Function "Callable[[], Any]" could always be true in boolean context [truthy-function] +[case testTruthyIterable] +# flags: --strict-optional +from typing import Iterable +def func(var: Iterable[str]) -> None: + if var: # E: "var" has type "Iterable[str]" which can always be true in boolean context. Consider using "Collection[str]" instead. [truthy-iterable] + ... + [case testNoOverloadImplementation] from typing import overload From d146b47ce40f09baa436deed0248eec67bad181f Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 9 Nov 2022 12:45:13 +0100 Subject: [PATCH 2/3] Disable truthy-iterable by default --- mypy/errorcodes.py | 1 + test-data/unit/check-errorcodes.test | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index e9b828d826c2..3aee6881067e 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -164,6 +164,7 @@ def __str__(self) -> str: "truthy-iterable", "Warn about Iterable 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" diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 4b18896b8fd6..798c52629a35 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -858,7 +858,7 @@ if not f: # E: Function "Callable[[], Any]" could always be true in boolean con conditional_result = 'foo' if f else 'bar' # E: Function "Callable[[], Any]" could always be true in boolean context [truthy-function] [case testTruthyIterable] -# flags: --strict-optional +# flags: --strict-optional --enable-error-code truthy-iterable from typing import Iterable def func(var: Iterable[str]) -> None: if var: # E: "var" has type "Iterable[str]" which can always be true in boolean context. Consider using "Collection[str]" instead. [truthy-iterable] From f1d7baffdfbcdbb812f44a5ffaefcfac535063fd Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Sun, 13 Nov 2022 17:58:46 +0100 Subject: [PATCH 3/3] Code review --- docs/source/error_code_list2.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index d0b4611b0e30..0a2d8a8c5c5c 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -231,7 +231,7 @@ since unless implemented by a sub-type, the expression will always evaluate to t if foo: ... -This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``Sized``), +The check is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``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. @@ -246,7 +246,7 @@ Check that iterable is not implicitly true in boolean context [truthy-iterable] from typing import Iterable def transform(items: Iterable[int]) -> list[int]: - # Error :"items" has type "Iterable[int]" which can always be true in boolean context. Consider using "Collection[int]" instead. [truthy-iterable] + # Error: "items" has type "Iterable[int]" which can always be true in boolean context. Consider using "Collection[int]" instead. [truthy-iterable] if not items: return [42] return [x + 1 for x in items]