introduce violation WPS358 #1259
Conversation
| @@ -12,9 +15,13 @@ | |||
| tuple_assignment1 = 'first, {0} = (1, 2)' | |||
| tuple_assignment1 = '{0}, second = (1, 2)' | |||
|
|
|||
| list_assignment = '[first, second] = (1, 2)' | |||
There was a problem hiding this comment.
What about cases like these?
>>> [a, b], c = ((1, 2), 3)
>>> a, [b, c] = (1, (2, 3))| @@ -274,6 +276,12 @@ def _check_unpacking_targets( | |||
| best_practices.WrongUnpackingViolation(node), | |||
| ) | |||
|
|
|||
| def _check_unpacking_target_type(self, node: ast.Assign) -> None: | |||
| if isinstance(node.targets[0], ast.List): | |||
There was a problem hiding this comment.
Turns out we need to check all elements inside targets because of:
>>> [a, b], c = ((1, 2), 3)
>>> a, [b, c] = (1, (2, 3))There was a problem hiding this comment.
that's true. Should we stop detection on first spotted list or raise violations for all?
There was a problem hiding this comment.
unpacking to nested list actually raise WPS414 - WrongUnpackingViolation.
Should the second violation be raised?
I think we should do one of the following then:
- update my PR to raise
WPS414and updateWPS414violations docstrings (I dislike this, because knowWPS414can be raised in 2 totally different cases) - change name of
WPS414to something less confusing and more compliant with its docstring title (Forbids to have tuple unpacking with side-effects.) and refactor check raising this violation, because unpacking to list doesn't have any side effects if it's not dynamic e.ga, b[0] = (1, 3)butb[0]is not parsed asast.Listbyast
What do you think?
There was a problem hiding this comment.
I think that these two violations should be independent. And both of them can be raised at the same time.
Pull Request Test Coverage Report for Build 2820
馃挍 - Coveralls |
| spread_assignment1 = '{0}, *second = [1, 2, 3]' | ||
| spread_assignment2 = 'first, *{0} = [1, 2, 3]' | ||
|
|
||
| spread_assignment_in_list = '[first, *rest] = [1, 2, 3]' | ||
|
|
||
| nested_spread_assignment_in_list = 'first, [second, *rest] = [1, (2, 3, 4)]' | ||
|
|
||
| multiple_level_nested_spread_assign_in_list = ( |
There was a problem hiding this comment.
One more case to test: z = (a, [b, c]) = (1, (2, 3))
There was a problem hiding this comment.
I have added support for such cases and much more in new commits.
I am happy that unpacking iterables is not supported when using walrus operator 馃槃
Add violation WPS358 forbidding unpacking iterables to lists.
Check for WPS358 in all assignments targets and in following nodes/expressions: + for loops + list comprehensions + generator expressions + set comprehensions + dict comprehensions + with statements
Rename WPS358 violation from ``IterableUnpackingToListViolation`` to ``UnpackingIterableToListViolation``.
Codecov Report
@@ Coverage Diff @@
## master #1259 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 105 105
Lines 5485 5499 +14
Branches 1209 1213 +4
=========================================
+ Hits 5485 5499 +14
Continue to review full report at Codecov.
|
Add violation WPS358 forbidding unpacking iterables to lists.
I have made things!
Checklist
CHANGELOG.mdRelated issues
Closes #1256
馃檹 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.