Merge pull request #2184 from bp72/issue/2154

Apply the bracket fix from issue 471 only for use_parentheses=True
PyCQA · Dec 13, 2023 · c36e43c · c36e43c
2 parents e38702f + ee8d87f
commit c36e43c
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 9 deletions.
diff --git a/isort/parse.py b/isort/parse.py
@@ -519,7 +519,11 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
                 if comments and attach_comments_to is not None:
                     attach_comments_to.extend(comments)
 
-                if "," in import_string.split(just_imports[-1])[-1]:
+                if (
+                    just_imports
+                    and just_imports[-1]
+                    and "," in import_string.split(just_imports[-1])[-1]
+                ):
                     trailing_commas.add(import_from)
             else:
                 if comments and attach_comments_to is not None:

diff --git a/isort/wrap.py b/isort/wrap.py
@@ -122,20 +122,21 @@ def line(content: str, line_separator: str, config: Config = DEFAULT_CONFIG) ->
                             _separator = line_separator
                         else:
                             _separator = ""
-                        _comment = ""
+                        noqa_comment = ""
                         if comment and "noqa" in comment:
-                            _comment = f"{config.comment_prefix}{comment}"
+                            noqa_comment = f"{config.comment_prefix}{comment}"
                             cont_line = cont_line.rstrip()
                             _comma = "," if config.include_trailing_comma else ""
                         output = (
-                            f"{content}{splitter}({_comment}"
+                            f"{content}{splitter}({noqa_comment}"
                             f"{line_separator}{cont_line}{_comma}{_separator})"
                         )
-                    lines = output.split(line_separator)
-                    if config.comment_prefix in lines[-1] and lines[-1].endswith(")"):
-                        content, comment = lines[-1].split(config.comment_prefix, 1)
-                        lines[-1] = content + ")" + config.comment_prefix + comment[:-1]
-                    return line_separator.join(lines)
+                        lines = output.split(line_separator)
+                        if config.comment_prefix in lines[-1] and lines[-1].endswith(")"):
+                            content, comment = lines[-1].split(config.comment_prefix, 1)
+                            lines[-1] = content + ")" + config.comment_prefix + comment[:-1]
+                        output = line_separator.join(lines)
+                    return output
                 return f"{content}{splitter}\\{line_separator}{cont_line}"
     elif len(content) > config.line_length and wrap_mode == Modes.NOQA and "# NOQA" not in content:  # type: ignore
         return f"{content}{config.comment_prefix} NOQA"

diff --git a/tests/unit/test_wrap.py b/tests/unit/test_wrap.py
@@ -1,5 +1,8 @@
+import pytest
+
 from isort import wrap
 from isort.settings import Config
+from isort.wrap_modes import WrapModes
 
 
 def test_import_statement():
@@ -16,3 +19,32 @@ def test_import_statement():
     assert wrap.import_statement("from x import ", ["y", "z"], [], explode=True) == (
         "from x import (\n    y,\n    z,\n)"
     )
+
+
+@pytest.mark.parametrize(
+    "multi_line_output, expected",
+    (
+        (
+            WrapModes.VERTICAL_HANGING_INDENT,  # type: ignore
+            """from a import (
+    b as c  # comment that is long enough that this import doesn't fit in one line (parens)
+)""",
+        ),
+        (
+            WrapModes.VERTICAL,  # type: ignore
+            """from a import (
+    b as c)  # comment that is long enough that this import doesn't fit in one line (parens)""",
+        ),
+    ),
+)
+def test_line__comment_with_brackets__expects_unchanged_comment(multi_line_output, expected):
+    content = (
+        "from a import b as c  "
+        "# comment that is long enough that this import doesn't fit in one line (parens)"
+    )
+    config = Config(
+        multi_line_output=multi_line_output,
+        use_parentheses=True,
+    )
+
+    assert wrap.line(content=content, line_separator="\n", config=config) == expected