Skip to content
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

Add error-code for truthy-function #13686

Merged
merged 3 commits into from
Oct 14, 2022
Merged

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Sep 19, 2022

Description

A separate error code for truthy-function so it can be enabled by default.

Closes #12621

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Sep 27, 2022

@hauntsaninja Maybe this is something you're interested in? You expressed support for it in #12621 (comment).

@hauntsaninja
Copy link
Collaborator

That's a lot of primer hits; how many of them actually help users?

JelleZijlstra added a commit to JelleZijlstra/bandersnatch that referenced this pull request Sep 27, 2022
@cdce8p
Copy link
Collaborator Author

cdce8p commented Sep 27, 2022

That's a lot of primer hits; how many of them actually help users?

Surprisingly many. Below the categories of primer errors I identified. IMO enabling it would be a net improvement. If it shouldn't be enabled by default however, I think we should still create a separate error code for it with truthy-function. truthy-bool does emit a lot of unnecessary noice in most projects.

True positives

Boolean of class

class SomeClass: ...

if SomeClass:
    ...

# or
assert SomeClass
sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/requests.py:22:41: error: Function "Type[InsecureRequestWarning]" could always be true in boolean context  [truthy-function]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/connectionpool.py:1027: error: Function "Type[HTTPSConnection]" could always be true in boolean context  [truthy-function]

artigraph (https://github.com/artigraph/artigraph)
+ tests/arti/test_thresholds.py:5: error: Function "Type[Threshold]" could always be true in boolean context  [truthy-function]

Boolean of function

def func: ...

if func:
    ...

# or
assert func
schema_salad (https://github.com/common-workflow-language/schema_salad)
+ schema_salad/tests/test_typescript_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_typescript_codegen.py:58:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_java_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_dotnet_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_dotnet_codegen.py:57:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/objects.py:980: error: Function "Callable[[Any], Any]" could always be true in boolean context

bandersnatch (https://github.com/pypa/bandersnatch)
+ src/bandersnatch/simple.py:251: error: Function "Callable[[], bool]" could always be true in boolean context

arviz (https://github.com/arviz-devs/arviz)
+ arviz/tests/base_tests/test_utils.py:250: error: Function "Callable[[Any, Any], Any]" could always be true in boolean context

Especially the assert func in tests can be quite useful. E.g. schema_salad has multiple instances of assert <Path>.exists instead of <Path>.exists().

Implicit optional

func: Callable = None
if func: ...
psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/types/uuid.py:41: error: Function "Callable[..., UUID]" could always be true in boolean context  [truthy-function]
+ psycopg/psycopg/types/net.py:45: error: Function "Callable[[str], Union[IPv4Address, IPv6Address]]" could always be true in boolean context  [truthy-function]

tornado (https://github.com/tornadoweb/tornado)
+ tornado/simple_httpclient.py:550: error: Function "Callable[[HTTPResponse], None]" could always be true in boolean context

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/class_validators.py:263: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/monitor.py:433: error: Function "Callable[[], Any]" could always be true in boolean context  [truthy-function]
+ pymongo/monitor.py:436: error: Function "Callable[[], Any]" could always be true in boolean context  [truthy-function]

Some true positives

psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/connection.py:338: error: Function "Callable[[ReferenceType[BaseConnection[Row]], PGresult], None]" could always be true in boolean context  [truthy-function]

If-check is for self._notice_handler which is a function, not self._notice_handlers a list.

--

sympy (https://github.com/sympy/sympy)
+ sympy/combinatorics/free_groups.py:96: error: Function "Type[Expr]" could always be true in boolean context
class A: ...
class B: ...

isinstance(var, A or B)

--

Tanjun (https://github.com/FasterSpeeding/Tanjun)
+ tanjun/hooks.py:45: error: Function "Type[TypeVar]" could always be true in boolean context  [truthy-function]

if TypeVar instead of the correct if TYPE_CHECKING.

--

+ homeassistant/helpers/schema_config_entry_flow.py:137: error: Function "Callable[[Dict[str, Any]], Optional[str]]" could always be true in boolean context  [truthy-function]
+ homeassistant/components/goodwe/number.py:131: error: Function "Callable[[Any, int], Awaitable[None]]" could always be true in boolean context  [truthy-function]
+ homeassistant/components/kostal_plenticore/sensor.py:178: error: Function "Callable[[str], Any]" could always be true in boolean context  [truthy-function]

I fixed these already: home-assistant/core#78819

False positives

There are also some false-positives, although those arguably need better typing or can't be fixed.

Import guard

try:
    from asyncio import create_task
except ImportError:  # Python < 3.7
    create_task = None  # type: ignore

if create_task:
    ...
graphql-core (https://github.com/graphql-python/graphql-core)
+ tests/test_user_registry.py:513: error: Function "Callable[[Union[Generator[Any, None, _T], Coroutine[Any, Any, _T]], DefaultNamedArg(Optional[str], 'name')], Task[_T]]" could always be true in boolean context

Moneypatching in tests

Deleting os.path.abspath, undoing it, and assert that function is present.

pytest (https://github.com/pytest-dev/pytest)
+ testing/test_monkeypatch.py:95: error: Function overloaded function could always be true in boolean context  [truthy-function]

@JelleZijlstra
Copy link
Member

The Tanjun one was actually also wrong and just got fixed: FasterSpeeding/Tanjun@d697a6b

cooperlees pushed a commit to pypa/bandersnatch that referenced this pull request Sep 28, 2022
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for the analysis! This looks pretty good to me. Will leave it open for a couple days in case anyone else has opinions.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/rst.py: note: In function "default_role":
+ sphinx/util/rst.py:66:12: error: Function "Callable[[str, str, str, int, Inliner, Dict[str, Any], List[str]], Tuple[List[Node], List[system_message]]]" could always be true in boolean context  [truthy-function]
+ sphinx/directives/__init__.py: note: In member "run" of class "DefaultRole":
+ sphinx/directives/__init__.py:280:12: error: Function "Callable[[str, str, str, int, Inliner, Dict[str, Any], List[str]], Tuple[List[Node], List[system_message]]]" could always be true in boolean context  [truthy-function]
+ sphinx/writers/html5.py: note: In member "depart_math" of class "HTML5Translator":
+ sphinx/writers/html5.py:810:12: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]
+ sphinx/writers/html5.py: note: In member "depart_math_block" of class "HTML5Translator":
+ sphinx/writers/html5.py:821:12: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]
+ sphinx/writers/html.py: note: In member "depart_math" of class "HTMLTranslator":
+ sphinx/writers/html.py:872:12: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]
+ sphinx/writers/html.py: note: In member "depart_math_block" of class "HTMLTranslator":
+ sphinx/writers/html.py:883:12: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]
+ sphinx/util/requests.py: note: In function "ignore_insecure_warning":
+ sphinx/util/requests.py:22:41: error: Function "Type[InsecureRequestWarning]" could always be true in boolean context  [truthy-function]

graphql-core (https://github.com/graphql-python/graphql-core)
+ tests/test_user_registry.py:507: error: Function "Callable[[Union[Generator[Any, None, _T], Coroutine[Any, Any, _T]], DefaultNamedArg(Optional[str], 'name')], Task[_T]]" could always be true in boolean context  [truthy-function]

psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/connection.py:338: error: Function "Callable[[ReferenceType[BaseConnection[Row]], PGresult], None]" could always be true in boolean context  [truthy-function]
+ psycopg/psycopg/types/uuid.py:41: error: Function "Callable[..., UUID]" could always be true in boolean context  [truthy-function]
+ psycopg/psycopg/types/net.py:45: error: Function "Callable[[str], Union[IPv4Address, IPv6Address]]" could always be true in boolean context  [truthy-function]

pyppeteer (https://github.com/pyppeteer/pyppeteer)
+ pyppeteer/browser.py:54: error: Function "Callable[[], Awaitable[None]]" could always be true in boolean context  [truthy-function]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/connectionpool.py:1027: error: Function "Type[HTTPSConnection]" could always be true in boolean context  [truthy-function]
+ src/urllib3/connectionpool.py:1027: note: Error code "truthy-function" not covered by "type: ignore" comment

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/settings.py:667: error: Function "Callable[[Settings, T], T]" could always be true in boolean context  [truthy-function]
+ src/prefect/engine.py:1186: error: Function "Callable[[TaskRunContext, Dict[str, Any]], Optional[str]]" could always be true in boolean context  [truthy-function]

artigraph (https://github.com/artigraph/artigraph)
+ tests/arti/test_thresholds.py:5: error: Function "Type[Threshold]" could always be true in boolean context  [truthy-function]

sympy (https://github.com/sympy/sympy)
+ sympy/combinatorics/free_groups.py:96: error: Function "Type[Expr]" could always be true in boolean context  [truthy-function]

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/objects.py:980: error: Function "Callable[[Any], Any]" could always be true in boolean context  [truthy-function]

schema_salad (https://github.com/common-workflow-language/schema_salad)
+ schema_salad/tests/test_typescript_codegen.py: note: In function "test_cwl_gen":
+ schema_salad/tests/test_typescript_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_typescript_codegen.py: note: In function "test_class_field":
+ schema_salad/tests/test_typescript_codegen.py:58:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_java_codegen.py: note: In function "test_cwl_gen":
+ schema_salad/tests/test_java_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_dotnet_codegen.py: note: In function "test_cwl_gen":
+ schema_salad/tests/test_dotnet_codegen.py:25:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
+ schema_salad/tests/test_dotnet_codegen.py: note: In function "test_class_field":
+ schema_salad/tests/test_dotnet_codegen.py:57:12: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]

arviz (https://github.com/arviz-devs/arviz)
+ arviz/tests/base_tests/test_utils.py:250: error: Function "Callable[[Any, Any], Any]" could always be true in boolean context  [truthy-function]

tornado (https://github.com/tornadoweb/tornado)
+ tornado/simple_httpclient.py:550: error: Function "Callable[[HTTPResponse], None]" could always be true in boolean context  [truthy-function]

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/class_validators.py:263: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]

pytest (https://github.com/pytest-dev/pytest)
+ testing/test_monkeypatch.py:95: error: Function overloaded function could always be true in boolean context  [truthy-function]

core (https://github.com/home-assistant/core)
+ homeassistant/components/radarr/sensor.py:167: error: Function "Callable[[RadarrSensorEntityDescription[Any], Any], Optional[Tuple[RadarrSensorEntityDescription[Any], str]]]" could always be true in boolean context  [truthy-function]
+ homeassistant/components/lidarr/sensor.py:123: error: Function "Callable[[LidarrSensorEntityDescription[Any], Any], Optional[Tuple[LidarrSensorEntityDescription[Any], str]]]" could always be true in boolean context  [truthy-function]

cloud-init (https://github.com/canonical/cloud-init)
+ cloudinit/url_helper.py:654: error: Function "Callable[[Any, int], int]" could always be true in boolean context  [truthy-function]

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/monitor.py: note: In function "_shutdown_resources":
+ pymongo/monitor.py:433: error: Function "Callable[[], Any]" could always be true in boolean context  [truthy-function]
+ pymongo/monitor.py:436: error: Function "Callable[[], Any]" could always be true in boolean context  [truthy-function]

@sobolevn
Copy link
Member

sobolevn commented Oct 2, 2022

dulwich has this line: https://github.com/jelmer/dulwich/blob/master/dulwich/objects.py#L980

key_func = name_order and key_entry_name_order or key_entry

I am pretty sure that it is an example of old-style ternary. This rule does not like it.
Probably, this is a good thing. But, I thought it is worth highlighting.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Oct 2, 2022

dullwitch has this line: https://github.com/jelmer/dulwich/blob/master/dulwich/objects.py#L980

key_func = name_order and key_entry_name_order or key_entry

I am pretty sure that it is an example of old-style ternary. This rule does not like it. Probably, this is a good thing. But, I thought it is worth highlighting.

Not sure we can do anything about it. I would probably use a type: ignore in this case.

@hauntsaninja hauntsaninja merged commit b3d94dc into python:master Oct 14, 2022
@cdce8p cdce8p deleted the truthy-function branch October 14, 2022 06:43
andersk added a commit to andersk/mypy that referenced this pull request Jan 3, 2023
This error was enabled by default since its introduction (python#13686);
document it in the correct section.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@@ -258,6 +258,19 @@ except that attempting to invoke an undefined method (e.g. ``__len__``) results
while attempting to evaluate an object in boolean context without a concrete implementation results in a truthy value.


Check that function isn't used in boolean context [truthy-function]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be under “enabled by default” rather than “optional checks”, right?

andersk added a commit to andersk/mypy that referenced this pull request Jan 10, 2023
This error was enabled by default since its introduction (python#13686);
document it in the correct section.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
hauntsaninja pushed a commit that referenced this pull request Jan 11, 2023
…lt” (#14380)

This error was enabled by default since its introduction (#13686);
document it in the correct section.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add truthy-callable-bool check
5 participants