Forbid importing the same object under different aliases (#2769)

Closes #2768

CHANGELOG.md

@@ -23,6 +23,8 @@ Semantic versioning in our case means:
 
 - `wemake` output formatter now respects `NO_COLOR=1` option
   to disable text highlighting. See https://no-color.org
+- Add `ImportObjectCollisionViolation` to detect
+  the same objects imported under different aliases
 
 ### Bugfixes
 

tests/fixtures/noqa/noqa.py

@@ -16,6 +16,7 @@
 
 from foo import bar
 from foo.bar import baz  # noqa: WPS458
+from package import module, module as alias  # noqa: WPS474
 
 from .version import get_version  # noqa: WPS300
 

tests/test_checker/test_noqa.py

@@ -265,6 +265,8 @@
     'WPS470': 1,
     'WPS471': 1,
     'WPS472': 1,
+    'WPS473': 0,
+    'WPS474': 1,
 
     'WPS500': 1,
     'WPS501': 1,
@@ -317,7 +319,6 @@
     'WPS614': 1,
     'WPS615': 2,
     'WPS616': 1,
-    'WPS473': 0,
 })
 
 # Violations which may be tweaked by `i_control_code` option:

tests/test_visitors/test_ast/test_imports/test_import_object_collision.py

@@ -0,0 +1,115 @@
+import pytest
+
+from wemake_python_styleguide.violations.best_practices import (
+    ImportObjectCollisionViolation,
+)
+from wemake_python_styleguide.violations.consistency import (
+    LocalFolderImportViolation,
+)
+from wemake_python_styleguide.visitors.ast.imports import WrongImportVisitor
+
+# Correct:
+
+correct_single_import_from = 'from utils import public'
+correct_no_colliding_imports = """
+from other import public
+from other import correct
+from third import name1, name2
+"""
+
+correct_aliases = """
+from other import public as alias1
+from my import public as alias2
+from third import public
+from four.alias1 import public as alias3, other as alias4
+"""
+
+correct_same_module_imports = """
+from my import name1 as alias1
+from my import name2 as alias2
+"""
+
+correct_dot_imports = """
+from . import sub
+from .. import sub as alias1
+from ... import sub as alias2
+"""
+
+correct_relative_imports = """
+from .sub import name
+from ..sub import name as alias1
+from ...sub import name as alias2
+"""
+
+# Wrong:
+
+colliding_object_import1 = """
+from module import name, name as alias
+"""
+
+colliding_object_import2 = """
+from module import name
+from module import name as alias
+"""
+
+colliding_nested_object_import = """
+from module.name import sub
+from module.name import sub as alias
+"""
+
+colliding_dot_import1 = """
+from . import sub
+from . import sub as alias
+"""
+
+colliding_dot_import2 = """
+from ...package import sub
+from ...package import sub as alias
+"""
+
+
+@pytest.mark.parametrize('code', [
+    correct_single_import_from,
+    correct_no_colliding_imports,
+    correct_aliases,
+    correct_same_module_imports,
+    correct_dot_imports,
+    correct_relative_imports,
+])
+def test_correct_imports(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    default_options,
+):
+    """Testing that no colliding imports are allowed."""
+    tree = parse_ast_tree(code)
+
+    visitor = WrongImportVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [], ignored_types=(LocalFolderImportViolation,))
+
+
+@pytest.mark.parametrize('code', [
+    colliding_object_import1,
+    colliding_object_import2,
+    colliding_nested_object_import,
+])
+def test_imports_collision(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    default_options,
+):
+    """Testing that colliding imports are restricted."""
+    tree = parse_ast_tree(code)
+
+    visitor = WrongImportVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(
+        visitor,
+        [ImportObjectCollisionViolation],
+        ignored_types=(LocalFolderImportViolation,),
+    )

wemake_python_styleguide/logic/tree/imports.py

@@ -16,6 +16,20 @@ class ImportedObjectInfo(NamedTuple):
     node: AnyImport
 
 
+def get_module_name(node: ast.ImportFrom) -> str:
+    """
+    Returns module name for any ``ImportFrom``.
+
+    Handles all corner cases, including:
+    - `from . import a` -> `.`
+    - `from ..sub import b` -> `..sub`
+    """
+    return '{0}{1}'.format(
+        '.' * node.level,
+        node.module or '',
+    )
+
+
 def get_import_parts(node: AnyImport) -> List[str]:
     """Returns list of import modules."""
     module_path = getattr(node, 'module', '') or ''

wemake_python_styleguide/violations/best_practices.py

@@ -90,6 +90,7 @@
    ConsecutiveSlicesViolation
    GettingElementByUnpackingViolation
    WrongEmptyLinesCountViolation
+   ImportObjectCollisionViolation
 
 Best practices
 --------------
@@ -168,6 +169,7 @@
 .. autoclass:: ConsecutiveSlicesViolation
 .. autoclass:: GettingElementByUnpackingViolation
 .. autoclass:: WrongEmptyLinesCountViolation
+.. autoclass:: ImportObjectCollisionViolation
 
 """
 
@@ -2819,3 +2821,31 @@ def func(name):
 
     error_template = 'Found too many empty lines in `def`: {0}'
     code = 473
+
+
+@final
+class ImportObjectCollisionViolation(ASTViolation):
+    """
+    Do not allow importing the same object under different aliases.
+
+    Reasoning:
+        This can lead to reader confusion,
+        because two names usually mean two different things.
+
+    Solution:
+        Remove useless aliases.
+
+    Example::
+
+        # Correct:
+        from module import name
+
+        # Wrong:
+        from module import name, name as alias
+
+    .. versionadded:: 0.19.0
+
+    """
+
+    error_template = 'Found import object collision: {0}'
+    code = 474

wemake_python_styleguide/visitors/ast/imports.py

@@ -1,6 +1,7 @@
 import ast
+from collections import defaultdict
 from itertools import chain, product
-from typing import Iterable, List
+from typing import DefaultDict, Iterable, List, Set
 
 from typing_extensions import Final, final
 
@@ -13,6 +14,7 @@
 from wemake_python_styleguide.violations.best_practices import (
     FutureImportViolation,
     ImportCollisionViolation,
+    ImportObjectCollisionViolation,
     NestedImportViolation,
     ProtectedModuleMemberViolation,
     ProtectedModuleViolation,
@@ -145,6 +147,9 @@ class _ImportCollisionValidator(object):
     def __init__(self, error_callback: ErrorCallback) -> None:
         self._error_callback = error_callback
         self._imported_names: List[imports.ImportedObjectInfo] = []
+        # This helps us to detect cases like:
+        # `from x import y, y as z`
+        self._imported_objects: DefaultDict[str, Set[str]] = defaultdict(set)
 
     def validate(self) -> None:
         """Validates that there are no intersecting imported modules."""
@@ -173,6 +178,13 @@ def add_import(self, node: ast.Import) -> None:
     def add_import_from(self, node: ast.ImportFrom) -> None:
         """Extract info needed for validation from ``ast.ImportFrom``."""
         for alias in node.names:
+            identifier = imports.get_module_name(node)
+            if alias.name in self._imported_objects[identifier]:
+                self._error_callback(
+                    ImportObjectCollisionViolation(node, alias.name),
+                )
+            self._imported_objects[identifier].add(alias.name)
+
             if not alias.asname:
                 self._imported_names.append(imports.ImportedObjectInfo(
                     _MODULE_MEMBERS_SEPARATOR.join(