From 95dab39e346e0c1665ba9cd716f96de047a5cec6 Mon Sep 17 00:00:00 2001 From: Timothy Crosley Date: Mon, 13 Jul 2020 23:32:42 -0700 Subject: [PATCH] Fixed #1280: rewrite of as imports changes the behavior of the imports --- CHANGELOG.md | 2 ++ isort/hooks.py | 15 ++++++++++----- isort/output.py | 9 +++++---- isort/parse.py | 19 +++++++++---------- tests/test_isort.py | 25 +++++++++++++------------ tests/test_regressions.py | 17 +++++++++++++++++ 6 files changed, 56 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bd87643e..ced2475a3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/isort/hooks.py b/isort/hooks.py index 13b0b7434..2f3decd4c 100644 --- a/isort/hooks.py +++ b/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 diff --git a/isort/output.py b/isort/output.py index 2ca4c60d7..b3cc785ce 100644 --- a/isort/output.py +++ b/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}") diff --git a/isort/parse.py b/isort/parse.py index 125ba7fc1..7f112fc14 100644 --- a/isort/parse.py +++ b/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 = [] diff --git a/tests/test_isort.py b/tests/test_isort.py index c13e37a65..e39b7ed86 100644 --- a/tests/test_isort.py +++ b/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 diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 77521b089..d1f3a8567 100644 --- a/tests/test_regressions.py +++ b/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, + )