Skip to content

Commit 33b29f0

Browse files
committed
fix: keep comment-only lines inside from-import groups
Comment-only members of a parenthesised from-import were collapsed onto the opening import line as a single semicolon-joined comment. With the black profile that rewrote intentional WIP comments and produced lines that fail line-length checkers. Track those body comments separately and re-emit them as indented lines inside parenthesised wrapping; preserve the historical collapse for non-parenthesised wrap modes. Fixes #1852 Signed-off-by: Alex Chen <l46983284@gmail.com>
1 parent 131f4ad commit 33b29f0

3 files changed

Lines changed: 379 additions & 6 deletions

File tree

isort/output.py

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from . import _parse_utils, parse, sorting, wrap, wrap_modes
1010
from .comments import add_to_line as with_comments
11+
from .comments import parse as parse_comment
1112
from .identify import STATEMENT_DECLARATIONS
1213
from .place import module_with_reason
1314
from .settings import DEFAULT_CONFIG, Config
@@ -297,6 +298,69 @@ def _build_import_group(
297298
return group_output
298299

299300

301+
def _inject_from_body_comments(
302+
import_statement: str,
303+
body_comments: list[str],
304+
line_separator: str,
305+
indent: str,
306+
*,
307+
comment_prefix: str,
308+
ignore_comments: bool = False,
309+
) -> str:
310+
"""Re-insert comment-only lines inside a multi-line from-import statement.
311+
312+
Comment-only members of a parenthesised import group must stay as their own
313+
indented lines. Collapsing them onto the opening ``import (`` line produces
314+
a single long ``# a,; b,; c`` comment that breaks line-length checkers.
315+
See issue #1852.
316+
317+
When the statement is single-line (no closing ``)``), fold the body comments
318+
onto that statement with ``with_comments`` instead of emitting orphan
319+
indented comment lines. Orphan lines are non-idempotent under a second sort.
320+
"""
321+
if not body_comments:
322+
return import_statement
323+
324+
comment_lines = [
325+
f"{indent}# {comment_text}".rstrip() if comment_text else f"{indent}#"
326+
for comment_text in body_comments
327+
]
328+
lines = import_statement.split(line_separator)
329+
330+
# Preferred placement: immediately before the closing ``)`` of a multi-line
331+
# parenthesised import so the comments stay inside the group.
332+
for index in range(len(lines) - 1, -1, -1):
333+
if lines[index].lstrip().startswith(")"):
334+
lines[index:index] = comment_lines
335+
return line_separator.join(lines)
336+
337+
# Single-line / no-paren fallback: main-compatible fold onto the statement.
338+
# Merge any trailing comment already on the statement (nested inline) after body
339+
# comments so we keep main's ``# body; nested`` shape instead of clobbering.
340+
if ignore_comments:
341+
return with_comments(
342+
body_comments,
343+
import_statement,
344+
removed=True,
345+
comment_prefix=comment_prefix,
346+
)
347+
_base, existing_comment = parse_comment(import_statement)
348+
# Drop spacing that used to precede an inline comment so re-attach is stable.
349+
_base = _base.rstrip()
350+
merged = list(body_comments)
351+
if existing_comment:
352+
for part in existing_comment.split(";"):
353+
part = part.strip()
354+
if part and part not in merged:
355+
merged.append(part)
356+
return with_comments(
357+
merged,
358+
_base,
359+
removed=False,
360+
comment_prefix=comment_prefix,
361+
)
362+
363+
300364
def _build_as_imports(
301365
*,
302366
from_import: str, # Y in `from X import Y as Z`
@@ -592,22 +656,34 @@ def _with_from_imports_for_module(
592656

593657
comments: list[str] = parsed.categorized_comments["from"].pop(module, [])
594658
above_comments = parsed.categorized_comments["above"]["from"].pop(module, None)
659+
body_comments: list[str] = list(
660+
parsed.categorized_comments.get("from_body", {}).pop(module, [])
661+
)
662+
# Parenthesised black-style wrapping can keep comment-only import members as
663+
# their own lines (issue #1852). Other wrap modes historically collapsed those
664+
# comments onto the import statement (issue #1396); preserve that behaviour.
665+
if body_comments and not config.use_parentheses:
666+
comments = list(comments or []) + body_comments
667+
body_comments = []
595668
if above_comments:
596669
output.extend(above_comments)
597670

598671
only_show_as_imports = False
599672
if "*" in from_imports:
600673
from_imports.remove("*")
601674

675+
# Fold from_body comments onto the star statement (main-compatible).
676+
star_comments = list(comments if config.combine_star else [])
677+
if body_comments and not config.ignore_comments:
678+
star_comments = star_comments + body_comments
679+
body_comments = []
602680
output.append(
603681
wrap.line(
604682
with_comments(
605683
_with_star_comments(
606684
parsed,
607685
module,
608-
# If we are combining the star imports we want to include all from-import
609-
# comments we found above.
610-
comments if config.combine_star else [],
686+
star_comments,
611687
),
612688
f"{import_start}*",
613689
removed=config.ignore_comments,
@@ -626,6 +702,14 @@ def _with_from_imports_for_module(
626702

627703
# Handle force_single_line
628704
if config.force_single_line and module not in config.single_line_exclusions:
705+
# Pending comments (opening + body) must land on the first *emitted*
706+
# single-line statement. As-only modules never emit the bare name, so
707+
# folding into ``comments`` on that non-emitted line drops body text.
708+
pending_comments: list[str] = list(comments or [])
709+
if body_comments and not config.ignore_comments:
710+
pending_comments.extend(body_comments)
711+
body_comments = []
712+
comments = []
629713
for from_import in from_imports:
630714
if from_import in as_imports:
631715
output.extend(
@@ -636,7 +720,7 @@ def _with_from_imports_for_module(
636720
line_separator=parsed.line_separator,
637721
config=config,
638722
straight_comments=[
639-
*comments,
723+
*pending_comments,
640724
*parsed.categorized_comments["straight"].get(
641725
f"{module}.{from_import}", []
642726
),
@@ -648,12 +732,13 @@ def _with_from_imports_for_module(
648732
),
649733
)
650734
)
735+
pending_comments = []
651736
else:
652737
single_import_line = with_comments(
653738
[
654739
c
655740
for c in (
656-
*comments,
741+
*pending_comments,
657742
parsed.categorized_comments["nested"]
658743
.get(module, {})
659744
.pop(from_import, None),
@@ -666,7 +751,7 @@ def _with_from_imports_for_module(
666751
)
667752

668753
output.append(wrap.line(single_import_line, parsed.line_separator, config))
669-
comments = []
754+
pending_comments = []
670755
return output
671756

672757
while from_imports:
@@ -771,7 +856,30 @@ def _with_from_imports_for_module(
771856
processed_as_imports_this_iteration=processed_as_imports_this_iteration,
772857
)
773858
if grouped_from_import_statement:
859+
if body_comments and not config.ignore_comments:
860+
grouped_from_import_statement = _inject_from_body_comments(
861+
grouped_from_import_statement,
862+
body_comments,
863+
parsed.line_separator,
864+
config.indent,
865+
comment_prefix=config.comment_prefix,
866+
ignore_comments=config.ignore_comments,
867+
)
868+
body_comments = []
774869
output.append(grouped_from_import_statement)
870+
elif body_comments and not config.ignore_comments and output:
871+
# as-import-only / nested-only paths may emit lines without a final
872+
# grouped import_statement. Never drop body comments: fold them onto
873+
# the last statement emitted for this module.
874+
output[-1] = _inject_from_body_comments(
875+
output[-1],
876+
body_comments,
877+
parsed.line_separator,
878+
config.indent,
879+
comment_prefix=config.comment_prefix,
880+
ignore_comments=config.ignore_comments,
881+
)
882+
body_comments = []
775883

776884
# Reset comments as we have just parsed them.
777885
comments = []

isort/parse.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@
3131
"straight": dict[str, list[str]],
3232
"nested": dict[str, dict[str, str]],
3333
"above": CommentsAboveDict,
34+
# Comment-only lines that appeared inside a parenthesised from-import
35+
# (e.g. `` # PasswordChangeView,``). Kept separate from opening-line
36+
# ``from`` comments so they can be re-emitted as their own indented lines
37+
# instead of being collapsed onto the ``import (`` line. See issue #1852.
38+
"from_body": dict[str, list[str]],
3439
},
3540
)
3641

@@ -112,6 +117,7 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
112117
"straight": {},
113118
"nested": {},
114119
"above": {"straight": {}, "from": {}},
120+
"from_body": {},
115121
}
116122

117123
trailing_commas: set[str] = set()
@@ -238,6 +244,13 @@ def _get_next_line() -> tuple[str, str | None]:
238244
raw_lines.append(extra_line.line)
239245
# If during parsing of the continuation lines we encounter a comment, we record it.
240246
if extra_line.comment is not None:
247+
code_part = extra_line.line.split("#", 1)[0].strip().rstrip(",")
248+
# A continuation line that is only a comment (no import name before ``#``)
249+
# is not an attribute comment and must not be attached to the opening
250+
# ``from ... import (`` line. Keep it as a body comment so output can
251+
# re-emit it as its own indented line. See issue #1852.
252+
if type_of_import == "from" and not code_part:
253+
continue
241254
comments.append(extra_line.comment)
242255
stripped_line = strip_syntax(extra_line.line).strip()
243256
if (
@@ -316,6 +329,18 @@ def _get_next_line() -> tuple[str, str | None]:
316329

317330
if type_of_import == "from":
318331
import_from = just_imports.pop(0)
332+
# Preserve comment-only lines from inside the parenthesised import group.
333+
# They were skipped above so they would not collapse onto the opening line.
334+
body_comments = [
335+
extra_line.comment
336+
for extra_line in extra_lines
337+
if extra_line.comment is not None
338+
and not extra_line.line.split("#", 1)[0].strip().rstrip(",")
339+
]
340+
if body_comments:
341+
categorized_comments["from_body"].setdefault(import_from, []).extend(
342+
body_comments
343+
)
319344
placed_module = finder(import_from)
320345
if config.verbose and not config.only_modified:
321346
print(f"from-type place_module for {import_from} returned {placed_module}")

0 commit comments

Comments
 (0)