From 05b4ce9786b8f434c6cf1c8d9f2458df8d7b49e9 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 11 Jun 2022 12:46:49 -0400 Subject: [PATCH 1/4] Emit `using-constant-test` when testing truth value of a variable holding a generator --- doc/whatsnew/2/2.15/index.rst | 4 ++++ pylint/checkers/base/basic_checker.py | 28 +++++++++++++++++++++- tests/functional/u/using_constant_test.py | 13 ++++++++++ tests/functional/u/using_constant_test.txt | 2 ++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/doc/whatsnew/2/2.15/index.rst b/doc/whatsnew/2/2.15/index.rst index cac82dd23c..d800786a3f 100644 --- a/doc/whatsnew/2/2.15/index.rst +++ b/doc/whatsnew/2/2.15/index.rst @@ -40,6 +40,10 @@ False negatives fixed Closes #6909 +* Emit ``using-constant-test`` when testing the truth value of a variable holding a generator. + + Refs #6909 + Other bug fixes =============== diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index 0fcac26af9..83fb6133e3 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -315,11 +315,37 @@ def _check_using_constant_test( ) inferred = None emit = isinstance(test, (nodes.Const,) + structs + const_nodes) + maybe_generator_call = None if not isinstance(test, except_nodes): inferred = utils.safe_infer(test) + if inferred is astroid.Uninferable and isinstance(test, nodes.Name): + lookup_result = test.frame(future=True).lookup(test.name) + if lookup_result: + maybe_generator_assigned = ( + isinstance(assign_name.parent.value, nodes.GeneratorExp) + for assign_name in lookup_result[1] + if isinstance(assign_name.parent, nodes.Assign) + ) + first_item = next(maybe_generator_assigned, None) + if first_item is not None: + # Emit if this variable is certain to hold a generator + if all( + itertools.chain((first_item,), maybe_generator_assigned) + ): + emit = True + # If this variable holds the result of a call, save it for next test + elif ( + len(lookup_result[1]) == 1 + and isinstance(lookup_result[1][0].parent, nodes.Assign) + and isinstance(lookup_result[1][0].parent.value, nodes.Call) + ): + maybe_generator_call = lookup_result[1][0].parent.value + # Emit if calling a function that only returns GeneratorExp (always tests True) elif isinstance(test, nodes.Call): - inferred_call = utils.safe_infer(test.func) + maybe_generator_call = test + if maybe_generator_call: + inferred_call = utils.safe_infer(maybe_generator_call.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. diff --git a/tests/functional/u/using_constant_test.py b/tests/functional/u/using_constant_test.py index b83a01f5d5..a3b35b3553 100644 --- a/tests/functional/u/using_constant_test.py +++ b/tests/functional/u/using_constant_test.py @@ -163,3 +163,16 @@ def maybe_get_generator(arg): if maybe_get_generator(None): pass + +y = (a for a in range(10)) +if y: # [using-constant-test] + pass + +z = (a for a in range(10)) +z = "red herring" +if z: + pass + +gen = get_generator() +if gen: # [using-constant-test] + pass diff --git a/tests/functional/u/using_constant_test.txt b/tests/functional/u/using_constant_test.txt index 8101edfe4f..eb9b69c17f 100644 --- a/tests/functional/u/using_constant_test.txt +++ b/tests/functional/u/using_constant_test.txt @@ -26,3 +26,5 @@ using-constant-test:89:3:89:15::Using a conditional statement with a constant va 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 +using-constant-test:168:3:168:4::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:177:0:178:8::Using a conditional statement with a constant value:INFERENCE From 6522e5829af3a7660fd4ad700c88a718104a2f1b Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 11 Jun 2022 13:29:46 -0400 Subject: [PATCH 2/4] refactor --- doc/whatsnew/2/2.15/index.rst | 7 ++-- pylint/checkers/base/basic_checker.py | 51 ++++++++++++++++----------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/doc/whatsnew/2/2.15/index.rst b/doc/whatsnew/2/2.15/index.rst index d800786a3f..a3e5cb8352 100644 --- a/doc/whatsnew/2/2.15/index.rst +++ b/doc/whatsnew/2/2.15/index.rst @@ -36,14 +36,11 @@ False negatives fixed Closes #6648 -* Emit ``using-constant-test`` when testing the truth value of a call that only returns generators. +* Emit ``using-constant-test`` when testing the truth value of a variable or call result + holding a generator. Closes #6909 -* Emit ``using-constant-test`` when testing the truth value of a variable holding a generator. - - Refs #6909 - Other bug fixes =============== diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index 83fb6133e3..5f493426af 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -319,27 +319,7 @@ def _check_using_constant_test( if not isinstance(test, except_nodes): inferred = utils.safe_infer(test) if inferred is astroid.Uninferable and isinstance(test, nodes.Name): - lookup_result = test.frame(future=True).lookup(test.name) - if lookup_result: - maybe_generator_assigned = ( - isinstance(assign_name.parent.value, nodes.GeneratorExp) - for assign_name in lookup_result[1] - if isinstance(assign_name.parent, nodes.Assign) - ) - first_item = next(maybe_generator_assigned, None) - if first_item is not None: - # Emit if this variable is certain to hold a generator - if all( - itertools.chain((first_item,), maybe_generator_assigned) - ): - emit = True - # If this variable holds the result of a call, save it for next test - elif ( - len(lookup_result[1]) == 1 - and isinstance(lookup_result[1][0].parent, nodes.Assign) - and isinstance(lookup_result[1][0].parent.value, nodes.Call) - ): - maybe_generator_call = lookup_result[1][0].parent.value + emit, maybe_generator_call = BasicChecker._name_holds_generator(test) # Emit if calling a function that only returns GeneratorExp (always tests True) elif isinstance(test, nodes.Call): @@ -387,6 +367,35 @@ def _check_using_constant_test( pass self.add_message("using-constant-test", node=test, confidence=INFERENCE) + @staticmethod + def _name_holds_generator(test: nodes.Name) -> tuple[bool, nodes.Call | None]: + """Return whether `test` tests a name certain to hold a generator, or optionally + a call that should be then tested to see if *it* returns only generators. + """ + assert isinstance(test, nodes.Name) + emit = False + maybe_generator_call = None + lookup_result = test.frame(future=True).lookup(test.name) + if lookup_result: + maybe_generator_assigned = ( + isinstance(assign_name.parent.value, nodes.GeneratorExp) + for assign_name in lookup_result[1] + if isinstance(assign_name.parent, nodes.Assign) + ) + first_item = next(maybe_generator_assigned, None) + if first_item is not None: + # Emit if this variable is certain to hold a generator + if all(itertools.chain((first_item,), maybe_generator_assigned)): + emit = True + # If this variable holds the result of a call, save it for next test + elif ( + len(lookup_result[1]) == 1 + and isinstance(lookup_result[1][0].parent, nodes.Assign) + and isinstance(lookup_result[1][0].parent.value, nodes.Call) + ): + maybe_generator_call = lookup_result[1][0].parent.value + return emit, maybe_generator_call + def visit_module(self, _: nodes.Module) -> None: """Check module name, docstring and required arguments.""" self.linter.stats.node_count["module"] += 1 From 41234ffa7f126eb93b7554aca74c7c255c5d6e11 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 12 Jun 2022 14:17:58 -0400 Subject: [PATCH 3/4] Update confidence --- pylint/checkers/base/basic_checker.py | 3 +- .../raise/missing_raises_doc_Google.txt | 2 +- tests/functional/u/using_constant_test.txt | 34 +++++++++---------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index 5f493426af..a008dcf1ab 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -339,9 +339,10 @@ def _check_using_constant_test( self.add_message( "using-constant-test", node=node, confidence=INFERENCE ) + return if emit: - self.add_message("using-constant-test", node=test) + self.add_message("using-constant-test", node=test, confidence=INFERENCE) elif isinstance(inferred, const_nodes): # If the constant node is a FunctionDef or Lambda then # it may be an illicit function call due to missing parentheses diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_Google.txt b/tests/functional/ext/docparams/raise/missing_raises_doc_Google.txt index 0c6022c32b..6b9ef2e8f1 100644 --- a/tests/functional/ext/docparams/raise/missing_raises_doc_Google.txt +++ b/tests/functional/ext/docparams/raise/missing_raises_doc_Google.txt @@ -11,4 +11,4 @@ missing-raises-doc:148:4:148:18:Foo.foo_method:"""AttributeError"" not documente unreachable:158:8:158:17:Foo.foo_method:Unreachable code:UNDEFINED unreachable:180:8:180:17:Foo.foo_method:Unreachable code:UNDEFINED missing-raises-doc:183:4:183:18:Foo.foo_method:"""AttributeError"" not documented as being raised":UNDEFINED -using-constant-test:190:11:190:15:Foo.foo_method:Using a conditional statement with a constant value:UNDEFINED +using-constant-test:190:11:190:15:Foo.foo_method:Using a conditional statement with a constant value:INFERENCE diff --git a/tests/functional/u/using_constant_test.txt b/tests/functional/u/using_constant_test.txt index eb9b69c17f..033bad0b0f 100644 --- a/tests/functional/u/using_constant_test.txt +++ b/tests/functional/u/using_constant_test.txt @@ -1,30 +1,30 @@ 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:26:3:26:31::Using a conditional statement with a constant value:INFERENCE +using-constant-test:29:3:29:15::Using a conditional statement with a constant value:INFERENCE 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 -using-constant-test:47:3:47:6::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:50:3:50:6::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:53:3:53:5::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:56:3:56:12::Using a conditional statement with a constant value:UNDEFINED -using-constant-test:59:3:59:12::Using a conditional statement with a constant value:UNDEFINED -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:38:3:38:4::Using a conditional statement with a constant value:INFERENCE +using-constant-test:41:3:41:7::Using a conditional statement with a constant value:INFERENCE +using-constant-test:44:3:44:5::Using a conditional statement with a constant value:INFERENCE +using-constant-test:47:3:47:6::Using a conditional statement with a constant value:INFERENCE +using-constant-test:50:3:50:6::Using a conditional statement with a constant value:INFERENCE +using-constant-test:53:3:53:5::Using a conditional statement with a constant value:INFERENCE +using-constant-test:56:3:56:12::Using a conditional statement with a constant value:INFERENCE +using-constant-test:59:3:59:12::Using a conditional statement with a constant value:INFERENCE +using-constant-test:62:3:62:5::Using a conditional statement with a constant value:INFERENCE +using-constant-test:65:3:65:12::Using a conditional statement with a constant value:INFERENCE +using-constant-test:68:3:68:5::Using a conditional statement with a constant value:INFERENCE 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:76:8:76:9::Using a conditional statement with a constant value:INFERENCE 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:81:36:81:37:test_comprehensions:Using a conditional statement with a constant value:INFERENCE 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:83:36:83:37:test_comprehensions:Using a conditional statement with a constant value:INFERENCE 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 -using-constant-test:168:3:168:4::Using a conditional statement with a constant value:UNDEFINED +using-constant-test:168:3:168:4::Using a conditional statement with a constant value:INFERENCE using-constant-test:177:0:178:8::Using a conditional statement with a constant value:INFERENCE From 3e66e2b9fc77a8e9861e47d2b32e18122168953e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 12 Jun 2022 15:05:04 -0400 Subject: [PATCH 4/4] reduce indentation --- pylint/checkers/base/basic_checker.py | 37 ++++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/pylint/checkers/base/basic_checker.py b/pylint/checkers/base/basic_checker.py index a008dcf1ab..a9bee5e79b 100644 --- a/pylint/checkers/base/basic_checker.py +++ b/pylint/checkers/base/basic_checker.py @@ -377,24 +377,25 @@ def _name_holds_generator(test: nodes.Name) -> tuple[bool, nodes.Call | None]: emit = False maybe_generator_call = None lookup_result = test.frame(future=True).lookup(test.name) - if lookup_result: - maybe_generator_assigned = ( - isinstance(assign_name.parent.value, nodes.GeneratorExp) - for assign_name in lookup_result[1] - if isinstance(assign_name.parent, nodes.Assign) - ) - first_item = next(maybe_generator_assigned, None) - if first_item is not None: - # Emit if this variable is certain to hold a generator - if all(itertools.chain((first_item,), maybe_generator_assigned)): - emit = True - # If this variable holds the result of a call, save it for next test - elif ( - len(lookup_result[1]) == 1 - and isinstance(lookup_result[1][0].parent, nodes.Assign) - and isinstance(lookup_result[1][0].parent.value, nodes.Call) - ): - maybe_generator_call = lookup_result[1][0].parent.value + if not lookup_result: + return emit, maybe_generator_call + maybe_generator_assigned = ( + isinstance(assign_name.parent.value, nodes.GeneratorExp) + for assign_name in lookup_result[1] + if isinstance(assign_name.parent, nodes.Assign) + ) + first_item = next(maybe_generator_assigned, None) + if first_item is not None: + # Emit if this variable is certain to hold a generator + if all(itertools.chain((first_item,), maybe_generator_assigned)): + emit = True + # If this variable holds the result of a call, save it for next test + elif ( + len(lookup_result[1]) == 1 + and isinstance(lookup_result[1][0].parent, nodes.Assign) + and isinstance(lookup_result[1][0].parent.value, nodes.Call) + ): + maybe_generator_call = lookup_result[1][0].parent.value return emit, maybe_generator_call def visit_module(self, _: nodes.Module) -> None: