diff --git a/isort/output.py b/isort/output.py index ce687ff27..cf6e49684 100644 --- a/isort/output.py +++ b/isort/output.py @@ -1,7 +1,7 @@ import copy import itertools from functools import partial -from typing import Iterable, List, Tuple +from typing import Dict, Iterable, List, Tuple from isort.format import format_simplified @@ -57,9 +57,6 @@ def sorted_imports( from_modules, key=lambda key: sorting.module_key(key, config, section_name=section) ) - if config.force_sort_within_sections: - copied_comments = copy.deepcopy(parsed.categorized_comments) - section_output: List[str] = [] if config.from_first: section_output = _with_from_imports( @@ -107,11 +104,20 @@ def sorted_imports( ) if config.force_sort_within_sections: - # Remove comments - section_output = [line for line in section_output if not line.startswith("#")] + # collapse comments + comments_above = [] + new_section_output: List[str] = [] + for line in section_output: + if line.startswith("#"): + comments_above.append(line) + elif comments_above: + new_section_output.append(_LineWithComments(line, comments_above)) + comments_above = [] + else: + new_section_output.append(line) - section_output = sorting.naturally( - section_output, + new_section_output = sorting.naturally( + new_section_output, key=partial( sorting.section_key, order_by_type=config.order_by_type, @@ -121,19 +127,11 @@ def sorted_imports( ), ) - # Add comments back - all_comments = copied_comments["above"]["from"] - all_comments.update(copied_comments["above"]["straight"]) - comment_indexes = {} - for module, comment_list in all_comments.items(): - for idx, line in enumerate(section_output): - if module in line: - comment_indexes[idx] = comment_list - added = 0 - for idx, comment_list in comment_indexes.items(): - for comment in comment_list: - section_output.insert(idx + added, comment) - added += 1 + # uncollapse comments + section_output = [] + for line in new_section_output: + section_output.extend(getattr(line, "comments", ())) + section_output.append(str(line)) section_name = section no_lines_before = section_name in config.no_lines_before @@ -535,3 +533,9 @@ def _normalize_empty_lines(lines: List[str]) -> List[str]: lines.append("") return lines + + +class _LineWithComments(str): + def __new__(cls, value, comments): + cls.comments = comments + return super().__new__(cls, value) # type: ignore diff --git a/scripts/build_config_option_docs.py b/scripts/build_config_option_docs.py index a1d43f43a..927c858b4 100755 --- a/scripts/build_config_option_docs.py +++ b/scripts/build_config_option_docs.py @@ -170,7 +170,7 @@ def config_options() -> Generator[ConfigOption, None, None]: extra_kwargs["description"] = cli.help yield ConfigOption( - name=name, default=cli.default, cli_options=cli.option_strings, **extra_kwargs, + name=name, default=cli.default, cli_options=cli.option_strings, **extra_kwargs ) diff --git a/tests/test_isort.py b/tests/test_isort.py index 4ef2a0a92..94fea3d70 100644 --- a/tests/test_isort.py +++ b/tests/test_isort.py @@ -4087,25 +4087,6 @@ def test_isort_ensures_blank_line_between_import_and_comment() -> None: assert api.sort_code_string(test_input, **config) == expected_output -def test_moving_comments_issue_726(): - config = {"force_sort_within_sections": 1} # type: Dict[str, Any] - test_input = ( - "from Blue import models as BlueModels\n" - "# comment for PlaidModel\n" - "from Plaid.models import PlaidModel\n" - ) - assert api.sort_code_string(test_input, **config) == test_input - - test_input = ( - "# comment for BlueModels\n" - "from Blue import models as BlueModels\n" - "# comment for PlaidModel\n" - "# another comment for PlaidModel\n" - "from Plaid.models import PlaidModel\n" - ) - assert api.sort_code_string(test_input, **config) == test_input - - def test_pyi_formatting_issue_942(tmpdir) -> None: test_input = "import os\n\n\ndef my_method():\n" expected_py_output = test_input.splitlines() diff --git a/tests/test_regressions.py b/tests/test_regressions.py new file mode 100644 index 000000000..eb141663e --- /dev/null +++ b/tests/test_regressions.py @@ -0,0 +1,38 @@ +"""A growing set of tests designed to ensure isort doesn't have regressions in new versions""" +import isort + + +def test_isort_duplicating_comments_issue_1264(): + """Ensure isort doesn't duplicate comments when force_sort_within_sections is set to `True` + as was the case in issue #1264: https://github.com/timothycrosley/isort/issues/1264 + """ + assert ( + isort.code( + """ +from homeassistant.util.logging import catch_log_exception + +# Loading the config flow... +from . import config_flow +""", + force_sort_within_sections=True, + ).count("# Loading the config flow...") + == 1 + ) + + +def test_moving_comments_issue_726(): + test_input = ( + "from Blue import models as BlueModels\n" + "# comment for PlaidModel\n" + "from Plaid.models import PlaidModel\n" + ) + assert isort.code(test_input, force_sort_within_sections=True) == test_input + + test_input = ( + "# comment for BlueModels\n" + "from Blue import models as BlueModels\n" + "# comment for PlaidModel\n" + "# another comment for PlaidModel\n" + "from Plaid.models import PlaidModel\n" + ) + assert isort.code(test_input, force_sort_within_sections=True) == test_input