From 21f270976e1a172a9c74f9d97d40f7ab8074e41f Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 10 Jun 2022 15:41:13 -0400 Subject: [PATCH 1/4] Emit `using-constant-test` when testing truth value of a call returning a generator Update confidence level to INFERENCE for `using-constant-test` and `missing-parentheses-for-call-in-test` --- doc/whatsnew/2/2.15/index.rst | 4 +++ pylint/checkers/base/basic_checker.py | 19 ++++++++++-- .../missing_parentheses_for_call_in_test.txt | 30 +++++++++---------- tests/functional/u/using_constant_test.py | 16 ++++++++++ tests/functional/u/using_constant_test.txt | 21 ++++++------- 5 files changed, 62 insertions(+), 28 deletions(-) diff --git a/doc/whatsnew/2/2.15/index.rst b/doc/whatsnew/2/2.15/index.rst index 02fbf47db9..cac82dd23c 100644 --- a/doc/whatsnew/2/2.15/index.rst +++ b/doc/whatsnew/2/2.15/index.rst @@ -36,6 +36,10 @@ False negatives fixed Closes #6648 +* Emit ``using-constant-test`` when testing the truth value of a call that only returns generators. + + Closes #6909 + Other bug fixes =============== diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index bd78c2d4f1..7f8c13f978 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -17,7 +17,7 @@ from pylint import utils as lint_utils from pylint.checkers import BaseChecker, utils -from pylint.interfaces import HIGH +from pylint.interfaces import HIGH, INFERENCE from pylint.reporters.ureports import nodes as reporter_nodes from pylint.utils import LinterStats @@ -317,6 +317,17 @@ def _check_using_constant_test( emit = isinstance(test, (nodes.Const,) + structs + const_nodes) if not isinstance(test, except_nodes): inferred = utils.safe_infer(test) + # Emit if the call is to a function that only returns GeneratorExp (always truthy) + elif isinstance(test, nodes.Call): + inferred_call = utils.safe_infer(test.func) + if isinstance(inferred_call, nodes.FunctionDef): + return_nodes = list(inferred_call._get_return_nodes_skip_functions()) + if return_nodes and all( + isinstance(n.value, nodes.GeneratorExp) for n in return_nodes + ): + self.add_message( + "using-constant-test", node=node, confidence=INFERENCE + ) if emit: self.add_message("using-constant-test", node=test) @@ -336,12 +347,14 @@ def _check_using_constant_test( for inf_call in call_inferred: if inf_call != astroid.Uninferable: self.add_message( - "missing-parentheses-for-call-in-test", node=test + "missing-parentheses-for-call-in-test", + node=test, + confidence=INFERENCE, ) break except astroid.InferenceError: pass - self.add_message("using-constant-test", node=test) + self.add_message("using-constant-test", node=test, confidence=INFERENCE) def visit_module(self, _: nodes.Module) -> None: """Check module name, docstring and required arguments.""" diff --git a/tests/functional/m/missing/missing_parentheses_for_call_in_test.txt b/tests/functional/m/missing/missing_parentheses_for_call_in_test.txt index f782a9704a..02566e43ce 100644 --- a/tests/functional/m/missing/missing_parentheses_for_call_in_test.txt +++ b/tests/functional/m/missing/missing_parentheses_for_call_in_test.txt @@ -1,15 +1,15 @@ -missing-parentheses-for-call-in-test:29:3:29:16::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:35:3:35:19::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:43:3:43:23::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:51:5:51:25::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:56:3:56:14::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:64:3:64:17::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:70:17:70:30::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:72:23:72:34::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:73:24:73:38::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:75:26:75:39::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:76:26:76:37::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:79:26:79:40::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:80:26:80:42::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:85:3:85:26::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED -missing-parentheses-for-call-in-test:91:3:91:20::Using a conditional statement with potentially wrong function or method call due to missing parentheses:UNDEFINED +missing-parentheses-for-call-in-test:29:3:29:16::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:35:3:35:19::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:43:3:43:23::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:51:5:51:25::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:56:3:56:14::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:64:3:64:17::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:70:17:70:30::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:72:23:72:34::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:73:24:73:38::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:75:26:75:39::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:76:26:76:37::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:79:26:79:40::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:80:26:80:42::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:85:3:85:26::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE +missing-parentheses-for-call-in-test:91:3:91:20::Using a conditional statement with potentially wrong function or method call due to missing parentheses:INFERENCE diff --git a/tests/functional/u/using_constant_test.py b/tests/functional/u/using_constant_test.py index 78974681ee..b83a01f5d5 100644 --- a/tests/functional/u/using_constant_test.py +++ b/tests/functional/u/using_constant_test.py @@ -147,3 +147,19 @@ def test_good_comprehension_checks(): {data for data in range(100) if abs(data)} {data: 1 for data in range(100) if data} {data: 1 for data in range(100)} + + +# Calls to functions returning generator expressions are always truthy +def get_generator(): + return (x for x in range(0)) + +if get_generator(): # [using-constant-test] + pass + +def maybe_get_generator(arg): + if arg: + return (x for x in range(0)) + return None + +if maybe_get_generator(None): + pass diff --git a/tests/functional/u/using_constant_test.txt b/tests/functional/u/using_constant_test.txt index e311cae55f..8101edfe4f 100644 --- a/tests/functional/u/using_constant_test.txt +++ b/tests/functional/u/using_constant_test.txt @@ -1,8 +1,8 @@ -using-constant-test:22:3:22:14::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:22:3:22:14::Using a conditional statement with a constant value:INFERENCE using-constant-test:26:3:26:31::Using a conditional statement with a constant value:UNDEFINED using-constant-test:29:3:29:15::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:32:3:32:11::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:35:3:35:8::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:32:3:32:11::Using a conditional statement with a constant value:INFERENCE +using-constant-test:35:3:35:8::Using a conditional statement with a constant value:INFERENCE using-constant-test:38:3:38:4::Using a conditional statement with a constant value:UNDEFINED using-constant-test:41:3:41:7::Using a conditional statement with a constant value:UNDEFINED using-constant-test:44:3:44:5::Using a conditional statement with a constant value:UNDEFINED @@ -14,14 +14,15 @@ using-constant-test:59:3:59:12::Using a conditional statement with a constant va using-constant-test:62:3:62:5::Using a conditional statement with a constant value:UNDEFINED using-constant-test:65:3:65:12::Using a conditional statement with a constant value:UNDEFINED using-constant-test:68:3:68:5::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:73:3:73:12::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:73:3:73:12::Using a conditional statement with a constant value:INFERENCE using-constant-test:76:8:76:9::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:80:36:80:39:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED +using-constant-test:80:36:80:39:test_comprehensions:Using a conditional statement with a constant value:INFERENCE using-constant-test:81:36:81:37:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED -using-constant-test:82:36:82:39:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED +using-constant-test:82:36:82:39:test_comprehensions:Using a conditional statement with a constant value:INFERENCE using-constant-test:83:36:83:37:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED -using-constant-test:84:36:84:39:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED -using-constant-test:85:39:85:42:test_comprehensions:Using a conditional statement with a constant value:UNDEFINED -using-constant-test:89:3:89:15::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:93:3:93:18::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:84:36:84:39:test_comprehensions:Using a conditional statement with a constant value:INFERENCE +using-constant-test:85:39:85:42:test_comprehensions:Using a conditional statement with a constant value:INFERENCE +using-constant-test:89:3:89:15::Using a conditional statement with a constant value:INFERENCE +using-constant-test:93:3:93:18::Using a conditional statement with a constant value:INFERENCE comparison-of-constants:117:3:117:8::"Comparison between constants: '2 < 3' has a constant value":HIGH +using-constant-test:156:0:157:8::Using a conditional statement with a constant value:INFERENCE From bf7f322c9d9226a33da124a883c4435e96772ee7 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 10 Jun 2022 16:11:18 -0400 Subject: [PATCH 2/4] don't say "truthy" --- pylint/checkers/base/basic_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index 7f8c13f978..a3f583c4c5 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -317,7 +317,7 @@ def _check_using_constant_test( emit = isinstance(test, (nodes.Const,) + structs + const_nodes) if not isinstance(test, except_nodes): inferred = utils.safe_infer(test) - # Emit if the call is to a function that only returns GeneratorExp (always truthy) + # Emit if calling a function that only returns GeneratorExp (always tests True) elif isinstance(test, nodes.Call): inferred_call = utils.safe_infer(test.func) if isinstance(inferred_call, nodes.FunctionDef): From cc9054101ca938f0fdc91d963248fd9e4f16cc37 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 10 Jun 2022 16:50:59 -0400 Subject: [PATCH 3/4] avoid two iterations --- pylint/checkers/base/basic_checker.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index a3f583c4c5..79e4191cbc 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -321,10 +321,13 @@ def _check_using_constant_test( elif isinstance(test, nodes.Call): inferred_call = utils.safe_infer(test.func) if isinstance(inferred_call, nodes.FunctionDef): - return_nodes = list(inferred_call._get_return_nodes_skip_functions()) - if return_nodes and all( - isinstance(n.value, nodes.GeneratorExp) for n in return_nodes - ): + all_returns_were_generator = None + for return_node in inferred_call._get_return_nodes_skip_functions(): + if not isinstance(return_node.value, nodes.GeneratorExp): + all_returns_were_generator = False + break + all_returns_were_generator = True + if all_returns_were_generator: self.add_message( "using-constant-test", node=node, confidence=INFERENCE ) From 7272b431a37adcf0e691ec569e4478414b5715e6 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 11 Jun 2022 09:01:26 -0400 Subject: [PATCH 4/4] add comment --- pylint/checkers/base/basic_checker.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index 79e4191cbc..0fcac26af9 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -321,6 +321,8 @@ def _check_using_constant_test( elif isinstance(test, nodes.Call): inferred_call = utils.safe_infer(test.func) if isinstance(inferred_call, nodes.FunctionDef): + # Can't use all(x) or not any(not x) for this condition, because it + # will return True for empty generators, which is not what we want. all_returns_were_generator = None for return_node in inferred_call._get_return_nodes_skip_functions(): if not isinstance(return_node.value, nodes.GeneratorExp):