From 31c9daa84723e956039990f58d996bf24a6fe64c Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 27 Oct 2021 11:18:06 +0200 Subject: [PATCH 1/4] only skip real comments --- isort/core.py | 2 +- isort/settings.py | 4 ++-- tests/unit/test_isort.py | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/isort/core.py b/isort/core.py index 018c8e0b7..8e12f195b 100644 --- a/isort/core.py +++ b/isort/core.py @@ -151,7 +151,7 @@ def process( line_separator = line[len(line.rstrip()) :].replace(" ", "").replace("\t", "") for file_skip_comment in FILE_SKIP_COMMENTS: - if file_skip_comment in line: + if line.startswith(file_skip_comment): if raise_on_skip: raise FileSkipComment("Passed in content") isort_off = True diff --git a/isort/settings.py b/isort/settings.py index d0060d056..e0c8b98d9 100644 --- a/isort/settings.py +++ b/isort/settings.py @@ -55,8 +55,8 @@ SUPPORTED_EXTENSIONS = frozenset({"py", "pyi", *CYTHON_EXTENSIONS}) BLOCKED_EXTENSIONS = frozenset({"pex"}) FILE_SKIP_COMMENTS: Tuple[str, ...] = ( - "isort:" + "skip_file", - "isort: " + "skip_file", + "# isort:" + "skip_file", + "# isort: " + "skip_file", ) # Concatenated to avoid this file being skipped MAX_CONFIG_SEARCH_DEPTH: int = 25 # The number of parent directories to for a config file within STOP_CONFIG_SEARCH_ON_DIRS: Tuple[str, ...] = (".git", ".hg") diff --git a/tests/unit/test_isort.py b/tests/unit/test_isort.py index b0534142b..3467569f9 100644 --- a/tests/unit/test_isort.py +++ b/tests/unit/test_isort.py @@ -837,6 +837,11 @@ def test_skip_within_file() -> None: with pytest.raises(FileSkipped): isort.code(test_input, known_third_party=["django"]) +def test_skip_comment_is_no_comment() -> None: + """Ensure skipping a whole file works.""" + test_input = "content = \"# isort:skip_file\"" + test_output = isort.code(test_input) + assert test_output == test_input def test_force_to_top() -> None: """Ensure forcing a single import to the top of its category works as expected.""" From f0f4645d49d78650032b330c14e4ca023c9763c6 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Wed, 27 Oct 2021 11:24:30 +0200 Subject: [PATCH 2/4] handle without space after hash --- isort/settings.py | 2 ++ tests/unit/test_isort.py | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/isort/settings.py b/isort/settings.py index e0c8b98d9..0d736d642 100644 --- a/isort/settings.py +++ b/isort/settings.py @@ -57,6 +57,8 @@ FILE_SKIP_COMMENTS: Tuple[str, ...] = ( "# isort:" + "skip_file", "# isort: " + "skip_file", + "#isort:" + "skip_file", + "#isort: " + "skip_file", ) # Concatenated to avoid this file being skipped MAX_CONFIG_SEARCH_DEPTH: int = 25 # The number of parent directories to for a config file within STOP_CONFIG_SEARCH_ON_DIRS: Tuple[str, ...] = (".git", ".hg") diff --git a/tests/unit/test_isort.py b/tests/unit/test_isort.py index 3467569f9..c16a84361 100644 --- a/tests/unit/test_isort.py +++ b/tests/unit/test_isort.py @@ -837,6 +837,12 @@ def test_skip_within_file() -> None: with pytest.raises(FileSkipped): isort.code(test_input, known_third_party=["django"]) +def test_skip_comment_without_space_after_hash() -> None: + """Ensure skipping a whole file works.""" + test_input = "#isort: skip_file\nimport django\nimport myproject\n" + with pytest.raises(FileSkipped): + isort.code(test_input, known_third_party=["django"]) + def test_skip_comment_is_no_comment() -> None: """Ensure skipping a whole file works.""" test_input = "content = \"# isort:skip_file\"" @@ -849,7 +855,6 @@ def test_force_to_top() -> None: test_output = isort.code(test_input, force_to_top=["lib5"]) assert test_output == "import lib5\nimport lib1\nimport lib2\nimport lib6\n" - def test_add_imports() -> None: """Ensures adding imports works as expected.""" test_input = "import lib6\nimport lib2\nimport lib5\nimport lib1\n\n" From 4e3f64bf91b50c9d2923c60853e0701ab04f388f Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 28 Oct 2021 07:20:40 +0200 Subject: [PATCH 3/4] fix linter --- tests/unit/test_isort.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_isort.py b/tests/unit/test_isort.py index c16a84361..05740d292 100644 --- a/tests/unit/test_isort.py +++ b/tests/unit/test_isort.py @@ -837,24 +837,28 @@ def test_skip_within_file() -> None: with pytest.raises(FileSkipped): isort.code(test_input, known_third_party=["django"]) + def test_skip_comment_without_space_after_hash() -> None: """Ensure skipping a whole file works.""" test_input = "#isort: skip_file\nimport django\nimport myproject\n" with pytest.raises(FileSkipped): isort.code(test_input, known_third_party=["django"]) + def test_skip_comment_is_no_comment() -> None: """Ensure skipping a whole file works.""" - test_input = "content = \"# isort:skip_file\"" + test_input = 'content = "# isort:skip_file"' test_output = isort.code(test_input) assert test_output == test_input + def test_force_to_top() -> None: """Ensure forcing a single import to the top of its category works as expected.""" test_input = "import lib6\nimport lib2\nimport lib5\nimport lib1\n" test_output = isort.code(test_input, force_to_top=["lib5"]) assert test_output == "import lib5\nimport lib1\nimport lib2\nimport lib6\n" + def test_add_imports() -> None: """Ensures adding imports works as expected.""" test_input = "import lib6\nimport lib2\nimport lib5\nimport lib1\n\n" From 3c5da48559a4052b47f170c1a3fe3cdd5bd928da Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 28 Oct 2021 13:56:19 +0200 Subject: [PATCH 4/4] use regex to mach file_skip --- isort/core.py | 14 +++++++------- isort/settings.py | 7 +------ tests/unit/test_isort.py | 7 +++++++ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/isort/core.py b/isort/core.py index 8e12f195b..ec565bd5b 100644 --- a/isort/core.py +++ b/isort/core.py @@ -1,6 +1,7 @@ import textwrap from io import StringIO from itertools import chain +from re import match from typing import List, TextIO, Union import isort.literal @@ -9,7 +10,7 @@ from . import output, parse from .exceptions import FileSkipComment from .format import format_natural, remove_whitespace -from .settings import FILE_SKIP_COMMENTS +from .settings import FILE_SKIP_RE CIMPORT_IDENTIFIERS = ("cimport ", "cimport*", "from.cimport") IMPORT_START_IDENTIFIERS = ("from ", "from.import", "import ", "import*") + CIMPORT_IDENTIFIERS @@ -150,12 +151,11 @@ def process( if stripped_line and not line_separator: line_separator = line[len(line.rstrip()) :].replace(" ", "").replace("\t", "") - for file_skip_comment in FILE_SKIP_COMMENTS: - if line.startswith(file_skip_comment): - if raise_on_skip: - raise FileSkipComment("Passed in content") - isort_off = True - skip_file = True + if FILE_SKIP_RE.match(line): + if raise_on_skip: + raise FileSkipComment("Passed in content") + isort_off = True + skip_file = True if not in_quote: if stripped_line == "# isort: off": diff --git a/isort/settings.py b/isort/settings.py index 0d736d642..29e17bcb7 100644 --- a/isort/settings.py +++ b/isort/settings.py @@ -54,12 +54,7 @@ CYTHON_EXTENSIONS = frozenset({"pyx", "pxd"}) SUPPORTED_EXTENSIONS = frozenset({"py", "pyi", *CYTHON_EXTENSIONS}) BLOCKED_EXTENSIONS = frozenset({"pex"}) -FILE_SKIP_COMMENTS: Tuple[str, ...] = ( - "# isort:" + "skip_file", - "# isort: " + "skip_file", - "#isort:" + "skip_file", - "#isort: " + "skip_file", -) # Concatenated to avoid this file being skipped +FILE_SKIP_RE = re.compile(r"^#?\s?isort:\s?skip_file") MAX_CONFIG_SEARCH_DEPTH: int = 25 # The number of parent directories to for a config file within STOP_CONFIG_SEARCH_ON_DIRS: Tuple[str, ...] = (".git", ".hg") VALID_PY_TARGETS: Tuple[str, ...] = tuple( diff --git a/tests/unit/test_isort.py b/tests/unit/test_isort.py index 05740d292..d9b663d11 100644 --- a/tests/unit/test_isort.py +++ b/tests/unit/test_isort.py @@ -845,6 +845,13 @@ def test_skip_comment_without_space_after_hash() -> None: isort.code(test_input, known_third_party=["django"]) +def test_skip_comment_with_multiline_comment() -> None: + """Ensure skipping a whole file works.""" + test_input = '"""some comment\n\nisort: skip_file\nimport django\nimport myproject\n"""' + with pytest.raises(FileSkipped): + isort.code(test_input, known_third_party=["django"]) + + def test_skip_comment_is_no_comment() -> None: """Ensure skipping a whole file works.""" test_input = 'content = "# isort:skip_file"'