Repository: wemake-services/wemake-python-styleguide

Issue:
False-positive 405 & 414 on loop unpacking
# Bug report

## What's wrong

```python
for kind, (model_class, list_model_class) in mapping.items():
    ...  # do things
```

...triggers WPS405 and WPS414.

405 says:

> Use regular ast.Name variables. Or tuple of ast.Name variables. Star names are also fine.

I'm using a name and a tuple with two names. This should be OK right?

414 says to avoid side-effects. There are no side-effects when unpacking into `kind, (model_class, list_model_class)`.

## How it should work

No 405 or 414 triggered.

## System information

### flake8 information

Contents of `flake8 --bug-report`:

```json
{
  "dependencies": [],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.6.12",
    "system": "Linux"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "black",
      "version": "0.2.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-bandit",
      "version": "2.1.2"
    },
    {
      "is_local": false,
      "plugin": "flake8-broken-line",
      "version": "0.2.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-bugbear",
      "version": "19.8.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-comprehensions",
      "version": "3.2.3"
    },
    {
      "is_local": false,
      "plugin": "flake8-darglint",
      "version": "1.5.5"
    },
    {
      "is_local": false,
      "plugin": "flake8-debugger",
      "version": "3.2.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-docstrings",
      "version": "1.5.0, pydocstyle: 5.1.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-eradicate",
      "version": "0.3.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-string-format",
      "version": "0.2.3"
    },
    {
      "is_local": false,
      "plugin": "flake8-tidy-imports",
      "version": "4.1.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-variables-names",
      "version": "0.0.3"
    },
    {
      "is_local": false,
      "plugin": "flake8_builtins",
      "version": "1.5.2"
    },
    {
      "is_local": false,
      "plugin": "flake8_commas",
      "version": "2.0.0"
    },
    {
      "is_local": false,
      "plugin": "flake8_isort",
      "version": "3.0.1"
    },
    {
      "is_local": false,
      "plugin": "flake8_quotes",
      "version": "2.1.2"
    },
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "naming",
      "version": "0.9.1"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.6.0"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "2.2.0"
    },
    {
      "is_local": false,
      "plugin": "pylint",
      "version": "2.6.0"
    },
    {
      "is_local": false,
      "plugin": "rst-docstrings",
      "version": "0.0.12"
    },
    {
      "is_local": false,
      "plugin": "wemake_python_styleguide",
      "version": "0.14.1"
    }
  ],
  "version": "3.8.4"
}
```

### pip information

Contents of `pip freeze`: sorry lots of private stuff, can sanitize and post if needed.


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=h1) Report
> Merging [#1850](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=desc) (b317102) into [master](https://codecov.io/gh/wemake-services/wemake-python-styleguide/commit/7ad2e202921c58309bcc146faa0d8c66265c7bb3?el=desc) (7ad2e20) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/graphs/tree.svg?width=650&height=150&src=pr&token=yMZCKw0D02)](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##            master     #1850   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          107       107           
  Lines         5991      5971   -20     
  Branches      1341      1339    -2     
=========================================
- Hits          5991      5971   -20     
```


| [Impacted Files](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ake\_python\_styleguide/violations/best\_practices.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL3Zpb2xhdGlvbnMvYmVzdF9wcmFjdGljZXMucHk=) | `100.00% <ø> (ø)` | |
| [wemake\_python\_styleguide/logic/tree/variables.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2xvZ2ljL3RyZWUvdmFyaWFibGVzLnB5) | `100.00% <100.00%> (ø)` | |
| [wemake\_python\_styleguide/visitors/ast/builtins.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL3Zpc2l0b3JzL2FzdC9idWlsdGlucy5weQ==) | `100.00% <100.00%> (ø)` | |
| [wemake\_python\_styleguide/compat/nodes.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2NvbXBhdC9ub2Rlcy5weQ==) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/logic/walrus.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2xvZ2ljL3dhbHJ1cy5weQ==) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/compat/routing.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2NvbXBhdC9yb3V0aW5nLnB5) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/logic/safe\_eval.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2xvZ2ljL3NhZmVfZXZhbC5weQ==) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/compat/packaging.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL2NvbXBhdC9wYWNrYWdpbmcucHk=) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/visitors/ast/functions.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL3Zpc2l0b3JzL2FzdC9mdW5jdGlvbnMucHk=) | `100.00% <0.00%> (ø)` | |
| [wemake\_python\_styleguide/visitors/ast/operators.py](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850/diff?src=pr&el=tree#diff-d2VtYWtlX3B5dGhvbl9zdHlsZWd1aWRlL3Zpc2l0b3JzL2FzdC9vcGVyYXRvcnMucHk=) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=footer). Last update [7ad2e20...b317102](https://codecov.io/gh/wemake-services/wemake-python-styleguide/pull/1850?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/wemake_python_styleguide/logic/tree/variables.py b/wemake_python_styleguide/logic/tree/variables.py
index 05dbadce..9f3c9154 100644
--- a/wemake_python_styleguide/logic/tree/variables.py
+++ b/wemake_python_styleguide/logic/tree/variables.py
@@ -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
+            for element in target.elts
+        )
+    return _is_valid_single(target)
+
+
 def _is_valid_single(node: _VarDefinition) -> bool:
     return (
         isinstance(node, ast.Name) or
diff --git a/wemake_python_styleguide/violations/best_practices.py b/wemake_python_styleguide/violations/best_practices.py
index 4ec9afb8..7b39a304 100644
--- a/wemake_python_styleguide/violations/best_practices.py
+++ b/wemake_python_styleguide/violations/best_practices.py
@@ -660,19 +660,14 @@ class WrongUnpackingViolation(ASTViolation):
     Example::
 
         # Correct:
-        first, second = some()
+        reader, writter = call()
+        self.reader = reader
+        self.writer = writer
 
         # Wrong:
         first, some_dict['alias'] = some()
-
-        # Wrong:
         self.reader, self.writer = call()
 
-        # Correct:
-        reader, writter = call()
-        self.reader = reader
-        self.writer = writer
-
     .. versionadded:: 0.6.0
     .. versionchanged:: 0.11.0
 
diff --git a/wemake_python_styleguide/visitors/ast/builtins.py b/wemake_python_styleguide/visitors/ast/builtins.py
index 4c9561c9..c17d318e 100644
--- a/wemake_python_styleguide/visitors/ast/builtins.py
+++ b/wemake_python_styleguide/visitors/ast/builtins.py
@@ -7,7 +7,6 @@ from typing import (
     ClassVar,
     DefaultDict,
     FrozenSet,
-    Iterable,
     List,
     Optional,
     Sequence,
@@ -25,12 +24,12 @@ from wemake_python_styleguide.compat.aliases import (
 )
 from wemake_python_styleguide.compat.functions import get_slice_expr
 from wemake_python_styleguide.logic import nodes, safe_eval, source, walk
-from wemake_python_styleguide.logic.naming.name_nodes import extract_name
 from wemake_python_styleguide.logic.tree import (
     attributes,
     functions,
     operators,
     strings,
+    variables,
 )
 from wemake_python_styleguide.types import (
     AnyChainable,
@@ -420,18 +419,15 @@ class WrongAssignmentVisitor(base.BaseNodeVisitor):
     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),
                 )


Test patch:
diff --git a/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_rules.py b/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_rules.py
index cb7dc876..1fdaffb7 100644
--- a/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_rules.py
+++ b/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_rules.py
@@ -89,7 +89,6 @@ wrong_single_destructuring3 = '[first] = {0}'
     tuple_assignment1,
     tuple_assignment2,
     spread_assignment1,
-    spread_assignment2,
     for_assignment,
     for_unpacking1,
     for_unpacking2,
@@ -101,6 +100,10 @@ wrong_single_destructuring3 = '[first] = {0}'
     with_unpacking1,
     with_unpacking2,
 ])
+@pytest.mark.parametrize('definition', [
+    'some_name',
+    '(a, b)',  # tuple
+])
 @pytest.mark.parametrize('target', [
     '(1, 2)',
     '[1, 2]',
@@ -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()
diff --git a/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_to_list.py b/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_to_list.py
index 4adab83b..7cd1272a 100644
--- a/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_to_list.py
+++ b/tests/test_visitors/test_ast/test_builtins/test_assign/test_unpacking_to_list.py
@@ -11,6 +11,8 @@ from wemake_python_styleguide.visitors.ast.builtins import (
     WrongAssignmentVisitor,
 )
 
+# Assignments:
+
 list_target = '[first, second]'
 nested_list_target0 = '([first, second], third)'
 nested_list_target1 = '(first, [second, third])'
@@ -27,6 +29,8 @@ regular_assignment = '{0} = some()'
 
 regular_multiple_assignment = 'result = {0} = some_other_result = some()'
 
+# Templates:
+
 for_assignment = """
 def wrapper():
     for {0} in iterable:
@@ -86,9 +90,10 @@ def test_unpacking_to_list(
     default_options,
     code,
     assignment,
+    mode,
 ):
     """Ensure that unpacking iterable to list is restricted."""
-    tree = parse_ast_tree(code.format(assignment))
+    tree = parse_ast_tree(mode(code.format(assignment)))
 
     visitor = WrongAssignmentVisitor(default_options, tree=tree)
     visitor.run()
@@ -129,7 +134,8 @@ def test_unpacking_to_nested_list(
 
     assert_errors(
         visitor,
-        [UnpackingIterableToListViolation, WrongUnpackingViolation],
+        [UnpackingIterableToListViolation],
+        ignored_types=(WrongUnpackingViolation,),
     )
 
 
@@ -157,7 +163,7 @@ def test_unpacking_to_list_in_middle_target(
     visitor = WrongAssignmentVisitor(default_options, tree=tree)
     visitor.run()
 
-    assert_errors(
-        visitor,
-        [MultipleAssignmentsViolation, UnpackingIterableToListViolation],
-    )
+    assert_errors(visitor, [
+        MultipleAssignmentsViolation,
+        UnpackingIterableToListViolation,
+    ])
diff --git a/tests/test_visitors/test_ast/test_loops/test_comprehensions/test_variable_definition_comprehensions.py b/tests/test_visitors/test_ast/test_loops/test_comprehensions/test_variable_definition_comprehensions.py
index 39b3ad35..8e98a717 100644
--- a/tests/test_visitors/test_ast/test_loops/test_comprehensions/test_variable_definition_comprehensions.py
+++ b/tests/test_visitors/test_ast/test_loops/test_comprehensions/test_variable_definition_comprehensions.py
@@ -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,