Skip to content

False negative - tuple unpacking #5708

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
12 changes: 8 additions & 4 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ Release date: TBA

Closes #5281

* Fix false positive - Allow unpacking of ``self`` in a subclass of ``typing.NamedTuple``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for the commit history I would be better to leave this where it currently is, right? This is not a blocking issue for me though.


Closes #5312

* Fixed false negative ``unpacking-non-sequence`` when value is an empty list.

Closes #5707

* Better warning messages for useless else or elif when a function returns early.

Closes #5614
Expand Down Expand Up @@ -294,10 +302,6 @@ Release date: TBA

Closes #2399

* Fix false positive - Allow unpacking of ``self`` in a subclass of ``typing.NamedTuple``.

Closes #5312

* Fix false negative for ``consider-iterating-dictionary`` during membership checks encapsulated in iterables
or ``not in`` checks

Expand Down
8 changes: 8 additions & 0 deletions doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,14 @@ Other Changes

Closes #3781

* Fix false positive - Allow unpacking of ``self`` in a subclass of ``typing.NamedTuple``.

Closes #5312

* Fixed false negative ``unpacking-non-sequence`` when value is an empty list.

Closes #5707

* Fixed false positive for ``global-variable-not-assigned`` when the ``del`` statement is used

Closes #5333
22 changes: 13 additions & 9 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -2533,15 +2533,8 @@ def _check_unpacking(self, inferred, node, targets):
return

# Attempt to check unpacking is properly balanced
values: Optional[List] = None
if isinstance(inferred, (nodes.Tuple, nodes.List)):
values = inferred.itered()
elif isinstance(inferred, astroid.Instance) and any(
ancestor.qname() == "typing.NamedTuple" for ancestor in inferred.ancestors()
):
values = [i for i in inferred.values() if isinstance(i, nodes.AssignName)]

if values:
values = self._nodes_to_unpack(inferred)
if values is not None:
if len(targets) != len(values):
# Check if we have starred nodes.
if any(isinstance(target, nodes.Starred) for target in targets):
Expand All @@ -2563,6 +2556,17 @@ def _check_unpacking(self, inferred, node, targets):
args=(_get_unpacking_extra_info(node, inferred),),
)

@staticmethod
def _nodes_to_unpack(node: nodes.NodeNG) -> Optional[List[nodes.NodeNG]]:
"""Return the list of values of the `Assign` node"""
if isinstance(node, (nodes.Tuple, nodes.List)):
return node.itered()
if isinstance(node, astroid.Instance) and any(
ancestor.qname() == "typing.NamedTuple" for ancestor in node.ancestors()
):
return [i for i in node.values() if isinstance(i, nodes.AssignName)]
return None

def _check_module_attrs(self, node, module, module_names):
"""check that module_names (list of string) are accessible through the
given module
Expand Down
65 changes: 47 additions & 18 deletions tests/functional/u/unbalanced_tuple_unpacking.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,85 +5,99 @@

# pylint: disable=missing-class-docstring, missing-function-docstring, using-constant-test, useless-object-inheritance,import-outside-toplevel


def do_stuff():
"""This is not right."""
first, second = 1, 2, 3 # [unbalanced-tuple-unpacking]
first, second = 1, 2, 3 # [unbalanced-tuple-unpacking]
return first + second


def do_stuff1():
"""This is not right."""
first, second = [1, 2, 3] # [unbalanced-tuple-unpacking]
first, second = [1, 2, 3] # [unbalanced-tuple-unpacking]
return first + second


def do_stuff2():
"""This is not right."""
(first, second) = 1, 2, 3 # [unbalanced-tuple-unpacking]
(first, second) = 1, 2, 3 # [unbalanced-tuple-unpacking]
return first + second


def do_stuff3():
"""This is not right."""
first, second = range(100)
return first + second


def do_stuff4():
""" This is right """
"""This is right"""
first, second = 1, 2
return first + second


def do_stuff5():
""" This is also right """
"""This is also right"""
first, second = (1, 2)
return first + second


def do_stuff6():
""" This is right """
"""This is right"""
(first, second) = (1, 2)
return first + second


def temp():
""" This is not weird """
"""This is not weird"""
if True:
return [1, 2]
return [2, 3, 4]


def do_stuff7():
""" This is not right, but we're not sure """
"""This is not right, but we're not sure"""
first, second = temp()
return first + second


def temp2():
""" This is weird, but correct """
"""This is weird, but correct"""
if True:
return (1, 2)

if True:
return (2, 3)
return (4, 5)


def do_stuff8():
""" This is correct """
"""This is correct"""
first, second = temp2()
return first + second


def do_stuff9():
""" This is not correct """
first, second = unpack() # [unbalanced-tuple-unpacking]
"""This is not correct"""
first, second = unpack() # [unbalanced-tuple-unpacking]
return first + second


class UnbalancedUnpacking(object):
""" Test unbalanced tuple unpacking in instance attributes. """
"""Test unbalanced tuple unpacking in instance attributes."""

# pylint: disable=attribute-defined-outside-init, invalid-name, too-few-public-methods
def test(self):
""" unpacking in instance attributes """
"""unpacking in instance attributes"""
# we're not sure if temp() returns two or three values
# so we shouldn't emit an error
self.a, self.b = temp()
self.a, self.b = temp2()
self.a, self.b = unpack() # [unbalanced-tuple-unpacking]
self.a, self.b = unpack() # [unbalanced-tuple-unpacking]


def issue329(*args):
""" Don't emit unbalanced tuple unpacking if the
"""Don't emit unbalanced tuple unpacking if the
rhs of the assignment is a variable-length argument,
because we don't know the actual length of the tuple.
"""
Expand All @@ -97,6 +111,7 @@ def test_decimal():
See astroid https://bitbucket.org/logilab/astroid/issues/92/
"""
from decimal import Decimal

dec = Decimal(2)
first, second, third = dec.as_tuple()
return first, second, third
Expand All @@ -105,6 +120,7 @@ def test_decimal():
def test_issue_559():
"""Test that we don't have a false positive wrt to issue #559."""
from ctypes import c_int

root_x, root_y, win_x, win_y = [c_int()] * 4
return root_x, root_y, win_x, win_y

Expand All @@ -121,10 +137,23 @@ def my_sum(self):

def sum_unpack_3_into_4(self):
"""Attempt to unpack 3 variables into 4"""
first, second, third, fourth = self # [unbalanced-tuple-unpacking]
first, second, third, fourth = self # [unbalanced-tuple-unpacking]
return first + second + third + fourth

def sum_unpack_3_into_2(self):
"""Attempt to unpack 3 variables into 2"""
first, second = self # [unbalanced-tuple-unpacking]
first, second = self # [unbalanced-tuple-unpacking]
return first + second


def my_function(mystring):
"""The number of items on the right-hand-side of the assignment to this function is not known"""
mylist = []
for item in mystring:
mylist.append(item)
return mylist


a, b = my_function("12") # [unbalanced-tuple-unpacking]
c = my_function("12")
d, *_ = my_function("12")
15 changes: 8 additions & 7 deletions tests/functional/u/unbalanced_tuple_unpacking.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
unbalanced-tuple-unpacking:10:4:10:27:do_stuff:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:15:4:15:29:do_stuff1:"Possible unbalanced tuple unpacking with sequence [1, 2, 3]: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:20:4:20:29:do_stuff2:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:70:4:70:28:do_stuff9:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:82:8:82:33:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:124:8:124:43:MyClass.sum_unpack_3_into_4:"Possible unbalanced tuple unpacking with sequence defined at line 112: left side has 4 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:129:8:129:28:MyClass.sum_unpack_3_into_2:"Possible unbalanced tuple unpacking with sequence defined at line 112: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:11:4:11:27:do_stuff:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:17:4:17:29:do_stuff1:"Possible unbalanced tuple unpacking with sequence [1, 2, 3]: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:23:4:23:29:do_stuff2:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:82:4:82:28:do_stuff9:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:96:8:96:33:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:140:8:140:43:MyClass.sum_unpack_3_into_4:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 4 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:145:8:145:28:MyClass.sum_unpack_3_into_2:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 2 label(s), right side has 3 value(s)":UNDEFINED
unbalanced-tuple-unpacking:157:0:157:24::"Possible unbalanced tuple unpacking with sequence defined at line 151: left side has 2 label(s), right side has 0 value(s)":UNDEFINED