Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
1824 add getting first element using unpacking violation #1927
Conversation
uhbif19
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1927 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 120 120
Lines 6377 6394 +17
Branches 1421 1424 +3
=========================================
+ Hits 6377 6394 +17
Continue to review full report at Codecov.
|
uhbif19
changed the title
| """ | ||
|
|
||
| error_template = 'Found incorrect unpacking target' |
There was a problem hiding this comment.
This message is identical to WrongUnpackingViolation.error_template, let's change it to something more related to this violation, e.g.:
| error_template = 'Found incorrect unpacking target' | |
| error_template = 'Found wrong retrieving of the first element' |
There was a problem hiding this comment.
Thanks, I forget to fix this after copy-paste. Done.
CHANGELOG.md
Outdated
|
|
||
| ### Features | ||
|
|
||
| * Forbids getting first element of list by unpacking |
There was a problem hiding this comment.
Done.
Maybe if the project have MD style rules, it can use some linter? I found rule that seems to fix bullet list style: https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md#md004---unordered-list-style
I may work on adding linter to the project.
There was a problem hiding this comment.
You can check markdown-lint action by reviewdog- https://github.com/reviewdog/action-markdownlint
| len(targets) == 2 and | ||
| isinstance(targets[0], ast.Name) and | ||
| isinstance(targets[1], ast.Starred) and | ||
| _is_unused_variable_name(targets[1].value) |
There was a problem hiding this comment.
Let's refactor this part to use: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/logic/naming/access.py#L9
There was a problem hiding this comment.
@sobolevn I would like too, but in "Feedback" section of PR I described, why this do not works.
There was a problem hiding this comment.
Oh, sorry. Let's use is_protected then
There was a problem hiding this comment.
This does not work too:
>>> is_protected('_')
False
>>> is_protected('__')
False
While access.is_unused(node.id) or access.is_protected(node.id) works.
But for me such code looks misleading. Would you consider adding different "weak" kind of is_unused in naming.access?
There was a problem hiding this comment.
Ok, then let's create a new function access.looks_like_unused()
I have made things!
This code only checks unpacking for variable assignment case outlined in issue.
I do not cover cases like
for a, *_r in smthgora, *_b, *_cbecause it seems unlikely that somebody will use them.If you think that my assumption is not correct, I will update the code.
Checklist
CHANGELOG.mdRelated issues
Feedback
The function
logic.naming.access.is_unuseduses the term "unused" differently. It says that_restis not unused, but "private" name, while in issue such name is classified as unused.Related code uses term
targetin two different senses.One is for
target1 = target2 = ..., the same like ast module does.Second one is for `target1, target2 = ...', which conflicts with first sence.
I think that renaming second thing will clarify situation.