Merge pull request #1551 from PyCQA/issue/1548/empty-line-added-after…

…-import-skip

Fixed #1548: On rare occasions an unecessary empty line can be added …

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.
 

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

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:

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
+"""
+    )