Fixed #1548: On rare occasions an unecessary empty line can be added when an import is marked as skipped.issue/1548/empty-line-added-after-import-skip

4 files changed, 106 insertions, 6 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 02bb467f..f861830c 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 3131c201..292bdc1c 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 9a80e97b..96468f09 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 9f6f92de..87b533d6 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
+"""
+    )