From f30417c15377752680b25c44213671846a20c06c Mon Sep 17 00:00:00 2001 From: Nedelcu Date: Tue, 7 Jan 2025 22:27:25 +0200 Subject: [PATCH 01/10] (bugfix) Issue 10161: Incorrect scenario of unidiomatic-typecheck. --- doc/whatsnew/fragments/10161.false_positive | 3 +++ pylint/checkers/base/comparison_checker.py | 7 ++---- tests/functional/u/unidiomatic_typecheck.py | 26 ++++++++++++++------ tests/functional/u/unidiomatic_typecheck.txt | 6 ----- 4 files changed, 24 insertions(+), 18 deletions(-) create mode 100644 doc/whatsnew/fragments/10161.false_positive diff --git a/doc/whatsnew/fragments/10161.false_positive b/doc/whatsnew/fragments/10161.false_positive new file mode 100644 index 0000000000..f8c7540c8e --- /dev/null +++ b/doc/whatsnew/fragments/10161.false_positive @@ -0,0 +1,3 @@ +Fix a false positive for `unidiomatic-typecheck` when comparing two direct types. + +Closes #10161 diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index 353f74383e..b778210fec 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -343,15 +343,12 @@ def _check_type_x_is_y( ): return - if operator in {"is", "is not"} and _is_one_arg_pos_call(right): + if operator in {"!=", "==", "is", "is not"} and _is_one_arg_pos_call(right): right_func = utils.safe_infer(right.func) if ( isinstance(right_func, nodes.ClassDef) and right_func.qname() == TYPE_QNAME ): # type(x) == type(a) - right_arg = utils.safe_infer(right.args[0]) - if not isinstance(right_arg, LITERAL_NODE_TYPES): - # not e.g. type(x) == type([]) - return + return self.add_message("unidiomatic-typecheck", node=node) diff --git a/tests/functional/u/unidiomatic_typecheck.py b/tests/functional/u/unidiomatic_typecheck.py index 1a87280cae..09bf44c0e8 100644 --- a/tests/functional/u/unidiomatic_typecheck.py +++ b/tests/functional/u/unidiomatic_typecheck.py @@ -65,10 +65,22 @@ def deliberate_subclass_check_negatives(b): type(42) is type(b) type(42) is not type(b) -def type_of_literals_positives(a): - type(a) is type([]) # [unidiomatic-typecheck] - type(a) is not type([]) # [unidiomatic-typecheck] - type(a) is type({}) # [unidiomatic-typecheck] - type(a) is not type({}) # [unidiomatic-typecheck] - type(a) is type("") # [unidiomatic-typecheck] - type(a) is not type("") # [unidiomatic-typecheck] +def type_of_literals_negatives(a): + type(a) is type([]) + type(a) is not type([]) + type(a) is type({}) + type(a) is not type({}) + type(a) is type("") + type(a) is not type("") + type(a) == type([]) + type(a) != type([]) + type(a) == type({}) + type(a) != type({}) + type(a) == type("") + type(a) != type("") + +def double_type_check_negatives(a, b): + type(a) == type(b) + type(a) != type(b) + type(a) is type(b) + type(a) is not type(b) diff --git a/tests/functional/u/unidiomatic_typecheck.txt b/tests/functional/u/unidiomatic_typecheck.txt index 5916407bd9..0d2edd9baa 100644 --- a/tests/functional/u/unidiomatic_typecheck.txt +++ b/tests/functional/u/unidiomatic_typecheck.txt @@ -10,9 +10,3 @@ unidiomatic-typecheck:16:4:16:20:simple_inference_positives:Use isinstance() rat unidiomatic-typecheck:17:4:17:24:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:18:4:18:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:19:4:19:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:69:4:69:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:70:4:70:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:71:4:71:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:72:4:72:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:73:4:73:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:74:4:74:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED From 0a0960630ac9ee1fdef52a3f46169bd816f856dc Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 2 May 2025 10:54:07 +0200 Subject: [PATCH 02/10] Take zenlyj remark into account --- tests/functional/u/unidiomatic_typecheck.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/functional/u/unidiomatic_typecheck.py b/tests/functional/u/unidiomatic_typecheck.py index 09bf44c0e8..f9b03e1957 100644 --- a/tests/functional/u/unidiomatic_typecheck.py +++ b/tests/functional/u/unidiomatic_typecheck.py @@ -61,9 +61,15 @@ def parameter_shadowing_inference_negatives(type): type(42) in [int] type(42) not in [int] -def deliberate_subclass_check_negatives(b): +def deliberate_subclass_check_negatives(a,b): type(42) is type(b) type(42) is not type(b) + type(42) == type(b) + type(42) != type(b) + type(a) is type(b) + type(a) is not type(b) + type(a) == type(b) + type(a) != type(b) def type_of_literals_negatives(a): type(a) is type([]) @@ -78,9 +84,3 @@ def type_of_literals_negatives(a): type(a) != type({}) type(a) == type("") type(a) != type("") - -def double_type_check_negatives(a, b): - type(a) == type(b) - type(a) != type(b) - type(a) is type(b) - type(a) is not type(b) From 4380fbe4a8ca604a6eeeb81c0f35e21d6fadb19a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 2 May 2025 10:58:24 +0200 Subject: [PATCH 03/10] More descriptive fragment --- doc/whatsnew/fragments/10161.false_positive | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/10161.false_positive b/doc/whatsnew/fragments/10161.false_positive index f8c7540c8e..ec10ad4c1e 100644 --- a/doc/whatsnew/fragments/10161.false_positive +++ b/doc/whatsnew/fragments/10161.false_positive @@ -1,3 +1,3 @@ -Fix a false positive for `unidiomatic-typecheck` when comparing two direct types. +Comparisons of types won't raise an ``unidiomatic-typecheck`` warning anymore, consistent with the behavior applied only for ``==`` previously. Closes #10161 From 59a0a5d726ab5843952b0b79969788a30defb6c5 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 10:50:41 +0200 Subject: [PATCH 04/10] Update pylint/checkers/base/comparison_checker.py Co-authored-by: Zen Lee <53538590+zenlyj@users.noreply.github.com> --- pylint/checkers/base/comparison_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index b778210fec..000f1c4218 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -343,7 +343,7 @@ def _check_type_x_is_y( ): return - if operator in {"!=", "==", "is", "is not"} and _is_one_arg_pos_call(right): + if _is_one_arg_pos_call(right): right_func = utils.safe_infer(right.func) if ( isinstance(right_func, nodes.ClassDef) From d61b85cb7db0f816657f7d60fede5c3cd4efaf26 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 22:00:22 +0200 Subject: [PATCH 05/10] Update doc/whatsnew/fragments/10161.false_positive Co-authored-by: Jacob Walls --- doc/whatsnew/fragments/10161.false_positive | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/10161.false_positive b/doc/whatsnew/fragments/10161.false_positive index ec10ad4c1e..dc343e05ae 100644 --- a/doc/whatsnew/fragments/10161.false_positive +++ b/doc/whatsnew/fragments/10161.false_positive @@ -1,3 +1,3 @@ -Comparisons of types won't raise an ``unidiomatic-typecheck`` warning anymore, consistent with the behavior applied only for ``==`` previously. +Comparisons between two calls to `type()` won't raise an ``unidiomatic-typecheck`` warning anymore, consistent with the behavior applied only for ``==`` previously. Closes #10161 From 986d1b40881d51769fa8a2c82d92e85ad361f563 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 22:01:36 +0200 Subject: [PATCH 06/10] Further simplification --- pylint/checkers/base/comparison_checker.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index 000f1c4218..2222a8235e 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -323,18 +323,13 @@ def _check_unidiomatic_typecheck(self, node: nodes.Compare) -> None: if operator in TYPECHECK_COMPARISON_OPERATORS: left = node.left if _is_one_arg_pos_call(left): - self._check_type_x_is_y(node, left, operator, right) + self._check_type_x_is_y(node=node, left=left, right=right) elif isinstance(left, nodes.Name) and _is_one_arg_pos_call(right): - self._check_type_x_is_y( - node=node, left=right, operator=operator, right=left - ) # transforming Y == type(x) case to type(x) == Y + # transforming Y == type(x) case to type(x) == Y + self._check_type_x_is_y(node=node, left=right, right=left) def _check_type_x_is_y( - self, - node: nodes.Compare, - left: nodes.NodeNG, - operator: str, - right: nodes.NodeNG, + self, node: nodes.Compare, left: nodes.NodeNG, right: nodes.NodeNG ) -> None: """Check for expressions like type(x) == Y.""" left_func = utils.safe_infer(left.func) From ac6e5177706069d1dedc24a7bc038eccbf3b44f0 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 22:03:56 +0200 Subject: [PATCH 07/10] Revert following discussion --- pylint/checkers/base/comparison_checker.py | 6 ++++- tests/functional/u/unidiomatic_typecheck.py | 26 ++++++++++---------- tests/functional/u/unidiomatic_typecheck.txt | 12 +++++++++ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index 2222a8235e..41fba1d3a3 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -345,5 +345,9 @@ def _check_type_x_is_y( and right_func.qname() == TYPE_QNAME ): # type(x) == type(a) - return + right_arg = utils.safe_infer(right.args[0]) + if not isinstance(right_arg, LITERAL_NODE_TYPES): + # not e.g. type(x) == type([]) + return + self.add_message("unidiomatic-typecheck", node=node) diff --git a/tests/functional/u/unidiomatic_typecheck.py b/tests/functional/u/unidiomatic_typecheck.py index f9b03e1957..7e8116c769 100644 --- a/tests/functional/u/unidiomatic_typecheck.py +++ b/tests/functional/u/unidiomatic_typecheck.py @@ -71,16 +71,16 @@ def deliberate_subclass_check_negatives(a,b): type(a) == type(b) type(a) != type(b) -def type_of_literals_negatives(a): - type(a) is type([]) - type(a) is not type([]) - type(a) is type({}) - type(a) is not type({}) - type(a) is type("") - type(a) is not type("") - type(a) == type([]) - type(a) != type([]) - type(a) == type({}) - type(a) != type({}) - type(a) == type("") - type(a) != type("") +def type_of_literals_positives(a): + type(a) is type([]) # [unidiomatic-typecheck] + type(a) is not type([]) # [unidiomatic-typecheck] + type(a) is type({}) # [unidiomatic-typecheck] + type(a) is not type({}) # [unidiomatic-typecheck] + type(a) is type("") # [unidiomatic-typecheck] + type(a) is not type("") # [unidiomatic-typecheck] + type(a) == type([]) # [unidiomatic-typecheck] + type(a) != type([]) # [unidiomatic-typecheck] + type(a) == type({}) # [unidiomatic-typecheck] + type(a) != type({}) # [unidiomatic-typecheck] + type(a) == type("") # [unidiomatic-typecheck] + type(a) != type("") # [unidiomatic-typecheck] diff --git a/tests/functional/u/unidiomatic_typecheck.txt b/tests/functional/u/unidiomatic_typecheck.txt index 0d2edd9baa..15c898c101 100644 --- a/tests/functional/u/unidiomatic_typecheck.txt +++ b/tests/functional/u/unidiomatic_typecheck.txt @@ -10,3 +10,15 @@ unidiomatic-typecheck:16:4:16:20:simple_inference_positives:Use isinstance() rat unidiomatic-typecheck:17:4:17:24:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:18:4:18:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:19:4:19:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:75:4:75:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:76:4:76:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:77:4:77:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:78:4:78:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:79:4:79:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:80:4:80:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:81:4:81:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:82:4:82:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:83:4:83:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:84:4:84:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:85:4:85:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:86:4:86:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED From dc13f1bd075e1424d193d6b8122eeff6a3f0063e Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 22:13:00 +0200 Subject: [PATCH 08/10] black --- tests/functional/u/unidiomatic_typecheck.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/functional/u/unidiomatic_typecheck.py b/tests/functional/u/unidiomatic_typecheck.py index 7e8116c769..784668b8a6 100644 --- a/tests/functional/u/unidiomatic_typecheck.py +++ b/tests/functional/u/unidiomatic_typecheck.py @@ -61,7 +61,8 @@ def parameter_shadowing_inference_negatives(type): type(42) in [int] type(42) not in [int] -def deliberate_subclass_check_negatives(a,b): + +def deliberate_subclass_check_negatives(a, b): type(42) is type(b) type(42) is not type(b) type(42) == type(b) @@ -71,8 +72,9 @@ def deliberate_subclass_check_negatives(a,b): type(a) == type(b) type(a) != type(b) + def type_of_literals_positives(a): - type(a) is type([]) # [unidiomatic-typecheck] + type(a) is type([]) # [unidiomatic-typecheck] type(a) is not type([]) # [unidiomatic-typecheck] type(a) is type({}) # [unidiomatic-typecheck] type(a) is not type({}) # [unidiomatic-typecheck] From 59f975c58158a804121fd6cee1ed3566e0dabbef Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 3 May 2025 22:16:36 +0200 Subject: [PATCH 09/10] Update pylint/checkers/base/comparison_checker.py --- pylint/checkers/base/comparison_checker.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pylint/checkers/base/comparison_checker.py b/pylint/checkers/base/comparison_checker.py index 41fba1d3a3..a2c5b10f11 100644 --- a/pylint/checkers/base/comparison_checker.py +++ b/pylint/checkers/base/comparison_checker.py @@ -349,5 +349,4 @@ def _check_type_x_is_y( if not isinstance(right_arg, LITERAL_NODE_TYPES): # not e.g. type(x) == type([]) return - self.add_message("unidiomatic-typecheck", node=node) From 74c2fa7a88192937cc3119092993303ef8f3ef64 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 4 May 2025 14:48:08 +0200 Subject: [PATCH 10/10] fix functional tests --- tests/functional/u/unidiomatic_typecheck.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/u/unidiomatic_typecheck.txt b/tests/functional/u/unidiomatic_typecheck.txt index 15c898c101..76369310a6 100644 --- a/tests/functional/u/unidiomatic_typecheck.txt +++ b/tests/functional/u/unidiomatic_typecheck.txt @@ -10,15 +10,15 @@ unidiomatic-typecheck:16:4:16:20:simple_inference_positives:Use isinstance() rat unidiomatic-typecheck:17:4:17:24:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:18:4:18:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:19:4:19:20:simple_inference_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:75:4:75:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:76:4:76:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:77:4:77:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:78:4:78:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:79:4:79:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:80:4:80:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:81:4:81:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED -unidiomatic-typecheck:82:4:82:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:82:4:82:27:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:83:4:83:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:84:4:84:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:85:4:85:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED unidiomatic-typecheck:86:4:86:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:87:4:87:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED +unidiomatic-typecheck:88:4:88:23:type_of_literals_positives:Use isinstance() rather than type() for a typecheck.:UNDEFINED