Repository: timothycrosley/isort (python)

Issue:
isort does not skip files
**Describe the bug**

`isort` does not respect `skip` and `skip_glob` configuration options.

**To Reproduce**

Steps to reproduce the behavior:

1. Create `file.py` with the following content:
    ```python
    import os, \
        sys
    ```
2. Create `.isort.cfg` with the following content:
    ```
    [settings]
    skip_glob = file.py
    ```
3. Run `isort file.py`.
4. Open `file.py` and check out it's content to become:
    ```python
    import os
    import sys
    ```

**Expected behavior**

A `file.py` is skipped and is not touched.

**Screenshots**

—

**Environment (please complete the following information):**

- OS: Linux
- Python version: 3.7.2
- isort version: 4.3.11

**Additional context**

`isort` 4.3.10 is working correctly. Issue can be reproduced with `skip` option as well.

Pull request:
Feature/fix issue 885
Closes #885 

Patch:
diff --git a/isort/isort.py b/isort/isort.py
index 7b054edf..ff3921bf 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -47,7 +47,8 @@ class SortImports(object):
     skipped = False
 
     def __init__(self, file_path=None, file_contents=None, write_to_stdout=False, check=False,
-                 show_diff=False, settings_path=None, ask_to_apply=False, check_skip=True, **setting_overrides):
+                 show_diff=False, settings_path=None, ask_to_apply=False, run_path='', check_skip=True,
+                 **setting_overrides):
         if not settings_path and file_path:
             settings_path = os.path.dirname(os.path.abspath(file_path))
         settings_path = settings_path or os.getcwd()
@@ -93,13 +94,20 @@ class SortImports(object):
         self.file_path = file_path or ""
         if file_path:
             file_path = os.path.abspath(file_path)
-            if check_skip and settings.should_skip(file_path, self.config):
-                self.skipped = True
-                if self.config['verbose']:
-                    print("WARNING: {0} was skipped as it's listed in 'skip' setting"
-                          " or matches a glob in 'skip_glob' setting".format(file_path))
-                file_contents = None
-            elif not file_contents:
+            if check_skip:
+                if run_path and file_path.startswith(run_path):
+                    file_name = file_path.replace(run_path, '',  1)
+                else:
+                    file_name = file_path
+                    run_path = ''
+
+                if settings.should_skip(file_name, self.config, run_path):
+                    self.skipped = True
+                    if self.config['verbose']:
+                        print("WARNING: {0} was skipped as it's listed in 'skip' setting"
+                            " or matches a glob in 'skip_glob' setting".format(file_path))
+                    file_contents = None
+            if not self.skipped and not file_contents:
                 file_encoding = coding_check(file_path)
                 with io.open(file_path, encoding=file_encoding, newline='') as file_to_import_sort:
                     try:
diff --git a/isort/main.py b/isort/main.py
index 29049ac6..299562ed 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -85,7 +85,7 @@ class SortAttempt(object):
 
 def sort_imports(file_name, **arguments):
     try:
-        result = SortImports(file_name, check_skip=False, **arguments)
+        result = SortImports(file_name, **arguments)
         return SortAttempt(result.incorrectly_sorted, result.skipped)
     except IOError as e:
         print("WARNING: Unable to parse file {0} due to {1}".format(file_name, e))
@@ -99,9 +99,7 @@ def iter_source_code(paths, config, skipped):
 
     for path in paths:
         if os.path.isdir(path):
-            for dirpath, dirnames, filenames in os.walk(
-                    path, topdown=True, followlinks=True
-            ):
+            for dirpath, dirnames, filenames in os.walk(path, topdown=True, followlinks=True):
                 for dirname in list(dirnames):
                     if should_skip(dirname, config, dirpath):
                         skipped.append(dirname)
@@ -109,7 +107,8 @@ def iter_source_code(paths, config, skipped):
                 for filename in filenames:
                     filepath = os.path.join(dirpath, filename)
                     if is_python_file(filepath):
-                        if should_skip(filename, config, dirpath):
+                        relative_file = os.path.relpath(filepath, path)
+                        if should_skip(relative_file, config, path):
                             skipped.append(filename)
                         else:
                             yield filepath
@@ -314,6 +313,7 @@ def main(argv=None):
               '-rc for recursive')
         sys.exit(1)
 
+    arguments['check_skip'] = False
     if 'settings_path' in arguments:
         sp = arguments['settings_path']
         arguments['settings_path'] = os.path.abspath(sp) if os.path.isdir(sp) else os.path.dirname(os.path.abspath(sp))
diff --git a/isort/settings.py b/isort/settings.py
index 56ee223c..c796d966 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -325,7 +325,7 @@ def should_skip(filename, config, path=''):
     if normalized_path[1:2] == ':':
         normalized_path = normalized_path[2:]
 
-    if config['safety_excludes']:
+    if path and config['safety_excludes']:
         check_exclude = '/' + filename.replace('\\', '/') + '/'
         if path and os.path.basename(path) in ('lib', ):
             check_exclude = '/' + os.path.basename(path) + check_exclude


Test patch:
diff --git a/test_isort.py b/test_isort.py
index a93ea366..fb095fc7 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -2756,13 +2756,16 @@ def test_quiet(tmpdir, capfd, quiet):
 @pytest.mark.parametrize('enabled', (False, True))
 def test_safety_excludes(tmpdir, enabled):
     tmpdir.join("victim.py").write("# ...")
-    tmpdir.mkdir(".tox").join("verysafe.py").write("# ...")
+    toxdir = tmpdir.mkdir(".tox")
+    toxdir.join("verysafe.py").write("# ...")
     tmpdir.mkdir("lib").mkdir("python3.7").join("importantsystemlibrary.py").write("# ...")
     tmpdir.mkdir(".pants.d").join("pants.py").write("import os")
     config = dict(settings.default.copy(), safety_excludes=enabled)
     skipped = []
     codes = [str(tmpdir)],
     main.iter_source_code(codes, config, skipped)
+
+    # if enabled files within nested unsafe directories should be skipped
     file_names = set(os.path.relpath(f, str(tmpdir)) for f in main.iter_source_code([str(tmpdir)], config, skipped))
     if enabled:
         assert file_names == {'victim.py'}
@@ -2774,6 +2777,24 @@ def test_safety_excludes(tmpdir, enabled):
                               'victim.py'}
         assert not skipped
 
+    # directly pointing to files within unsafe directories shouldn't skip them either way
+    file_names = set(os.path.relpath(f, str(toxdir)) for f in main.iter_source_code([str(toxdir)], config, skipped))
+    assert file_names == {'verysafe.py'}
+
+
+@pytest.mark.parametrize('skip_glob_assert', (([], 0, {os.sep.join(('code', 'file.py'))}), (['**/*.py'], 1, {})))
+def test_skip_glob(tmpdir, skip_glob_assert):
+    skip_glob, skipped_count, file_names = skip_glob_assert
+    base_dir = tmpdir.mkdir('build')
+    code_dir = base_dir.mkdir('code')
+    code_dir.join('file.py').write('import os')
+
+    config = dict(settings.default.copy(), skip_glob=skip_glob)
+    skipped = []
+    file_names = set(os.path.relpath(f, str(base_dir)) for f in main.iter_source_code([str(base_dir)], config, skipped))
+    assert len(skipped) == skipped_count
+    assert file_names == file_names
+
 
 def test_comments_not_removed_issue_576():
     test_input = ('import distutils\n'