Skip to content

Fix #5112: Prevent used-before-assignment if named expression found first in container #5812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
44 changes: 23 additions & 21 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -2074,6 +2055,27 @@ 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
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)
)

@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."""
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/u/undefined/undefined_variable_py38.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,17 @@ 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_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():
"""2-element list where named expression comes too late"""
return [val3, val3 if (val3 := 'something') else 'anything'] # [used-before-assignment]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pff... Man that's convoluted. Can't we raise an don't-write-code-like-this warning here? πŸ˜…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I had to take 2-3mn and run the code to understand myself πŸ˜„ But do we really want to make the kind of person that would write this, flock to our bug tracker to complain πŸ˜‰ ?

1 change: 1 addition & 0 deletions tests/functional/u/undefined/undefined_variable_py38.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:171:12:171:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH