diff --git a/isort/output.py b/isort/output.py index cc4fb2b2..82587fe7 100644 --- a/isort/output.py +++ b/isort/output.py @@ -8,6 +8,7 @@ from . import _parse_utils, parse, sorting, wrap, wrap_modes from .comments import add_to_line as with_comments +from .comments import parse as parse_comment from .identify import STATEMENT_DECLARATIONS from .place import module_with_reason from .settings import DEFAULT_CONFIG, Config @@ -283,6 +284,69 @@ def _build_import_group( return group_output +def _inject_from_body_comments( + import_statement: str, + body_comments: list[str], + line_separator: str, + indent: str, + *, + comment_prefix: str, + ignore_comments: bool = False, +) -> str: + """Re-insert comment-only lines inside a multi-line from-import statement. + + Comment-only members of a parenthesised import group must stay as their own + indented lines. Collapsing them onto the opening ``import (`` line produces + a single long ``# a,; b,; c`` comment that breaks line-length checkers. + See issue #1852. + + When the statement is single-line (no closing ``)``), fold the body comments + onto that statement with ``with_comments`` instead of emitting orphan + indented comment lines. Orphan lines are non-idempotent under a second sort. + """ + if not body_comments: + return import_statement + + comment_lines = [ + f"{indent}# {comment_text}".rstrip() if comment_text else f"{indent}#" + for comment_text in body_comments + ] + lines = import_statement.split(line_separator) + + # Preferred placement: immediately before the closing ``)`` of a multi-line + # parenthesised import so the comments stay inside the group. + for index in range(len(lines) - 1, -1, -1): + if lines[index].lstrip().startswith(")"): + lines[index:index] = comment_lines + return line_separator.join(lines) + + # Single-line / no-paren fallback: main-compatible fold onto the statement. + # Merge any trailing comment already on the statement (nested inline) after body + # comments so we keep main's ``# body; nested`` shape instead of clobbering. + if ignore_comments: + return with_comments( + body_comments, + import_statement, + removed=True, + comment_prefix=comment_prefix, + ) + _base, existing_comment = parse_comment(import_statement) + # Drop spacing that used to precede an inline comment so re-attach is stable. + _base = _base.rstrip() + merged = list(body_comments) + if existing_comment: + for part in existing_comment.split(";"): + part = part.strip() + if part and part not in merged: + merged.append(part) + return with_comments( + merged, + _base, + removed=False, + comment_prefix=comment_prefix, + ) + + # Ignore DeepSource cyclomatic complexity check for this function. It was # already complex when this check was enabled. # skipcq: PY-R1000 @@ -353,15 +417,29 @@ def _with_from_imports( only_show_as_imports = False comments: list[str] | None = parsed.categorized_comments["from"].pop(module, None) above_comments = parsed.categorized_comments["above"]["from"].pop(module, None) + body_comments: list[str] = list( + parsed.categorized_comments.get("from_body", {}).pop(module, []) + ) + # Parenthesised black-style wrapping can keep comment-only import members as + # their own lines (issue #1852). Other wrap modes historically collapsed those + # comments onto the import statement (issue #1396); preserve that behaviour. + if body_comments and not config.use_parentheses: + comments = list(comments or []) + body_comments + body_comments = [] while from_imports: if above_comments: output.extend(above_comments) above_comments = None if "*" in from_imports and config.combine_star: + # Fold from_body comments onto the star statement (main-compatible). + star_comments = list(comments or ()) + if body_comments and not config.ignore_comments: + star_comments = star_comments + body_comments + body_comments = [] import_statement = wrap.line( with_comments( - _with_star_comments(parsed, module, list(comments or ())), + _with_star_comments(parsed, module, star_comments), f"{import_start}*", removed=config.ignore_comments, comment_prefix=config.comment_prefix, @@ -375,10 +453,19 @@ def _with_from_imports( only_show_as_imports = True elif config.force_single_line and module not in config.single_line_exclusions: import_statement = "" + # Pending comments (opening + body) must land on the first *emitted* + # single-line statement. As-only modules never emit the bare name, so + # folding into ``comments`` on that non-emitted line drops body text. + pending_comments: list[str] = list(comments or []) + if body_comments and not config.ignore_comments: + pending_comments.extend(body_comments) + body_comments = [] + comments = None while from_imports: from_import = from_imports.pop(0) + line_comments = pending_comments or None single_import_line = with_comments( - comments, + line_comments, import_start + from_import, removed=config.ignore_comments, comment_prefix=config.comment_prefix, @@ -389,9 +476,10 @@ def _with_from_imports( if comment is not None: comment_text = f" {comment}" if comment else "" single_import_line += ( - f"{(comments and ';') or config.comment_prefix}{comment_text}" + f"{(line_comments and ';') or config.comment_prefix}{comment_text}" ) if from_import in as_imports: + emitted_plain = False if ( parsed.imports[section][import_key][module][from_import] and not only_show_as_imports @@ -399,30 +487,26 @@ def _with_from_imports( output.append( wrap.line(single_import_line, parsed.line_separator, config) ) - from_comments = parsed.categorized_comments["straight"].get( - f"{module}.{from_import}" + pending_comments = [] + emitted_plain = True + from_comments = list( + parsed.categorized_comments["straight"].get(f"{module}.{from_import}") + or [] ) - - if not config.only_sections: - output.extend( - wrap.line( - with_comments( - from_comments, - import_start + as_import, - removed=config.ignore_comments, - comment_prefix=config.comment_prefix, - ), - parsed.line_separator, - config, - ) - for as_import in sorting.sort(config, as_imports[from_import]) - ) - - else: - output.extend( + as_import_names = ( + sorting.sort(config, as_imports[from_import]) + if not config.only_sections + else list(as_imports[from_import]) + ) + for index, as_import in enumerate(as_import_names): + as_line_comments = list(from_comments) if index == 0 else [] + if index == 0 and pending_comments and not emitted_plain: + as_line_comments = pending_comments + as_line_comments + pending_comments = [] + output.append( wrap.line( with_comments( - from_comments, + as_line_comments or None, import_start + as_import, removed=config.ignore_comments, comment_prefix=config.comment_prefix, @@ -430,11 +514,10 @@ def _with_from_imports( parsed.line_separator, config, ) - for as_import in as_imports[from_import] ) else: output.append(wrap.line(single_import_line, parsed.line_separator, config)) - comments = None + pending_comments = [] else: # Tracks whether any aliased imports were emitted before the grouped # non-aliased imports in this pass of the outer loop. When True it @@ -697,7 +780,30 @@ def _with_from_imports( comments = None if import_statement: + if body_comments and not config.ignore_comments: + import_statement = _inject_from_body_comments( + import_statement, + body_comments, + parsed.line_separator, + config.indent, + comment_prefix=config.comment_prefix, + ignore_comments=config.ignore_comments, + ) + body_comments = [] output.append(import_statement) + elif body_comments and not config.ignore_comments and output: + # as-import-only / nested-only paths may emit lines without a final + # grouped import_statement. Never drop body comments: fold them onto + # the last statement emitted for this module. + output[-1] = _inject_from_body_comments( + output[-1], + body_comments, + parsed.line_separator, + config.indent, + comment_prefix=config.comment_prefix, + ignore_comments=config.ignore_comments, + ) + body_comments = [] return output diff --git a/isort/parse.py b/isort/parse.py index 03678397..beeae9ad 100644 --- a/isort/parse.py +++ b/isort/parse.py @@ -31,6 +31,11 @@ "straight": dict[str, list[str]], "nested": dict[str, dict[str, str]], "above": CommentsAboveDict, + # Comment-only lines that appeared inside a parenthesised from-import + # (e.g. `` # PasswordChangeView,``). Kept separate from opening-line + # ``from`` comments so they can be re-emitted as their own indented lines + # instead of being collapsed onto the ``import (`` line. See issue #1852. + "from_body": dict[str, list[str]], }, ) @@ -108,6 +113,7 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte "straight": {}, "nested": {}, "above": {"straight": {}, "from": {}}, + "from_body": {}, } trailing_commas: set[str] = set() @@ -234,6 +240,13 @@ def _get_next_line() -> tuple[str, str | None]: raw_lines.append(extra_line.line) # If during parsing of the continuation lines we encounter a comment, we record it. if extra_line.comment is not None: + code_part = extra_line.line.split("#", 1)[0].strip().rstrip(",") + # A continuation line that is only a comment (no import name before ``#``) + # is not an attribute comment and must not be attached to the opening + # ``from ... import (`` line. Keep it as a body comment so output can + # re-emit it as its own indented line. See issue #1852. + if type_of_import == "from" and not code_part: + continue comments.append(extra_line.comment) stripped_line = strip_syntax(extra_line.line).strip() if ( @@ -312,6 +325,18 @@ def _get_next_line() -> tuple[str, str | None]: if type_of_import == "from": import_from = just_imports.pop(0) + # Preserve comment-only lines from inside the parenthesised import group. + # They were skipped above so they would not collapse onto the opening line. + body_comments = [ + extra_line.comment + for extra_line in extra_lines + if extra_line.comment is not None + and not extra_line.line.split("#", 1)[0].strip().rstrip(",") + ] + if body_comments: + categorized_comments["from_body"].setdefault(import_from, []).extend( + body_comments + ) placed_module = finder(import_from) if config.verbose and not config.only_modified: print(f"from-type place_module for {import_from} returned {placed_module}") diff --git a/tests/unit/test_regressions.py b/tests/unit/test_regressions.py index 09043352..dc37f79d 100644 --- a/tests/unit/test_regressions.py +++ b/tests/unit/test_regressions.py @@ -6,7 +6,11 @@ import isort import isort.sections +from isort import parse +from isort.comments import add_to_line as with_comments from isort.main import main +from isort.output import _inject_from_body_comments, _with_from_imports +from isort.settings import Config def test_isort_duplicating_comments_issue_1264(): @@ -2406,3 +2410,237 @@ def test_isort_does_not_drop_aliased_import_when_plain_name_has_a_comment(): expected = "from . import bar\nfrom . import one # NOQA\nfrom . import one as zzz\n" assert relative_sorted == expected assert isort.code(relative_sorted) == relative_sorted + + +def test_comment_only_lines_in_from_import_group_issue_1852(): + """Comment-only lines inside a parenthesised from-import must stay as lines. + + With the black profile, isort used to collapse commented-out import members + onto the opening ``import (`` line as a single ``# a,; b,; c`` comment. That + rewrites intentional WIP comments and commonly trips line-length checkers. + See issue #1852. + """ + to_sort = ( + "from dj_rest_auth.views import (\n" + " LoginView,\n" + " LogoutView,\n" + " # PasswordChangeView,\n" + " # PasswordResetConfirmView,\n" + " # PasswordResetView,\n" + " # UserDetailsView,\n" + ")\n" + ) + expected = ( + "from dj_rest_auth.views import (\n" + " LoginView,\n" + " LogoutView,\n" + " # PasswordChangeView,\n" + " # PasswordResetConfirmView,\n" + " # PasswordResetView,\n" + " # UserDetailsView,\n" + ")\n" + ) + + first_pass = isort.code(to_sort, profile="black", line_length=100) + assert first_pass == expected + assert isort.code(first_pass, profile="black", line_length=100) == first_pass + assert isort.check_code(first_pass, profile="black", line_length=100, show_diff=True) + + # Mixed live imports and comment-only lines should keep the comments as + # separate lines after the sorted live names. + mixed = "from foo import (\n zeta,\n # disabled\n alpha,\n)\n" + mixed_expected = "from foo import (\n alpha,\n zeta,\n # disabled\n)\n" + mixed_out = isort.code(mixed, profile="black") + assert mixed_out == mixed_expected + assert isort.code(mixed_out, profile="black") == mixed_out + + +def test_comment_only_empty_from_import_line_issue_1852(): + """A bare comment-only member must stay a bare indented comment.""" + to_sort = "from foo import (\n zeta,\n #\n alpha,\n)\n" + expected = "from foo import (\n alpha,\n zeta,\n #\n)\n" + + assert isort.code(to_sort, profile="black") == expected + + +def test_from_import_body_comments_without_parentheses_issue_1852(): + """Non-parenthesised wrapping keeps body comments on the opening import line.""" + to_sort = "from foo import (\n zeta,\n # disabled\n alpha,\n)\n" + expected = "from foo import ( # disabled\n alpha,\n zeta,\n)\n" + + assert isort.code(to_sort, profile="black", use_parentheses=False) == expected + + +def test_force_single_line_preserves_body_comments_issue_1852(): + """force_single_line must not drop comment-only from-import members. + + Regression against silent loss after from_body routing (issue #1852). + """ + to_sort = "from foo import (\n zeta,\n # disabled\n alpha,\n)\n" + expected = "from foo import alpha # disabled\nfrom foo import zeta\n" + + first_pass = isort.code(to_sort, profile="black", force_single_line=True) + assert first_pass == expected + assert isort.code(first_pass, profile="black", force_single_line=True) == first_pass + + +def test_combine_star_folds_body_comments_issue_1852(): + """combine_star should fold body comments onto the star statement.""" + to_sort = "from foo import (\n # disabled\n *\n)\n" + expected = "from foo import * # disabled\n" + + first_pass = isort.code(to_sort, combine_star=True) + assert first_pass == expected + assert isort.code(first_pass, combine_star=True) == first_pass + + +def test_inject_from_body_comments_branches_issue_1852(): + """Paren form keeps body lines; single-line form folds via with_comments.""" + assert _inject_from_body_comments( + "from foo import alpha", [], "\n", " ", comment_prefix=" #" + ) == ("from foo import alpha") + assert _inject_from_body_comments( + "from foo import alpha", ["disabled", ""], "\n", " ", comment_prefix=" #" + ) == ("from foo import alpha # disabled; ") + assert _inject_from_body_comments( + "from foo import (\n alpha,\n)", + ["disabled", ""], + "\n", + " ", + comment_prefix=" #", + ) == ("from foo import (\n alpha,\n # disabled\n #\n)") + # ignore_comments path still folds body comments as removed metadata + assert _inject_from_body_comments( + "from foo import alpha", + ["disabled"], + "\n", + " ", + comment_prefix=" #", + ignore_comments=True, + ) == ("from foo import alpha") + # nested trailing comment merges after body text + assert _inject_from_body_comments( + "from foo import alpha # nested", + ["disabled"], + "\n", + " ", + comment_prefix=" #", + ) == ("from foo import alpha # disabled; nested") + # empty segment from a trailing ";" in an existing comment is skipped + assert _inject_from_body_comments( + "from foo import alpha # nested;", + ["disabled"], + "\n", + " ", + comment_prefix=" #", + ) == ("from foo import alpha # disabled; nested") + # duplicate body text is not re-appended from the existing comment + assert _inject_from_body_comments( + "from foo import alpha # nested", + ["nested", "disabled"], + "\n", + " ", + comment_prefix=" #", + ) == ("from foo import alpha # nested; disabled") + + +def test_force_single_line_emits_plain_before_as_with_body_comment_issue_1852(): + """force_single_line with both live and as names emits plain first, then as. + + Covers the branch that clears pending body comments onto the plain line + before emitting the as-import. + """ + to_sort = "from foo import (\n # disabled\n alpha,\n beta as b,\n)\n" + expected = "from foo import alpha # disabled\nfrom foo import beta as b\n" + first = isort.code(to_sort, force_single_line=True) + assert first == expected + assert isort.code(first, force_single_line=True) == first + + +def test_as_import_only_body_comment_preserved_issue_1852(): + """as-import-only groups must not drop comment-only body members.""" + to_sort = "from foo import (\n alpha as a,\n # disabled\n)\n" + expected = "from foo import alpha as a # disabled\n" + first = isort.code(to_sort, profile="black") + assert first == expected + assert isort.code(first, profile="black") == first + + +def test_mixed_as_and_live_body_comment_preserved_issue_1852(): + """Body comments survive when as-imports emit before remaining live names.""" + to_sort = "from foo import (\n alpha as a,\n # disabled\n beta,\n)\n" + first = isort.code(to_sort, profile="black") + assert "# disabled" in first + assert "alpha as a" in first + assert "beta" in first + for line in first.splitlines(): + if line.startswith(" #"): + assert "(" in first + assert ")" in first + assert isort.code(first, profile="black") == first + + +def test_combine_star_folds_body_comments_when_comments_are_enabled_issue_1852(monkeypatch): + """combine_star folds body comments only when comments are retained.""" + parsed = parse.file_contents( + "from foo import (\n # disabled\n *\n)\n", + Config(combine_star=True, use_parentheses=True), + ) + config = Config(combine_star=True, use_parentheses=True) + section = "THIRDPARTY" + module = "foo" + captured: list[list[str]] = [] + original_with_comments = with_comments + + def with_comments_probe(comments, *args, **kwargs): + captured.append(list(comments or [])) + return original_with_comments(comments, *args, **kwargs) + + monkeypatch.setattr("isort.output.with_comments", with_comments_probe) + + assert parsed.categorized_comments["from_body"] == {module: ["disabled"]} + assert _with_from_imports( + parsed, + config, + parsed.imports[section]["from"], + section, + [], + "import", + is_lazy=False, + ) == ["from foo import * # disabled"] + assert captured == [["disabled"]] + + +def test_parse_from_body_comments_issue_1852(): + """The parser stores only comment-only continuation members in from_body.""" + parsed = parse.file_contents( + "from foo import (\n alpha,\n # disabled\n beta, # nested\n)\n", Config() + ) + + assert parsed.categorized_comments["from_body"] == {"foo": ["disabled"]} + assert parsed.categorized_comments["nested"] == {"foo": {"beta": "nested"}} + + +def test_force_single_line_as_import_keeps_body_comment_issue_1852(): + """force_single_line must keep body comments on the first emitted as-import.""" + to_sort = "from foo import (\n # disabled\n alpha as a,\n)\n" + expected = "from foo import alpha as a # disabled\n" + first = isort.code(to_sort, force_single_line=True) + assert first == expected + assert isort.code(first, force_single_line=True) == first + + mixed = "from foo import (\n alpha as a,\n # disabled\n beta,\n)\n" + mixed_out = isort.code(mixed, force_single_line=True) + assert mixed_out == ("from foo import alpha as a # disabled\nfrom foo import beta\n") + assert isort.code(mixed_out, force_single_line=True) == mixed_out + + +def test_black_nested_inline_and_body_comment_merge_issue_1852(): + """Body comments folded onto a nested-comment line keep both texts.""" + to_sort = "from foo import (\n alpha, # a\n beta, # b\n # disabled\n)\n" + out = isort.code(to_sort, profile="black") + assert "alpha # a" in out + assert "disabled" in out + assert "# b" in out or "; b" in out or out.endswith("b\n") or " b" in out + assert "b" in out.split("beta", 1)[1] + assert isort.code(out, profile="black") == out