From 245471d3dec8453d27a3c3c4895526a9cd49c647 Mon Sep 17 00:00:00 2001 From: Timothy Crosley Date: Thu, 8 Oct 2020 23:58:41 -0700 Subject: [PATCH] Fixed #1548: On rare occasions an unecessary empty line can be added when an import is marked as skipped. --- CHANGELOG.md | 3 ++ isort/core.py | 3 +- isort/parse.py | 20 ++++++-- tests/unit/test_regressions.py | 86 ++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02bb467f9..f861830c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Changelog NOTE: isort follows the [semver](https://semver.org/) versioning standard. Find out more about isort's release policy [here](https://pycqa.github.io/isort/docs/major_releases/release_policy/). +### 5.6.2 TBD + - Fixed #1548: On rare occasions an unecessary empty line can be added when an import is marked as skipped. + ### 5.6.1 [Hotfix] October 8, 2020 - Fixed #1546: Unstable (non-idempotent) behavior with certain src trees. diff --git a/isort/core.py b/isort/core.py index 3131c2011..292bdc1c2 100644 --- a/isort/core.py +++ b/isort/core.py @@ -85,7 +85,8 @@ def process( isort_off = True if current: if add_imports: - current += line_separator + line_separator.join(add_imports) + add_line_separator = line_separator or "\n" + current += add_line_separator + add_line_separator.join(add_imports) add_imports = [] parsed = parse.file_contents(current, config=config) verbose_output += parsed.verbose_output diff --git a/isort/parse.py b/isort/parse.py index 9a80e97b9..96468f095 100644 --- a/isort/parse.py +++ b/isort/parse.py @@ -213,12 +213,22 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte and not lstripped_line.startswith("#") and not lstripped_line.startswith("'''") and not lstripped_line.startswith('"""') - and not lstripped_line.startswith("import") - and not lstripped_line.startswith("from") ): - import_index = index - 1 - while import_index and not in_lines[import_index - 1]: - import_index -= 1 + if not lstripped_line.startswith("import") and not lstripped_line.startswith("from"): + import_index = index - 1 + while import_index and not in_lines[import_index - 1]: + import_index -= 1 + elif "isort:skip" in line or "isort: skip" in line: + commentless = line.split("#", 1)[0] + if ( + "(" in commentless + and not commentless.rstrip().endswith(")") + and import_index < line_count + ): + import_index = index + while import_index < line_count and not commentless.rstrip().endswith(")"): + commentless = in_lines[import_index].split("#", 1)[0] + import_index += 1 line, *end_of_line_comment = line.split("#", 1) if ";" in line: diff --git a/tests/unit/test_regressions.py b/tests/unit/test_regressions.py index 9f6f92de3..87b533d63 100644 --- a/tests/unit/test_regressions.py +++ b/tests/unit/test_regressions.py @@ -1336,3 +1336,89 @@ def test_isort_shouldnt_introduce_syntax_error_issue_1539(): import b ''' ) + + +def test_isort_shouldnt_split_skip_issue_1548(): + """Ensure isort doesn't add a spurious new line if isort: skip is combined with float to top. + See: https://github.com/PyCQA/isort/issues/1548. + """ + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip + prune_dependencies, +) +""", + show_diff=True, + profile="black", + float_to_top=True, + ) + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip + prune_dependencies, +) +import a +import b +""", + show_diff=True, + profile="black", + float_to_top=True, + ) + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import # isort:skip +import a +import b +""", + show_diff=True, + float_to_top=True, + ) + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip + a +) +import b +""", + show_diff=True, + profile="black", + float_to_top=True, + ) + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip + ) +""", + show_diff=True, + profile="black", + float_to_top=True, + ) + assert isort.check_code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip +)""", + show_diff=True, + profile="black", + float_to_top=True, + ) + assert ( + isort.code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip +) +""", + profile="black", + float_to_top=True, + add_imports=["import os"], + ) + == """from tools.dependency_pruning.prune_dependencies import ( # isort:skip +) +import os +""" + ) + assert ( + isort.code( + """from tools.dependency_pruning.prune_dependencies import ( # isort:skip +)""", + profile="black", + float_to_top=True, + add_imports=["import os"], + ) + == """from tools.dependency_pruning.prune_dependencies import ( # isort:skip +) +import os +""" + )