Stop swallowing a statement that immediately follows a sorted literal

[VXNCXNX]

Fixes #2286.

What's broken

A statement that immediately follows a sorted literal, with no blank line, makes isort raise.

>>> isort.code('__all__ = ["b", "a"]\nx = 1\n', sort_reexports=True)
ValueError: too many values to unpack (expected 2)

>>> isort.code('# isort: list\n__all__ = ["b", "a"]\nx = 1\n')
ValueError: too many values to unpack (expected 2)

The second needs no --sort-reexports, so this is not specific to reexports. The issue reports it as a reexport bug, but the same accumulator serves the action comments.

The fix

The elif code_sorting: branch in core.py accumulates lines and only stops at a blank line, so a trailing statement gets folded into the section. literal.assignment then runs code.split("=") over the combined text and unpacks two names from it.

At flush time the section is now split into the literal and whatever followed it, and the remainder is written verbatim after the sorted code. The flush point itself does not move, which matters: accumulated lines set line = "" and never reach output_stream, so the reexport rollback and truncate accounting is untouched.

I tried the more obvious fix first, flushing as soon as the brackets balance, and it corrupts output. That happens before the current line's bytes are written, so the rollback relocates content above __all__. test_reexport_not_first_line catches it. There is a regression test here for that case specifically.

# isort: assignments is returned whole, since it deliberately spans several bracket-free statements until a blank line.

literal.py now uses split("=", 1). That alone does not fix anything, it just turns the crash into a parse failure, but it seemed worth having.

Verification

Four tests in tests/unit/test_regressions.py: both repros above, a case where __all__ is not the first line, and a multi-line __all__. All four fail without the core.py change.

Also checked by hand, all sorting correctly, parsing, and idempotent on a second pass: a literal indented inside a class, a bracket inside a string value, a trailing comment, a nested # isort: dict, and the existing blank-line-separated form.

pytest tests/unit gives 588 passed against 584 before, the difference being these four. Two failures are the same on both trees, FileNotFoundError from git missing in my environment.

One case I did not fix: # isort: split inside a literal produces mangled output. That is pre-existing, byte for byte identical before and after this change on a blank-line-terminated literal, so I left it alone rather than widen the diff. Happy to open a separate issue for it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.37%. Comparing base (131f4ad) to head (70f7eac).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2627   +/-   ##
=======================================
  Coverage   99.37%   99.37%           
=======================================
  Files          41       41           
  Lines        3181     3198   +17     
  Branches      686      690    +4     
=======================================
+ Hits         3161     3178   +17     
  Misses         12       12           
  Partials        8        8           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DanielNoord]

Ci fails.

Also, this feels really really complex for such a small bug. Is there a solution that does not involve such a complex function?

[VXNCXNX]

Both fair. CI was Lint (C901, process went 93 > 91) and codecov/patch, not the test suite.

Dropped the bracket scanner for ast.parse — the split point is just where the second statement begins, so quotes, nesting and indentation come for free. 40 lines down to 8, and merging the two write calls keeps process under the complexity limit. core.py is back at 100%.

[Manny7717]

Verified locally (head ecb32a2, isort from source): all 5 new issue_2286 tests pass, and the original crash reproduces on main for both the --sort-reexports and # isort: list variants and is fixed here. Full CI matrix on this head is green.

However, I found a related case this PR silently corrupts, which I think is worth resolving before merge (inline comment below):

__all__ = ["b", "a"]
# note
x = 1

isort.code(..., sort_reexports=True) on main raises ValueError: too many values to unpack; on this head it returns:

__all____all__ = ["a", "b"]
x = 1

i.e. the standalone comment is silently dropped, the identifier is duplicated, and the real __all__ no longer exists — so from module import * semantics silently change. Also reproduces with the comment at EOF (__all__ = ["b", "a"]\n# note\n -> __all____all__ = ["a", "b"]\n). isort.check_code(..., sort_reexports=True) does return False for it, so --check/CI catches the change — but anyone running isort file.py gets a silently corrupted file.

Two related, milder observations from the same family:

• __all__ = ["b", "a"] # exports\nx = 1\n -> inline comment silently dropped (no corruption).
• # isort: list\n__all__ = ["b", "a"]\n# note\nx = 1\n -> sorted correctly, but the comment is relocated above the literal.

Suggested direction (happy to help): when a section contains comment lines, either bail out and leave the section unsorted, or exclude comments from both the literal section and the rollback accounting — with regression tests for the comment-in-section cases. This also speaks to the complexity concern raised on the PR: the AST-split needs to mirror exactly what was already emitted to the stream.

Out of scope / pre-existing (unchanged by this PR): a literal containing = (e.g. __all__ = [f"{x}={y}" for x in ...]) still crashes at isort/core.py:291 (stripped_line.split("=")) on both main and this head.

[Manny7717]

The split assumes everything in code_sorting_section was not yet written, but a standalone comment line inside the section was already emitted to the output stream earlier (the in_top_comment path), while reexport_rollback only covers the literal line. The reexport seek(tell() - reexport_rollback) therefore lands mid-line and the sorted rewrite overlaps a stale prefix.

Repro: __all__ = ["b", "a"]\n# note\nx = 1\n with --sort-reexports -> __all____all__ = ["a", "b"]\nx = 1\n (real __all__ deleted, comment dropped). Pre-fix this input crashed loudly; post-fix it corrupts silently.

Suggestion: if the section contains comment lines (standalone or trailing), skip sorting and pass the section through unchanged; or strip comments from literal_section and include them in the rollback. Either way a regression test for the comment-in-section case would pin it down.

[VXNCXNX]

@Manny7717 thank you, this was a real defect and I reproduced all of it. Fixed, with one correction to the diagnosis.

The corruption was not introduced by this PR. __all____all__ already happens on main:

MAIN  __all__ = ["b", "a"]\n# note\n   ->  '__all____all__ = ["a", "b"]\n'

What this PR did was widen the set of inputs that reach it, since inputs that used to crash on code.split("=") now get far enough to hit the bad seek. So the corruption is pre-existing and this PR has to fix it rather than merely avoid it.

Root cause. A comment line under the literal is matched by the top-of-file comment heuristic (index in {1, 2} and not contains_imports), so it is written straight to the output stream. reexport_rollback only covers the __all__ line, so by the time of the seek the stream is 7 bytes longer than the rollback accounts for, and seek(tell() - rollback) lands inside __all__. Hence the duplicated identifier.

I first tried the obvious repair, recording an absolute stream position instead of a relative rollback. That is wrong, and the test suite said so: isort buffers the import section, so the position when the __all__ line is seen is not where it is written, and test_reexport_not_first_line and the three multiline reexport tests all failed. The relative rollback is correct as long as nothing else is written in between, which is the actual invariant to restore.

So: guard the top-comment heuristic with not code_sorting, and end the literal at a standalone comment so it is passed through instead of being rebuilt away by the sorter. Both are load-bearing, each fails the new tests on its own when reverted.

input (sort_reexports=True)	main	before	now
__all__ + comment + stmt	ValueError	corrupts	__all__ sorted, comment in place
__all__ + comment at EOF	corrupts	corrupts	sorted, comment in place
# isort: list + comment	ValueError	comment moved above	comment in place

Four regression tests added, including one asserting check_code agrees with code on these inputs so --check can never call a file clean that isort would then rewrite.

Full suite: 630 passed. The only failures are the 4 that already fail on main in my environment (issue_909, issue_938, issue_970, issue_1732, all FileNotFoundError/SortingFunctionDoesNotExist). ruff and mypy clean.

Still dropped, and unchanged by this PR because main does the same in the # isort: list path: a trailing comment on the literal line itself (__all__ = [...] # exports). Happy to take that separately rather than grow this PR.

Your last point stands too: a literal containing = still crashes at stripped_line.split("="). Also pre-existing, also out of scope here.

---

@DanielNoord on both of your points:

CI is green now, that was a stale run from before a rebase.

On complexity, _split_code_sorting_section is 15 lines and its whole job is "find where the literal ends". It ends at the next statement, or at a standalone comment. ast.parse gives the first exactly, via body[1].lineno. The alternative is counting brackets by hand while tracking strings and comments, which is what the surrounding code does today and is what made the original bug possible. I think this is the smaller and more boring of the two options, but if you would rather not carry it at all, the narrowest possible change that fixes the reported crash alone is the one-character code.split("=", 1) in literal.py. That leaves the statement still swallowed, so it trades a crash for silent deletion, which is why I did not stop there. Happy to cut it down if you prefer.

AI-assisted: written with Claude Code, reviewed and tested by me before sending.

[Manny7717]

Verified your fix locally on head 70f7eac — and yes, your correction to my diagnosis is right: on main (131f4ad), the EOF shape already corrupts (__all____all__ = ["a", "b"]\n), while the statement shape crashed with ValueError: too many values to unpack. So this PR widened reachability of a pre-existing corruption and your fix closes it properly rather than just avoiding it. That's a better fix than my suggestion, and the relative-rollback-vs-absolute-position reasoning in the commit message is correct.

What I re-ran:

• All three shapes from my original finding now produce sorted output with the comment preserved in place: reexports+comment+stmt, reexports+comment at EOF, and # isort: list+comment.
• The new check_code-agreement test: verified by hand that check_code returns True iff code() is a no-op on all three inputs (the ERROR lines it prints for unsorted inputs are the expected check failure output).
• Full unit suite: 630 passed / 4 failed on head; the identical 4 fail on main (issue_909/938/970/1732, FileNotFoundError env noise) — zero new failures introduced.
• ruff clean on changed files.

Both remaining gaps are confirmed pre-existing and correctly scoped out: the trailing inline comment (__all__ = [...] # exports) is dropped identically on main, and a literal containing = still raises LiteralParsingFailure on both. Happy to look at the trailing-comment case separately if you open it as an issue.

[VXNCXNX]

Thanks for re-running it, and for catching the original defect. Our numbers match: 630 / 4 on this head, the same 4 on main.

Filed the trailing-comment case as #2646 since you offered to look at it. Two symptoms on that seam, both pre-existing and both untouched by this PR:

• # noqa: F401 and # type: ignore on an __all__ line are silently deleted, so a suppressed lint error comes back or a clean mypy run starts failing
• a trailing comment containing =, such as # pylint: disable=invalid-name, hits the code.split("=") you mentioned and raises

The second is the same = bug you flagged, with a more ordinary trigger than a literal containing =.

@DanielNoord one thing that is easy to miss from the checks list here: the CI run you saw failing was from before a rebase, and the current run is sitting at action_required waiting on workflow approval for a fork PR, so only DeepSource and readthedocs have reported. Locally on 70f7eac8 the unit suite is 630 passed with the same 4 environment failures main has, and ruff and mypy are clean. Happy to answer the complexity question further, or to cut this down to the one-character split("=", 1) if you would rather not carry _split_code_sorting_section.

[Manny7717]

The #2286 fix works (verified: __all__ = ["b", "a"]\nx = 1 and the # isort: list variant now sort instead of raising ValueError; all 9 new tests pass; full suite head 632 passed / 2 pre-existing env failures vs base 623 / 2 — zero regressions on the covered shapes). However, the new _split_code_sorting_section introduces a crash on input that base completed: any standalone comment line INSIDE a multi-line literal truncates the section at that comment, leaving an unclosed bracket → LiteralParsingFailure. Comments inside a sorted literal are exactly the shapes isort previously accepted (it rebuilds the literal from its parsed value, dropping the comment — pre-existing #2646-family behavior).

Repros (base @ 131f4ad OK → head @ 70f7eac CRASH):

# isort: list
NAMES = [
    "b",
    "a",
    # note
]

__all__ = [
    "b",
    "a",
    # note
]
# with sort_reexports=True

Base: NAMES = ["a", "b"] / __all__ = ["a", "b"] (sorted, comment dropped as before). Head: LiteralParsingFailure: isort failed to parse the given literal NAMES = [\n "b",\n "a",\n. — the section is cut at the comment line, so ast.literal_eval gets an unclosed [. CLI impact: isort --check on such a file goes from "Imports are incorrectly sorted" (actionable, exit 1) to "ERROR: isort failed to parse the given literal" — a hard failure blaming the user's code for something isort used to process.

Root cause: core.py:84-86. When the section has a single statement (len(body) == 1), literal_end = len(lines) and the comment scan runs over the WHOLE section including bracket-nested comment lines. The scan should only treat a comment as a terminator when it starts AFTER the first statement's end line (body[0].end_lineno - 1); comment lines inside the brackets belong to the literal, not to the trailing section.

Verified fix (one-loop-scope change, full suite still 632/2, all 9 new tests pass, all crash cases resolve):

literal_end = body[1].lineno - 1 if len(body) > 1 else len(lines)
stmt_end = body[0].end_lineno - 1
for index, line in enumerate(lines[stmt_end:literal_end], start=stmt_end):
    if line.lstrip().startswith("#"):
        literal_end = index
        break

Worth adding a regression test for an interior/trailing comment inside a multi-line literal (both the # isort: list and sort_reexports paths).

[Manny7717]

This loop scans from line 0 of the section, so a comment nested inside a multi-line literal's brackets (e.g. NAMES = [\n "b",\n # note\n "a",\n]) is mistaken for the end of the literal → the section is cut mid-bracket and literal.assignment raises LiteralParsingFailure. Base completed on such input (sorted + dropped the comment, the known #2646 limitation); head now hard-crashes, including via isort --check. Suggest starting the scan after the first statement's end line (body[0].end_lineno - 1) so only comments that follow the closed literal can terminate the section.

[VXNCXNX]

Reproduced both shapes and confirmed it against base: main sorts them and drops the comment, this head raised LiteralParsingFailure. Trading "imports are incorrectly sorted" for a parse error that blames the user's file is worse than the bug I was fixing, so this needed to go in.

Took your scan bounds as written, with one addition: body[0] needs an empty-body guard or it indexes out of range. Reverting just that loop fails the two new tests and nothing else. 632 passed with the same 4 environment failures main has, ruff and mypy clean.

[Manny7717]

Verified the follow-up fix on head 9217b1c — it resolves the regression I reported in my last review, and does so exactly along the lines I suggested.

What I verified

• The two new regression tests pass: test_isort_list_comment_inside_multiline_literal_issue_2286 and test_sort_reexports_comment_inside_multiline_all_issue_2286. All 11 issue_2286 tests in test_regressions.py pass; test_regressions.py + test_literal.py = 130 passed.
• The scan now starts at body[0].end_lineno - 1 (after the literal's closing bracket) with an empty-body guard, so comments nested inside the brackets belong to the literal instead of terminating the section mid-bracket.
• Manual probes of the reported crash shapes, both via # isort: list and sort_reexports=True: comments nested inside multi-line literals (head-position, middle, and inline-tail) now sort without raising LiteralParsingFailure; a standalone comment or statement after the closed literal still terminates the section correctly (original #2286 behavior preserved); the EOF shape stays intact.
• Full unit suite on head: 634 passed, 2 failed — both failures (test_settings_path_skip_issue_909, test_skip_paths_issue_938) are the known pre-existing FileNotFoundError env noise that also fails identically on main. Zero new failures.

Nice catch shipping the regression test alongside the fix — the previous head traded "wrongly sorted" for a user-blame parse error, and this restores graceful sorting for all the nested-comment shapes.