Fixed #1280: rewrite of as imports changes the behavior of the imports

CHANGELOG.md

@@ -7,6 +7,8 @@ NOTE: isort follows the [semver](https://semver.org/) versioning standard.
   - isort now throws an exception if an invalid settings path is given (issue #1174).
   - Fixed #1178: support for semicolons in decorators.
   - Fixed #1315: Extra newline before comment with -n + --fss.
+**Formatting changes implied:**
+  - Fixed #1280: rewrite of as imports changes the behavior of the imports.
 
 ### 5.0.9 July 11, 2020
 is

isort/hooks.py

@@ -8,7 +8,7 @@
 from pathlib import Path
 from typing import List
 
-from isort import Config, api
+from isort import Config, api, exceptions
 
 
 def get_output(command: List[str]) -> str:
@@ -61,9 +61,14 @@ def git_hook(strict: bool = False, modify: bool = False) -> int:
             staged_cmd = ["git", "show", f":{filename}"]
             staged_contents = get_output(staged_cmd)
 
-            if not api.check_code_string(staged_contents, file_path=Path(filename), config=config):
-                errors += 1
-                if modify:
-                    api.sort_file(filename, config=config)
+            try:
+                if not api.check_code_string(staged_contents,
+                                             file_path=Path(filename),
+                                             config=config):
+                    errors += 1
+                    if modify:
+                        api.sort_file(filename, config=config)
+            except exceptions.FileSkipComment:
+                pass
 
     return errors if strict else 0

isort/output.py

@@ -255,10 +255,10 @@ def _with_from_imports(
         sub_modules = [f"{module}.{from_import}" for from_import in from_imports]
         as_imports = {
             from_import: [
-                f"{from_import} as {as_module}" for as_module in parsed.as_map[sub_module]
+                f"{from_import} as {as_module}" for as_module in parsed.as_map["from"][sub_module]
             ]
             for from_import, sub_module in zip(from_imports, sub_modules)
-            if sub_module in parsed.as_map
+            if sub_module in parsed.as_map["from"]
         }
         if config.combine_as_imports and not ("*" in from_imports and config.combine_star):
             if not config.no_inline_sort:
@@ -487,11 +487,12 @@ def _with_straight_imports(
             continue
 
         import_definition = []
-        if module in parsed.as_map:
+        if module in parsed.as_map["straight"]:
             if config.keep_direct_and_as_imports and parsed.imports[section]["straight"][module]:
                 import_definition.append(f"{import_type} {module}")
             import_definition.extend(
-                f"{import_type} {module} as {as_import}" for as_import in parsed.as_map[module]
+                f"{import_type} {module} as {as_import}"
+                for as_import in parsed.as_map["straight"][module]
             )
         else:
             import_definition.append(f"{import_type} {module}")

isort/parse.py

@@ -131,7 +131,7 @@ class ParsedContent(NamedTuple):
     import_index: int
     place_imports: Dict[str, List[str]]
     import_placements: Dict[str, str]
-    as_map: Dict[str, List[str]]
+    as_map: Dict[str, Dict[str, List[str]]]
     imports: Dict[str, Dict[str, Any]]
     categorized_comments: "CommentsDict"
     change_count: int
@@ -155,7 +155,10 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
 
     place_imports: Dict[str, List[str]] = {}
     import_placements: Dict[str, str] = {}
-    as_map: Dict[str, List[str]] = defaultdict(list)
+    as_map: Dict[str, Dict[str, List[str]]] = {
+        "straight": defaultdict(list),
+        "from": defaultdict(list),
+    }
     imports: OrderedDict[str, Dict[str, Any]] = OrderedDict()
     for section in chain(config.sections, config.forced_separate):
         imports[section] = {"straight": OrderedDict(), "from": OrderedDict()}
@@ -318,17 +321,13 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
                     if type_of_import == "from":
                         module = just_imports[0] + "." + just_imports[as_index - 1]
                         as_name = just_imports[as_index + 1]
-                        if as_name not in as_map[module]:
-                            as_map[module].append(as_name)
+                        if as_name not in as_map["from"][module]:
+                            as_map["from"][module].append(as_name)
                     else:
                         module = just_imports[as_index - 1]
                         as_name = just_imports[as_index + 1]
-                        if as_name not in as_map[module]:
-                            as_map[module].append(as_name)
-                        if "." in module:
-                            type_of_import = "from"
-                            just_imports[:as_index] = module.rsplit(".", 1)
-                            as_index = just_imports.index("as")
+                        if as_name not in as_map["straight"][module]:
+                            as_map["straight"][module].append(as_name)
                     if not config.combine_as_imports:
                         categorized_comments["straight"][module] = comments
                         comments = []

tests/test_isort.py

@@ -1784,14 +1784,14 @@ def test_placement_control() -> None:
 
     assert test_output == (
         "import os\n"
+        "import p24.imports._argparse as argparse\n"
+        "import p24.imports._subprocess as subprocess\n"
         "import sys\n"
-        "from p24.imports import _VERSION as VERSION\n"
-        "from p24.imports import _argparse as argparse\n"
-        "from p24.imports import _subprocess as subprocess\n"
         "\n"
         "from bottle import Bottle, redirect, response, run\n"
         "\n"
-        "from p24.shared import media_wiki_syntax as syntax\n"
+        "import p24.imports._VERSION as VERSION\n"
+        "import p24.shared.media_wiki_syntax as syntax\n"
     )
 
 
@@ -1836,10 +1836,9 @@ def test_custom_sections() -> None:
     assert test_output == (
         "# Standard Library\n"
         "import os\n"
+        "import p24.imports._argparse as argparse\n"
+        "import p24.imports._subprocess as subprocess\n"
         "import sys\n"
-        "from p24.imports import _VERSION as VERSION\n"
-        "from p24.imports import _argparse as argparse\n"
-        "from p24.imports import _subprocess as subprocess\n"
         "\n"
         "# Django\n"
         "from django.conf import settings\n"
@@ -1853,7 +1852,8 @@ def test_custom_sections() -> None:
         "import pandas as pd\n"
         "\n"
         "# First Party\n"
-        "from p24.shared import media_wiki_syntax as syntax\n"
+        "import p24.imports._VERSION as VERSION\n"
+        "import p24.shared.media_wiki_syntax as syntax\n"
     )
 
 
@@ -4061,9 +4061,10 @@ def test_isort_ensures_blank_line_between_import_and_comment() -> None:
         "import one.b\n"
         "\n"
         "# noinspection PyUnresolvedReferences\n"
+        "import two.a as aa\n"
+        "\n"
         "# noinspection PyUnresolvedReferences\n"
-        "from two import a as aa\n"
-        "from two import b as bb\n"
+        "import two.b as bb\n"
         "\n"
         "# noinspection PyUnresolvedReferences\n"
         "from three.a import a\n"
@@ -4810,8 +4811,8 @@ def test_as_imports_mixed():
     test_input = """from datetime import datetime
 import datetime.datetime as dt
 """
-    expected_output = """from datetime import datetime
-from datetime import datetime as dt
+    expected_output = """import datetime.datetime as dt
+from datetime import datetime
 """
     assert isort.code(test_input, keep_direct_and_as_imports=True) == expected_output
 

tests/test_regressions.py

@@ -323,3 +323,20 @@ def test_isort_shouldnt_add_extra_new_line_when_fass_and_n_issue_1315():
 from .. import foo
 """
     )
+
+
+def test_isort_doesnt_rewrite_import_with_dot_to_from_import_issue_1280():
+    """Test to ensure isort doesn't rewrite imports in the from of import y.x into from y import x.
+    This is because they are not technically fully equivalent to eachother and can introduce broken
+    behaviour.
+    See: https://github.com/timothycrosley/isort/issues/1280
+    """
+    assert isort.check_code(
+        """
+        import test.module
+        import test.module as m
+        from test import module
+        from test import module as m
+    """,
+        show_diff=True,
+    )