From 24f5e457d7a91edf252a281a5861ba09ca5831ba Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 23 Aug 2023 14:34:17 +0300 Subject: [PATCH 1/7] Suppress second error message with `:=` and `[truthy-bool]` --- mypy/checker.py | 19 ++++++++++++++----- test-data/unit/check-errorcodes.test | 3 +++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 87dff91758f5d..ba11e3341a48b 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5502,7 +5502,9 @@ def combine_maps(list_maps: list[TypeMap]) -> TypeMap: else_map = {} return if_map, else_map - def find_isinstance_check(self, node: Expression) -> tuple[TypeMap, TypeMap]: + def find_isinstance_check( + self, node: Expression, *, is_walrus_value: bool = False + ) -> tuple[TypeMap, TypeMap]: """Find any isinstance checks (within a chain of ands). Includes implicit and explicit checks for None and calls to callable. Also includes TypeGuard functions. @@ -5516,12 +5518,14 @@ def find_isinstance_check(self, node: Expression) -> tuple[TypeMap, TypeMap]: May return {}, {}. Can return None, None in situations involving NoReturn. """ - if_map, else_map = self.find_isinstance_check_helper(node) + if_map, else_map = self.find_isinstance_check_helper(node, is_walrus_value=is_walrus_value) new_if_map = self.propagate_up_typemap_info(if_map) new_else_map = self.propagate_up_typemap_info(else_map) return new_if_map, new_else_map - def find_isinstance_check_helper(self, node: Expression) -> tuple[TypeMap, TypeMap]: + def find_isinstance_check_helper( + self, node: Expression, *, is_walrus_value: bool = False + ) -> tuple[TypeMap, TypeMap]: if is_true_literal(node): return {}, None if is_false_literal(node): @@ -5759,7 +5763,9 @@ def has_no_custom_eq_checks(t: Type) -> bool: if else_assignment_map is not None: else_map.update(else_assignment_map) - if_condition_map, else_condition_map = self.find_isinstance_check(node.value) + if_condition_map, else_condition_map = self.find_isinstance_check( + node.value, is_walrus_value=True + ) if if_condition_map is not None: if_map.update(if_condition_map) @@ -5797,7 +5803,10 @@ 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 original_vartype = self.lookup_type(node) - self._check_for_truthy_type(original_vartype, node) + if not is_walrus_value: + # We don't check `:=` values in expresions like `(a := A())`, + # because they produce two error messages. + self._check_for_truthy_type(original_vartype, node) vartype = try_expanding_sum_type_to_union(original_vartype, "builtins.bool") if_type = true_only(vartype) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 796e1c1ea98ef..55b7a49a31114 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -892,6 +892,9 @@ if a: any_or_object: Union[object, Any] if any_or_object: pass + +if (my_foo := Foo()): # E: "__main__.my_foo" has type "Foo" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool] + pass [builtins fixtures/list.pyi] [case testTruthyFunctions] From 1eb1abc09efcb484c5078abd0c7af905077575d7 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 23 Aug 2023 15:02:53 +0300 Subject: [PATCH 2/7] Fix tests --- test-data/unit/check-python38.test | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test-data/unit/check-python38.test b/test-data/unit/check-python38.test index d83f29f2186a9..697eae5076c9c 100644 --- a/test-data/unit/check-python38.test +++ b/test-data/unit/check-python38.test @@ -310,8 +310,7 @@ def f(x: int = (c := 4)) -> int: z2: NT # E: Variable "NT" is not valid as a type \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases - if Alias := int: # E: Function "Alias" could always be true in boolean context \ - # E: Function "int" could always be true in boolean context + if Alias := int: # E: Function "Alias" could always be true in boolean context z3: Alias # E: Variable "Alias" is not valid as a type \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases From 39b180337df8b7f95a01d25e403dacf7e5410fa0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 24 Aug 2023 11:01:03 +0300 Subject: [PATCH 3/7] Add more tests --- test-data/unit/check-errorcodes.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 55b7a49a31114..4b57358d3cbb5 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -895,6 +895,9 @@ if any_or_object: if (my_foo := Foo()): # E: "__main__.my_foo" has type "Foo" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool] pass + +if my_a := (a or Foo()): # E: "__main__.Foo" returns "Foo" which does not implement __bool__ or __len__ so it could always be true in boolean context [truthy-bool] + pass [builtins fixtures/list.pyi] [case testTruthyFunctions] From 30522e92095739216b8f16e990ddffe850dafaf9 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 24 Aug 2023 11:50:59 +0300 Subject: [PATCH 4/7] Update mypy/checker.py Co-authored-by: Ilya Priven --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index ba11e3341a48b..54ef79fa26e82 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5804,7 +5804,7 @@ def has_no_custom_eq_checks(t: Type) -> bool: # respectively original_vartype = self.lookup_type(node) if not is_walrus_value: - # We don't check `:=` values in expresions like `(a := A())`, + # We don't check `:=` values in expressions like `(a := A())`, # because they produce two error messages. self._check_for_truthy_type(original_vartype, node) vartype = try_expanding_sum_type_to_union(original_vartype, "builtins.bool") From 93108fcdd7d7f029763d4f9980140b889b037e48 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 28 Aug 2023 10:32:44 +0300 Subject: [PATCH 5/7] Address review --- mypy/checker.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index ba11e3341a48b..f3786dd49f839 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5503,7 +5503,7 @@ def combine_maps(list_maps: list[TypeMap]) -> TypeMap: return if_map, else_map def find_isinstance_check( - self, node: Expression, *, is_walrus_value: bool = False + self, node: Expression, *, in_boolean_context: bool = True ) -> tuple[TypeMap, TypeMap]: """Find any isinstance checks (within a chain of ands). Includes implicit and explicit checks for None and calls to callable. @@ -5518,13 +5518,15 @@ def find_isinstance_check( May return {}, {}. Can return None, None in situations involving NoReturn. """ - if_map, else_map = self.find_isinstance_check_helper(node, is_walrus_value=is_walrus_value) + if_map, else_map = self.find_isinstance_check_helper( + node, in_boolean_context=in_boolean_context + ) new_if_map = self.propagate_up_typemap_info(if_map) new_else_map = self.propagate_up_typemap_info(else_map) return new_if_map, new_else_map def find_isinstance_check_helper( - self, node: Expression, *, is_walrus_value: bool = False + self, node: Expression, *, in_boolean_context: bool = True ) -> tuple[TypeMap, TypeMap]: if is_true_literal(node): return {}, None @@ -5756,16 +5758,16 @@ def has_no_custom_eq_checks(t: Type) -> bool: if_map = {} else_map = {} - if_assignment_map, else_assignment_map = self.find_isinstance_check(node.target) + if_assignment_map, else_assignment_map = self.find_isinstance_check( + node.target, in_boolean_context=False + ) if if_assignment_map is not None: if_map.update(if_assignment_map) if else_assignment_map is not None: else_map.update(else_assignment_map) - if_condition_map, else_condition_map = self.find_isinstance_check( - node.value, is_walrus_value=True - ) + if_condition_map, else_condition_map = self.find_isinstance_check(node.value) if if_condition_map is not None: if_map.update(if_condition_map) @@ -5803,7 +5805,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 original_vartype = self.lookup_type(node) - if not is_walrus_value: + if not in_boolean_context: # We don't check `:=` values in expresions like `(a := A())`, # because they produce two error messages. self._check_for_truthy_type(original_vartype, node) From aca01cb7496d1207179a3614dc70657d9b500875 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 28 Aug 2023 10:55:30 +0300 Subject: [PATCH 6/7] Fix CI --- mypy/checker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a6b46524de6b1..05f324039c985 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5758,16 +5758,16 @@ def has_no_custom_eq_checks(t: Type) -> bool: if_map = {} else_map = {} - if_assignment_map, else_assignment_map = self.find_isinstance_check( - node.target, in_boolean_context=False - ) + if_assignment_map, else_assignment_map = self.find_isinstance_check(node.target) if if_assignment_map is not None: if_map.update(if_assignment_map) if else_assignment_map is not None: else_map.update(else_assignment_map) - if_condition_map, else_condition_map = self.find_isinstance_check(node.value) + if_condition_map, else_condition_map = self.find_isinstance_check( + node.value, in_boolean_context=False + ) if if_condition_map is not None: if_map.update(if_condition_map) From b8a5d9c802aefd51bfab849a53461f49a11bea7d Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Mon, 24 Jun 2024 11:59:22 +0300 Subject: [PATCH 7/7] Update checker.py --- mypy/checker.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index d469c420e2c59..4f20c6ee8493e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -5775,6 +5775,11 @@ def find_isinstance_check( If either of the values in the tuple is None, then that particular branch can never occur. + If `in_boolean_context=True` is passed, it means that we handle + a walrus expression. We treat rhs values + in expressions like `(a := A())` specially: + for example, some errors are suppressed. + May return {}, {}. Can return None, None in situations involving NoReturn. """