Preserve the magic trailing comma in --sort-reexports

isort/literal.py

@@ -1,4 +1,6 @@
 import ast
+import io
+import tokenize
 from collections.abc import Callable
 from typing import Any
 
@@ -27,6 +29,35 @@ def assignments(code: str) -> str:
     )
 
 
+def _has_magic_trailing_comma(literal: str) -> bool:
+    """Whether ``literal`` closes with a trailing comma, in black's "magic trailing
+    comma" sense: the author put it there to ask for one element per line.
+
+    Tokenizing rather than scanning the text keeps commas and brackets inside string
+    elements, and trailing comments, from being mistaken for the real closing bracket.
+    """
+    skip = frozenset(
+        {
+            tokenize.COMMENT,
+            tokenize.NL,
+            tokenize.NEWLINE,
+            tokenize.INDENT,
+            tokenize.DEDENT,
+            tokenize.ENDMARKER,
+        }
+    )
+    try:
+        tokens = [
+            token
+            for token in tokenize.generate_tokens(io.StringIO(literal).readline)
+            if token.type not in skip and token.string.strip()
+        ]
+    except (tokenize.TokenError, SyntaxError):  # pragma: no cover
+        return False
+
+    return len(tokens) >= 2 and tokens[-1].string in ")]}" and tokens[-2].string == ","
+
+
 def assignment(code: str, sort_type: str, extension: str, config: Config = DEFAULT_CONFIG) -> str:
     """Sorts the literal present within the provided code against the provided sort type,
     returning the sorted representation of the source code.
@@ -51,6 +82,13 @@ def assignment(code: str, sort_type: str, extension: str, config: Config = DEFAU
     if type(value) is not expected_type:
         raise LiteralSortTypeMismatch(type(value), expected_type)
 
+    # A one-element tuple's trailing comma is syntax, not a formatting request, so it
+    # must not be read as a magic trailing comma.
+    if _has_magic_trailing_comma(literal) and not (isinstance(value, tuple) and len(value) == 1):
+        # Zero line length forces the vertical-hanging-indent branch of
+        # _format_collection, preserving the requested one-element-per-line layout.
+        config = Config(config=config, line_length=0, include_trailing_comma=True)
+
     prefix_length = len(f"{variable_name} = ")
     sorted_value_code = f"{variable_name} = {sort_function(value, config, prefix_length)}"
     if config.formatting_function:

tests/unit/test_isort.py

@@ -5591,7 +5591,10 @@ def test_reexport_multiline() -> None:
     'bar',
 )
 """
-    expd_output = """__all__ = ("bar", "foo")
+    expd_output = """__all__ = (
+    "bar",
+    "foo",
+)
 """
     assert isort.code(test_input, config=Config(sort_reexports=True)) == expd_output
 
@@ -5659,7 +5662,7 @@ def test_reexport_multiline_import() -> None:
 )
 __all__ = [
     "foo",
-    "bar",
+    "bar"
 ]
 """
     expd_output = """from m import bar, foo
@@ -5676,7 +5679,7 @@ def test_reexport_multiline_in_center() -> None:
 )
 __all__ = [
     "foo",
-    "bar",
+    "bar"
 ]
 
 test
@@ -5693,7 +5696,7 @@ def test_reexport_multiline_in_center() -> None:
 def test_reexport_multiline_long_rollback() -> None:
     test_input = """from m import foo, bar
 __all__ = [                            "foo",
-    "bar",
+    "bar"
 ]
 
 test
@@ -5809,3 +5812,65 @@ def test_builtin_modules() -> None:
         "from python_none_objects import NoneIterable\n"
     )
     assert isort.code(test_input) == test_output
+
+
+def test_reexport_preserves_magic_trailing_comma() -> None:
+    """https://github.com/PyCQA/isort/issues/2578
+
+    A trailing comma in the original ``__all__`` is black's "magic trailing comma":
+    it asks for one element per line, even when the sorted result would fit on one.
+    """
+    test_input = """__all__ = (
+    "foo",
+    "bar",
+)
+"""
+    expd_output = """__all__ = (
+    "bar",
+    "foo",
+)
+"""
+    assert isort.code(test_input, config=Config(sort_reexports=True)) == expd_output
+
+
+def test_reexport_without_magic_trailing_comma_stays_collapsed() -> None:
+    """https://github.com/PyCQA/isort/issues/2578
+
+    Without the trailing comma the short form is kept, as before.
+    """
+    test_input = """__all__ = ("foo", "bar")
+"""
+    expd_output = """__all__ = ("bar", "foo")
+"""
+    assert isort.code(test_input, config=Config(sort_reexports=True)) == expd_output
+
+
+def test_reexport_single_element_tuple_comma_is_not_magic() -> None:
+    """https://github.com/PyCQA/isort/issues/2578
+
+    A one-element tuple's trailing comma is required syntax rather than a
+    formatting request, so it must not explode the literal.
+    """
+    test_input = """__all__ = ("foo",)
+"""
+    assert isort.code(test_input, config=Config(sort_reexports=True)) == test_input
+
+
+def test_reexport_magic_trailing_comma_ignores_strings_and_comments() -> None:
+    """https://github.com/PyCQA/isort/issues/2578
+
+    Detection tokenizes rather than scanning text, so a closing bracket or comma
+    inside a string element, or a trailing comment, is not mistaken for the end of
+    the literal.
+    """
+    test_input = """__all__ = (
+    "b#)",
+    "a,",  # trailing comment
+)
+"""
+    expd_output = """__all__ = (
+    "a,",
+    "b#)",
+)
+"""
+    assert isort.code(test_input, config=Config(sort_reexports=True)) == expd_output