From ef78ee7ebcd0fa973edee5efcc83afb9ca609b3d Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 14 Feb 2022 07:39:57 -0500 Subject: [PATCH 1/5] Fix #5112: Prevent `used-before-assignment` if named expr found first in container --- ChangeLog | 5 ++ doc/whatsnew/2.13.rst | 5 ++ pylint/checkers/variables.py | 46 ++++++++++--------- .../u/undefined/undefined_variable_py38.py | 9 ++++ .../u/undefined/undefined_variable_py38.txt | 1 + 5 files changed, 45 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index e27ef4cb27..2201d37e90 100644 --- a/ChangeLog +++ b/ChangeLog @@ -205,6 +205,11 @@ Release date: TBA Closes #367 +* Fixed a false positive for ``used-before-assignment`` when a named expression + appears as the first value in a container. + + Closes #5112 + * ``used-before-assignment`` now assumes that assignments in except blocks may not have occurred and warns accordingly. diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 57afc70fa2..8714c23c19 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -241,6 +241,11 @@ Other Changes Closes #3793 +* Fixed a false positive for ``used-before-assignment`` when a named expression + appears as the first value in a container. + + Closes #5112 + * Fixed false positive for ``used-before-assignment`` with self-referential type annotation in conditional statements within class methods. diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 90eee481f0..ba4b733f41 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -1971,7 +1971,7 @@ def _is_variable_violation( # same line as the function definition maybe_before_assign = False elif ( - isinstance( # pylint: disable=too-many-boolean-expressions + isinstance( defstmt, ( nodes.Assign, @@ -1981,26 +1981,7 @@ def _is_variable_violation( nodes.Return, ), ) - and ( - isinstance(defstmt.value, nodes.IfExp) - or ( - isinstance(defstmt.value, nodes.Lambda) - and isinstance(defstmt.value.body, nodes.IfExp) - ) - or ( - isinstance(defstmt.value, nodes.Call) - and ( - any( - isinstance(kwarg.value, nodes.IfExp) - for kwarg in defstmt.value.keywords - ) - or any( - isinstance(arg, nodes.IfExp) - for arg in defstmt.value.args - ) - ) - ) - ) + and VariablesChecker._maybe_used_and_assigned_at_once(defstmt) and frame is defframe and defframe.parent_of(node) and stmt is defstmt @@ -2074,6 +2055,29 @@ def _is_variable_violation( return maybe_before_assign, annotation_return, use_outer_definition + @staticmethod + def _maybe_used_and_assigned_at_once(defstmt: nodes.Statement) -> bool: + """Check if `defstmt` has the potential to use and assign a name in the + same statement. + """ + if isinstance(defstmt.value, nodes.BaseContainer) and defstmt.value.elts: + # The assignment must happen as part of the first element + # e.g. "assert x:= True, x" + # NOT "assert x, x:= True" + value = defstmt.value.elts[0] + else: + value = defstmt.value + if isinstance(value, nodes.IfExp): + return True + if isinstance(value, nodes.Lambda) and isinstance(value.body, nodes.IfExp): + return True + if isinstance(value, nodes.Call) and ( + any(isinstance(kwarg.value, nodes.IfExp) for kwarg in value.keywords) + or any(isinstance(arg, nodes.IfExp) for arg in value.args) + ): + return True + return False + @staticmethod def _is_only_type_assignment(node: nodes.Name, defstmt: nodes.Statement) -> bool: """Check if variable only gets assigned a type and never a value.""" diff --git a/tests/functional/u/undefined/undefined_variable_py38.py b/tests/functional/u/undefined/undefined_variable_py38.py index 924eb4a431..a1db8c46e0 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.py +++ b/tests/functional/u/undefined/undefined_variable_py38.py @@ -155,3 +155,12 @@ def __init__(self, value): dummy = Dummy(value=val if (val := 'something') else 'anything') + +def expression_in_ternary_operator_inside_container(): + """Named expression in ternary operator: inside container""" + return [val2 if (val2 := 'something') else 'anything'] + + +def expression_in_ternary_operator_inside_container_wrong_position(): + """Same case, but named expression comes too late""" + return [val3, val3 if (val3 := 'something') else 'anything'] # [used-before-assignment] diff --git a/tests/functional/u/undefined/undefined_variable_py38.txt b/tests/functional/u/undefined/undefined_variable_py38.txt index 43a2b68290..9ea3699a97 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.txt +++ b/tests/functional/u/undefined/undefined_variable_py38.txt @@ -5,3 +5,4 @@ undefined-variable:99:6:99:19::Undefined variable 'else_assign_2':INFERENCE unused-variable:126:4:126:10:type_annotation_unused_after_comprehension:Unused variable 'my_int':UNDEFINED used-before-assignment:134:10:134:16:type_annotation_used_improperly_after_comprehension:Using variable 'my_int' before assignment:HIGH used-before-assignment:141:10:141:16:type_annotation_used_improperly_after_comprehension_2:Using variable 'my_int' before assignment:HIGH +used-before-assignment:166:12:166:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH From ee9426d0b03f2be38bd67ea35f9acd5696197382 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 16 Feb 2022 08:46:59 -0500 Subject: [PATCH 2/5] Add test --- tests/functional/u/undefined/undefined_variable_py38.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/functional/u/undefined/undefined_variable_py38.py b/tests/functional/u/undefined/undefined_variable_py38.py index a1db8c46e0..01e0201a81 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.py +++ b/tests/functional/u/undefined/undefined_variable_py38.py @@ -161,6 +161,11 @@ def expression_in_ternary_operator_inside_container(): return [val2 if (val2 := 'something') else 'anything'] +def expression_in_ternary_operator_inside_container_tuple(): + """Same case, using a tuple inside a 1-element list""" + return [(val3, val3) if (val3 := 'something') else 'anything'] + + def expression_in_ternary_operator_inside_container_wrong_position(): - """Same case, but named expression comes too late""" + """2-element list where named expression comes too late""" return [val3, val3 if (val3 := 'something') else 'anything'] # [used-before-assignment] From 94dc95ea5bc5b595ff59c50f7bcfa70be613964a Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 16 Feb 2022 08:47:48 -0500 Subject: [PATCH 3/5] Update output --- tests/functional/u/undefined/undefined_variable_py38.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/u/undefined/undefined_variable_py38.txt b/tests/functional/u/undefined/undefined_variable_py38.txt index 9ea3699a97..4cce67ecc6 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.txt +++ b/tests/functional/u/undefined/undefined_variable_py38.txt @@ -5,4 +5,4 @@ undefined-variable:99:6:99:19::Undefined variable 'else_assign_2':INFERENCE unused-variable:126:4:126:10:type_annotation_unused_after_comprehension:Unused variable 'my_int':UNDEFINED used-before-assignment:134:10:134:16:type_annotation_used_improperly_after_comprehension:Using variable 'my_int' before assignment:HIGH used-before-assignment:141:10:141:16:type_annotation_used_improperly_after_comprehension_2:Using variable 'my_int' before assignment:HIGH -used-before-assignment:166:12:166:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH +used-before-assignment:171:12:171:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH From 4a8816ccdad457ffd874d5d9fb8efae691c2fe0e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 16 Feb 2022 08:48:28 -0500 Subject: [PATCH 4/5] Simplify return Co-authored-by: Pierre Sassoulas --- pylint/checkers/variables.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index ba4b733f41..1eb728362d 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2071,12 +2071,10 @@ def _maybe_used_and_assigned_at_once(defstmt: nodes.Statement) -> bool: return True if isinstance(value, nodes.Lambda) and isinstance(value.body, nodes.IfExp): return True - if isinstance(value, nodes.Call) and ( + return isinstance(value, nodes.Call) and ( any(isinstance(kwarg.value, nodes.IfExp) for kwarg in value.keywords) or any(isinstance(arg, nodes.IfExp) for arg in value.args) - ): - return True - return False + ) @staticmethod def _is_only_type_assignment(node: nodes.Name, defstmt: nodes.Statement) -> bool: From 68179fe350a74445514866b14b003586593d2b05 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Thu, 17 Feb 2022 14:57:40 -0500 Subject: [PATCH 5/5] Fix example code --- pylint/checkers/variables.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 1eb728362d..b5363249ed 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2062,8 +2062,8 @@ def _maybe_used_and_assigned_at_once(defstmt: nodes.Statement) -> bool: """ if isinstance(defstmt.value, nodes.BaseContainer) and defstmt.value.elts: # The assignment must happen as part of the first element - # e.g. "assert x:= True, x" - # NOT "assert x, x:= True" + # e.g. "assert (x:= True), x" + # NOT "assert x, (x:= True)" value = defstmt.value.elts[0] else: value = defstmt.value