Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #1684 #1850

Merged
merged 3 commits into from Feb 6, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -89,7 +89,6 @@ def wrapper():
tuple_assignment1,
tuple_assignment2,
spread_assignment1,
spread_assignment2,
for_assignment,
for_unpacking1,
for_unpacking2,
Expand All @@ -101,6 +100,10 @@ def wrapper():
with_unpacking1,
with_unpacking2,
])
@pytest.mark.parametrize('definition', [
'some_name',
'(a, b)', # tuple
])
@pytest.mark.parametrize('target', [
'(1, 2)',
'[1, 2]',
Expand All @@ -109,12 +112,41 @@ def test_correct_unpacking(
assert_errors,
parse_ast_tree,
code,
definition,
target,
default_options,
mode,
):
"""Testing that correct assignments work."""
tree = parse_ast_tree(mode(code.format(definition, target)))

visitor = WrongAssignmentVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])


@pytest.mark.parametrize('code', [
spread_assignment2,
])
@pytest.mark.parametrize('definition', [
'some_name',
])
@pytest.mark.parametrize('target', [
'(1, 2)',
'[1, 2]',
])
def test_correct_spread_unpacking(
assert_errors,
parse_ast_tree,
code,
definition,
target,
default_options,
mode,
):
"""Testing that correct assignments work."""
tree = parse_ast_tree(mode(code.format('some_name', target)))
tree = parse_ast_tree(mode(code.format(definition, target)))

visitor = WrongAssignmentVisitor(default_options, tree=tree)
visitor.run()
Expand Down
Expand Up @@ -69,6 +69,9 @@ def test_wrong_definitions_in_comprehension(
'(y, xy)',
'(first, *star)',
'(first, second, *star)',
# regression 1684
'first, (second, third)',
'(first, second), third',
])
def test_comprehension_without_bad_definitions(
assert_errors,
Expand Down
12 changes: 11 additions & 1 deletion wemake_python_styleguide/logic/tree/variables.py
Expand Up @@ -16,12 +16,22 @@ def is_valid_block_variable_definition(node: _VarDefinition) -> bool:
"""Is used to check either block variables are correctly defined."""
if isinstance(node, ast.Tuple):
return all(
_is_valid_single(var_definition)
is_valid_block_variable_definition(var_definition)
for var_definition in node.elts
)
return _is_valid_single(node)


def is_valid_unpacking_target(target: ast.expr) -> bool:
"""Checks if unpacking target is correct."""
if isinstance(target, ast.Tuple):
return all(
_is_valid_single(element) is not None
Copy link
Collaborator

Choose a reason for hiding this comment

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

_is_valid_single returns bool so why the result is compared to None?
Shouldn't it be just _is_valid_single(element)?

for element in target.elts
)
return _is_valid_single(target)


def _is_valid_single(node: _VarDefinition) -> bool:
return (
isinstance(node, ast.Name) or
Expand Down
8 changes: 3 additions & 5 deletions wemake_python_styleguide/visitors/ast/builtins.py
Expand Up @@ -31,6 +31,7 @@
functions,
operators,
strings,
variables,
)
from wemake_python_styleguide.types import (
AnyChainable,
Expand Down Expand Up @@ -420,18 +421,15 @@ def _check_assign_targets(self, node: ast.Assign) -> None:
def _check_unpacking_targets(
self,
node: ast.AST,
targets: Iterable[ast.AST],
targets: List[ast.expr],
) -> None:
targets = tuple(targets)

if len(targets) == 1:
self.add_violation(
best_practices.SingleElementDestructuringViolation(node),
)

for target in targets:
target_name = extract_name(target)
if target_name is None: # it means, that non name node was used
if not variables.is_valid_unpacking_target(target):
self.add_violation(
best_practices.WrongUnpackingViolation(node),
)
Expand Down
2 changes: 1 addition & 1 deletion wemake_python_styleguide/visitors/ast/keywords.py
Expand Up @@ -11,7 +11,7 @@
from wemake_python_styleguide.logic import walk
from wemake_python_styleguide.logic.naming import name_nodes
from wemake_python_styleguide.logic.nodes import get_parent
from wemake_python_styleguide.logic.tree import keywords, operators
from wemake_python_styleguide.logic.tree import keywords, operators, variables
Copy link
Contributor

Choose a reason for hiding this comment

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

[pep8] reported by reviewdog 馃惗
F401 'wemake_python_styleguide.logic.tree.variables' imported but unused

from wemake_python_styleguide.logic.tree.exceptions import get_exception_name
from wemake_python_styleguide.logic.tree.variables import (
is_valid_block_variable_definition,
Expand Down