Skip to content

Fix multi_line_output=10 emitting unparsable code around trailing comments - #2651

Open
dylanpulver wants to merge 1 commit into
PyCQA:mainfrom
dylanpulver:fix-hanging-indent-parentheses-comment-syntax
Open

Fix multi_line_output=10 emitting unparsable code around trailing comments#2651
dylanpulver wants to merge 1 commit into
PyCQA:mainfrom
dylanpulver:fix-hanging-indent-parentheses-comment-syntax

Conversation

@dylanpulver

Copy link
Copy Markdown

Fixes #2650.

multi_line_output=10 (HANGING_INDENT_WITH_PARENTHESES) can rewrite a valid file into one that no longer parses when an import carries a trailing comment:

$ isort --multi-line 10 --line-length 40 --force-grid-wrap 2 -
from a import (b, c, d  # trailing)

) sits inside the comment.

$ isort --multi-line 10 -
from mypkg.submodule import (alpha, beta, delta, epsilon, eta  # noqa: F401
    gamma, zeta)

the comma after eta is gone. Both raise SyntaxError.

Cause

The sibling grouped modes hoist the comment onto the ( line, so their last line never ends in a comment. This mode keeps it on the import line and then appends punctuation unconditionally: the final return puts ) after the comment, and at a wrap point statement + "," puts the comma after the comment, where comments.add_to_line strips it off with the old comment before re-adding.

Fix

_add_syntax() places ,/) before a trailing comment on the last line, used at both sites.

Verification

  • 7 sources x 12 modes x 3 line lengths x 6 option sets (1512 runs): 17 unparsable outputs before, all mode 10; 0 after.
  • Suite: 625 passed 1 skipped before, 626 after (the new test); nothing else changed.
  • isort --profile hug --check-only, ruff format --check, ruff check, mypy clean.
  • Reverting wrap_modes.py fails the new test on '(' was never closed; a partial fix that only relocates ) still fails it on the lost comma.

…ments

hanging_indent_with_parentheses keeps a trailing comment on the import line
rather than hoisting it to the opening parenthesis like its sibling modes do,
then appended punctuation to that line unconditionally. The closing ) landed
inside the comment, and a comma added at a wrap point was consumed by the
comment re-splice in comments.add_to_line, losing a separator.

Closes PyCQA#2650

Co-authored-by: claude-opus-5 <noreply@anthropic.com>

@Manny7717 Manny7717 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally against head f09b1ce (base = origin/main @ 131f4ad). Bug is real and the fix is correct.

Bug confirmed: mode 10 (HANGING_INDENT_WITH_PARENTHESES) appended punctuation after a trailing comment. CLI repro on base: from a import b, c, d # trailing\nfrom a import e\n with --multi-line 10 --line-length 40 --force-grid-wrap 2 emits from a import (b, c, d, e # trailing) — the ) lands inside the comment (SyntaxError). Same input on head emits valid from a import (b, c, d, e) # trailing.

Regression coverage proven: transplanted the new test into the base worktree — it fails there with the exact reported SyntaxError: '(' was never closed, and passes on head.

No collateral: full unit suite — head 624 passed / 2 failed / 1 skipped; base+new-test 623 passed / 3 failed. The 2 failures on head (test_settings_path_skip_issue_909, test_skip_paths_issue_938, FileNotFoundError) reproduce identically on base (pre-existing env noise, unrelated). ruff check and ruff format --check clean on both changed files.

Fix review: _add_syntax() correctly handles both append sites (the wrap-point comma and the final closing paren), preserves original spacing before # (code[len(code.rstrip()):] or " "), and is a no-op passthrough when the last line carries no comment. partition("#") is safe here because import statements in this mode cannot contain # before the comment start. The in-test matrix (all WrapModes × 3 line lengths × 3 sources × trailing-comma on/off, every output ast.parsed) is a good guard against this class of bug across sibling modes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multi_line_output=10 emits unparsable code when an import has a trailing comment

2 participants