Merge pull request #1833 from chrisdecker1201/bug/do_not_skip_string

only skip real comments

Author: timothycrosley

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 file_skip_comment in line:
-                    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":

isort/settings.py

@@ -54,10 +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",
-)  # 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(

tests/unit/test_isort.py

@@ -838,6 +838,27 @@ def test_skip_within_file() -> None:
         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_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"'
+    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"