Fixed #1741

CHANGELOG.md

@@ -5,9 +5,10 @@ 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.9.0 TBD
-  - Fixed (https://github.com/PyCQA/isort/pull/1695) added imports being added to doc string in some cases.
-  - Fixed (https://github.com/PyCQA/isort/pull/1714) in rare cases line continuation combined with tabs can output invalid code.
-  - Fixed (https://github.com/PyCQA/isort/pull/1726) isort ignores reverse_sort when force_sort_within_sections is true.
+  - Fixed (https://github.com/PyCQA/isort/pull/1695): added imports being added to doc string in some cases.
+  - Fixed (https://github.com/PyCQA/isort/pull/1714): in rare cases line continuation combined with tabs can output invalid code.
+  - Fixed (https://github.com/PyCQA/isort/pull/1726): isort ignores reverse_sort when force_sort_within_sections is true.
+  - Fixed (https://github.com/PyCQA/isort/issues/1741): comments in hanging indent modes can lead to invalid code.
   - Implemented #1697: Provisional support for PEP 582: skip `__pypackages__` directories by default.
   - Implemented #1705: More intuitive handling of isort:skip_file comments on streams.
 

isort/wrap_modes.py

@@ -106,64 +106,67 @@ def vertical(**interface):
     return f"{interface['statement']}({first_import}{_imports}{_comma_maybe})"
 
 
-def _hanging_indent_common(use_parentheses=False, **interface):
+def _hanging_indent_end_line(line: str) -> str:
+    if not line.endswith(" "):
+        line += " "
+    return line + "\\"
+
+
+@_wrap_mode
+def hanging_indent(**interface):
     if not interface["imports"]:
         return ""
-    line_length_limit = interface["line_length"] - (1 if use_parentheses else 3)
 
-    def end_line(line):
-        if use_parentheses:
-            return line
-        if not line.endswith(" "):
-            line += " "
-        return line + "\\"
+    line_length_limit = interface["line_length"] - 3
 
-    if use_parentheses:
-        interface["statement"] += "("
     next_import = interface["imports"].pop(0)
     next_statement = interface["statement"] + next_import
     # Check for first import
     if len(next_statement) > line_length_limit:
         next_statement = (
-            isort.comments.add_to_line(
-                interface["comments"],
-                end_line(interface["statement"]),
-                removed=interface["remove_comments"],
-                comment_prefix=interface["comment_prefix"],
-            )
-            + f"{interface['line_separator']}{interface['indent']}{next_import}"
+            _hanging_indent_end_line(interface["statement"])
+            + interface["line_separator"]
+            + interface["indent"]
+            + next_import
         )
-        interface["comments"] = []
+
     interface["statement"] = next_statement
     while interface["imports"]:
         next_import = interface["imports"].pop(0)
-        next_statement = isort.comments.add_to_line(
+        next_statement = interface["statement"] + ", " + next_import
+        if len(next_statement.split(interface["line_separator"])[-1]) > line_length_limit:
+            next_statement = (
+                _hanging_indent_end_line(interface["statement"] + ",")
+                + f"{interface['line_separator']}{interface['indent']}{next_import}"
+            )
+        interface["statement"] = next_statement
+
+    interface[
+        "statement"
+    ] = f"{interface['statement']}{',' if interface['include_trailing_comma'] else ''}"
+    if interface["comments"]:
+        statement_with_comments = isort.comments.add_to_line(
             interface["comments"],
-            interface["statement"] + ", " + next_import,
+            interface["statement"],
             removed=interface["remove_comments"],
             comment_prefix=interface["comment_prefix"],
         )
-        current_line = next_statement.split(interface["line_separator"])[-1]
-        if len(current_line) > line_length_limit:
-            next_statement = (
-                isort.comments.add_to_line(
+        if len(statement_with_comments.split(interface["line_separator"])[-1]) <= (
+            line_length_limit + 2
+        ):
+            return statement_with_comments
+        else:
+            return (
+                _hanging_indent_end_line(interface["statement"])
+                + interface["line_separator"]
+                + isort.comments.add_to_line(
                     interface["comments"],
-                    end_line(interface["statement"] + ","),
+                    interface["indent"],
                     removed=interface["remove_comments"],
-                    comment_prefix=interface["comment_prefix"],
+                    comment_prefix=interface["comment_prefix"].lstrip(),
                 )
-                + f"{interface['line_separator']}{interface['indent']}{next_import}"
             )
-            interface["comments"] = []
-        interface["statement"] = next_statement
-    _comma_maybe = "," if interface["include_trailing_comma"] else ""
-    _close_parentheses_maybe = ")" if use_parentheses else ""
-    return interface["statement"] + _comma_maybe + _close_parentheses_maybe
-
-
-@_wrap_mode
-def hanging_indent(**interface):
-    return _hanging_indent_common(use_parentheses=False, **interface)
+    return interface["statement"]
 
 
 @_wrap_mode
@@ -309,13 +312,64 @@ def vertical_prefix_from_module_import(**interface):
 
 @_wrap_mode
 def hanging_indent_with_parentheses(**interface):
-    return _hanging_indent_common(use_parentheses=True, **interface)
+    if not interface["imports"]:
+        return ""
+
+    line_length_limit = interface["line_length"] - 1
+
+    interface["statement"] += "("
+    next_import = interface["imports"].pop(0)
+    next_statement = interface["statement"] + next_import
+    # Check for first import
+    if len(next_statement) > line_length_limit:
+        next_statement = (
+            isort.comments.add_to_line(
+                interface["comments"],
+                interface["statement"],
+                removed=interface["remove_comments"],
+                comment_prefix=interface["comment_prefix"],
+            )
+            + f"{interface['line_separator']}{interface['indent']}{next_import}"
+        )
+        interface["comments"] = []
+    interface["statement"] = next_statement
+    while interface["imports"]:
+        next_import = interface["imports"].pop(0)
+        if (
+            not interface["line_separator"] in interface["statement"]
+            and "#" in interface["statement"]
+        ):
+            line, comments = interface["statement"].split("#", 1)
+            next_statement = (
+                f"{line.rstrip()}, {next_import}{interface['comment_prefix']}{comments}"
+            )
+        else:
+            next_statement = isort.comments.add_to_line(
+                interface["comments"],
+                interface["statement"] + ", " + next_import,
+                removed=interface["remove_comments"],
+                comment_prefix=interface["comment_prefix"],
+            )
+        current_line = next_statement.split(interface["line_separator"])[-1]
+        if len(current_line) > line_length_limit:
+            next_statement = (
+                isort.comments.add_to_line(
+                    interface["comments"],
+                    interface["statement"] + ",",
+                    removed=interface["remove_comments"],
+                    comment_prefix=interface["comment_prefix"],
+                )
+                + f"{interface['line_separator']}{interface['indent']}{next_import}"
+            )
+            interface["comments"] = []
+        interface["statement"] = next_statement
+    return f"{interface['statement']}{',' if interface['include_trailing_comma'] else ''})"
 
 
 @_wrap_mode
 def backslash_grid(**interface):
     interface["indent"] = interface["white_space"][:-1]
-    return _hanging_indent_common(use_parentheses=False, **interface)
+    return hanging_indent(**interface)
 
 
 WrapModes = enum.Enum(  # type: ignore

tests/unit/test_isort.py

@@ -338,11 +338,12 @@ def test_output_modes() -> None:
         indent="    ",
     )
     assert comment_output_hanging_indent == (
-        "from third_party import lib1, \\  # comment\n"
-        "    lib2, lib3, lib4, lib5, lib6, \\\n"
-        "    lib7, lib8, lib9, lib10, lib11, \\\n"
-        "    lib12, lib13, lib14, lib15, lib16, \\\n"
-        "    lib17, lib18, lib20, lib21, lib22\n"
+        "from third_party import lib1, lib2, \\\n"
+        "    lib3, lib4, lib5, lib6, lib7, \\\n"
+        "    lib8, lib9, lib10, lib11, lib12, \\\n"
+        "    lib13, lib14, lib15, lib16, lib17, \\\n"
+        "    lib18, lib20, lib21, lib22 \\\n"
+        "    # comment\n"
     )
 
     test_output_vertical_indent = isort.code(

tests/unit/test_regressions.py

@@ -2,6 +2,7 @@
 from io import StringIO
 
 import isort
+import pytest
 
 
 def test_isort_duplicating_comments_issue_1264():
@@ -1651,3 +1652,73 @@ def test_isort_treats_src_paths_same_as_from_config_as_cli_issue_1711(tmpdir):
         show_diff=True,
         config=config,
     )
+
+
+def test_isort_should_never_quietly_remove_imports_in_hanging_line_mode_issue_1741():
+    assert (
+        isort.code(
+            """
+from src import abcd, qwerty, efg, xyz  # some comment
+""",
+            line_length=50,
+            multi_line_output=2,
+        )
+        == """
+from src import abcd, efg, qwerty, xyz \\
+    # some comment
+"""
+    )
+    assert (
+        isort.code(
+            """
+from src import abcd, qwerty, efg, xyz  # some comment
+""",
+            line_length=54,
+            multi_line_output=2,
+        )
+        == """
+from src import abcd, efg, qwerty, xyz  # some comment
+"""
+    )
+    assert (
+        isort.code(
+            """
+from src import abcd, qwerty, efg, xyz  # some comment
+""",
+            line_length=53,
+            multi_line_output=2,
+        )
+        == """
+from src import abcd, efg, qwerty, xyz \\
+    # some comment
+"""
+    )
+    assert (
+        isort.code(
+            """
+from src import abcd, qwerty, efg, xyz  # some comment
+""",
+            line_length=30,
+            multi_line_output=2,
+        )
+        == """
+from src import abcd, efg, \\
+    qwerty, xyz \\
+    # some comment
+"""
+    )
+
+
+@pytest.mark.parametrize("multi_line_output", range(12))
+def test_isort_should_never_quietly_remove_imports_in_any_hangin_mode_issue_1741(multi_line_output: int):
+    sorted_code = isort.code(
+            """
+from src import abcd, qwerty, efg, xyz  # some comment
+""",
+            line_length=30,
+            multi_line_output=multi_line_output,
+        )
+    assert "abcd" in sorted_code
+    assert "qwerty" in sorted_code
+    assert "efg" in sorted_code
+    assert "xyz" in sorted_code