Merge branch 'develop' into Handle-EXT_SUFFIX-on-third-party-packages

6 files changed, 174 insertions, 39 deletions

diff --git a/README.rst b/README.rst
index cd55df31..6c4db34e 100644
--- a/README.rst
+++ b/README.rst
@@ -463,6 +463,12 @@ This will result in the following output style:
         UnexpectedCodePath,
     )
 
+It is also possible to opt-in to sorting imports by length for only specific
+sections by using ``length_sort_`` followed by the section name as a
+configuration item, e.g.::
+
+    length_sort_stdlib=1
+
 Skip processing of imports (outside of configuration)
 =====================================================
 
diff --git a/isort/isort.py b/isort/isort.py
index dfbf9afe..9c7ed8cf 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -254,7 +254,7 @@ class SortImports(object):
         return self.index == self.number_of_lines
 
     @staticmethod
-    def _module_key(module_name, config, sub_imports=False, ignore_case=False):
+    def _module_key(module_name, config, sub_imports=False, ignore_case=False, section_name=None):
         prefix = ""
         if ignore_case:
             module_name = str(module_name).lower()
@@ -269,8 +269,12 @@ class SortImports(object):
             else:
                 prefix = "C"
         module_name = module_name.lower()
+        if section_name is None or 'length_sort_' + str(section_name).lower() not in config:
+            length_sort = config['length_sort']
+        else:
+            length_sort = config['length_sort_' + str(section_name).lower()]
         return "{0}{1}{2}".format(module_name in config['force_to_top'] and "A" or "B", prefix,
-                                  config['length_sort'] and (str(len(module_name)) + ":" + module_name) or module_name)
+                                  length_sort and (str(len(module_name)) + ":" + module_name) or module_name)
 
     def _add_comments(self, comments, original_string=""):
         """
@@ -350,7 +354,7 @@ class SortImports(object):
             import_start = "from {0} import ".format(module)
             from_imports = list(self.imports[section]['from'][module])
             if not self.config['no_inline_sort'] or self.config['force_single_line']:
-                from_imports = nsorted(from_imports, key=lambda key: self._module_key(key, self.config, True, ignore_case))
+                from_imports = nsorted(from_imports, key=lambda key: self._module_key(key, self.config, True, ignore_case, section_name=section))
             if self.remove_imports:
                 from_imports = [line for line in from_imports if not "{0}.{1}".format(module, line) in
                                 self.remove_imports]
@@ -508,9 +512,9 @@ class SortImports(object):
         prev_section_has_imports = False
         for section in sections:
             straight_modules = self.imports[section]['straight']
-            straight_modules = nsorted(straight_modules, key=lambda key: self._module_key(key, self.config))
+            straight_modules = nsorted(straight_modules, key=lambda key: self._module_key(key, self.config, section_name=section))
             from_modules = self.imports[section]['from']
-            from_modules = nsorted(from_modules, key=lambda key: self._module_key(key, self.config))
+            from_modules = nsorted(from_modules, key=lambda key: self._module_key(key, self.config, section_name=section))
 
             section_output = []
             if self.config['from_first']:
diff --git a/isort/main.py b/isort/main.py
index 9356c6a3..a584c226 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -25,7 +25,6 @@ import glob
 import os
 import re
 import sys
-from concurrent.futures import ProcessPoolExecutor
 
 import setuptools
 
@@ -169,10 +168,10 @@ class ISortCommand(setuptools.Command):
                 except IOError as e:
                     print("WARNING: Unable to parse file {0} due to {1}".format(python_file, e))
         if wrong_sorted_files:
-            exit(1)
+            sys.exit(1)
 
 
-def create_parser():
+def parse_args(argv=None):
     parser = argparse.ArgumentParser(description='Sort Python import definitions alphabetically '
                                                  'within logical sections.')
     inline_args_group = parser.add_mutually_exclusive_group()
@@ -285,16 +284,21 @@ def create_parser():
                         help='Tells isort to ignore whitespace differences when --check-only is being used.')
     parser.add_argument('-y', '--apply', dest='apply', action='store_true',
                         help='Tells isort to apply changes recursively without asking')
+    parser.add_argument('--unsafe', dest='unsafe', action='store_true',
+                        help='Tells isort to look for files in standard library directories, etc. '
+                             'where it may not be safe to operate in')
     parser.add_argument('files', nargs='*', help='One or more Python source files that need their imports sorted.')
 
-    arguments = {key: value for key, value in vars(parser.parse_args()).items() if value}
+    arguments = {key: value for key, value in vars(parser.parse_args(argv)).items() if value}
     if 'dont_order_by_type' in arguments:
         arguments['order_by_type'] = False
+    if arguments.pop('unsafe', False):
+        arguments['safety_excludes'] = False
     return arguments
 
 
-def main():
-    arguments = create_parser()
+def main(argv=None):
+    arguments = parse_args(argv)
     if arguments.get('show_version'):
         print(INTRO)
         return
@@ -329,31 +333,26 @@ def main():
         num_skipped = 0
         if config['verbose'] or config.get('show_logo', False):
             print(INTRO)
+
         jobs = arguments.get('jobs')
         if jobs:
-            executor = ProcessPoolExecutor(max_workers=jobs)
-
-            for sort_attempt in executor.map(functools.partial(sort_imports, **arguments), file_names):
-                if not sort_attempt:
-                    continue
-                incorrectly_sorted = sort_attempt.incorrectly_sorted
-                if arguments.get('check', False) and incorrectly_sorted:
-                    wrong_sorted_files = True
-                if sort_attempt.skipped:
-                    num_skipped += 1
+            import multiprocessing
+            executor = multiprocessing.Pool(jobs)
+            attempt_iterator = executor.imap(functools.partial(sort_imports, **arguments), file_names)
         else:
-            for file_name in file_names:
-                try:
-                    sort_attempt = SortImports(file_name, **arguments)
-                    incorrectly_sorted = sort_attempt.incorrectly_sorted
-                    if arguments.get('check', False) and incorrectly_sorted:
-                        wrong_sorted_files = True
-                    if sort_attempt.skipped:
-                        num_skipped += 1
-                except IOError as e:
-                    print("WARNING: Unable to parse file {0} due to {1}".format(file_name, e))
+            attempt_iterator = (sort_imports(file_name, **arguments) for file_name in file_names)
+
+        for sort_attempt in attempt_iterator:
+            if not sort_attempt:
+                continue
+            incorrectly_sorted = sort_attempt.incorrectly_sorted
+            if arguments.get('check', False) and incorrectly_sorted:
+                wrong_sorted_files = True
+            if sort_attempt.skipped:
+                num_skipped += 1
+
         if wrong_sorted_files:
-            exit(1)
+            sys.exit(1)
 
         num_skipped += len(skipped)
         if num_skipped and not arguments.get('quiet', False):
diff --git a/isort/settings.py b/isort/settings.py
index 79ae6c4c..8d6be959 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -27,6 +27,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 import fnmatch
 import io
 import os
+import re
 import posixpath
 import sys
 import warnings
@@ -34,6 +35,7 @@ from collections import namedtuple
 from distutils.util import strtobool
 
 from .pie_slice import lru_cache
+from .utils import difference, union
 
 try:
     import configparser
@@ -48,6 +50,10 @@ except ImportError:
 MAX_CONFIG_SEARCH_DEPTH = 25  # The number of parent directories isort will look for a config file within
 DEFAULT_SECTIONS = ('FUTURE', 'STDLIB', 'THIRDPARTY', 'FIRSTPARTY', 'LOCALFOLDER')
 
+safety_exclude_re = re.compile(
+    r"/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|_build|buck-out|build|dist|lib/python[0-9].[0-9]+)/"
+)
+
 WrapModes = ('GRID', 'VERTICAL', 'HANGING_INDENT', 'VERTICAL_HANGING_INDENT', 'VERTICAL_GRID', 'VERTICAL_GRID_GROUPED',
              'VERTICAL_GRID_GROUPED_NO_COMMA', 'NOQA')
 WrapModes = namedtuple('WrapModes', WrapModes)(*range(len(WrapModes)))
@@ -145,7 +151,9 @@ default = {'force_to_top': [],
            'show_diff': False,
            'ignore_whitespace': False,
            'no_lines_before': [],
-           'no_inline_sort': False}
+           'no_inline_sort': False,
+           'safety_excludes': True,
+           }
 
 
 @lru_cache()
@@ -210,11 +218,11 @@ def _update_with_config_file(file_path, sections, computed_settings):
             else:
                 existing_data = set(computed_settings.get(access_key, default.get(access_key)))
                 if key.startswith('not_'):
-                    computed_settings[access_key] = list(existing_data.difference(_as_list(value)))
+                    computed_settings[access_key] = difference(existing_data, _as_list(value))
                 elif key.startswith('known_'):
-                    computed_settings[access_key] = list(existing_data.union(_abspaths(cwd, _as_list(value))))
+                    computed_settings[access_key] = union(existing_data, _abspaths(cwd, _as_list(value)))
                 else:
-                    computed_settings[access_key] = list(existing_data.union(_as_list(value)))
+                    computed_settings[access_key] = union(existing_data, _as_list(value))
         elif existing_value_type == bool:
             # Only some configuration formats support native boolean values.
             if not isinstance(value, bool):
@@ -292,8 +300,13 @@ def _get_config_data(file_path, sections):
 
 def should_skip(filename, config, path='/'):
     """Returns True if the file should be skipped based on the passed in settings."""
+    normalized_path = posixpath.join(path.replace('\\', '/'), filename)
+
+    if config['safety_excludes'] and safety_exclude_re.search(normalized_path):
+        return True
+
     for skip_path in config['skip']:
-        if posixpath.abspath(posixpath.join(path.replace('\\', '/'), filename)) == posixpath.abspath(skip_path.replace('\\', '/')):
+        if posixpath.abspath(normalized_path) == posixpath.abspath(skip_path.replace('\\', '/')):
             return True
 
     position = os.path.split(filename)
diff --git a/isort/utils.py b/isort/utils.py
index 9206c88e..ce4e588e 100644
--- a/isort/utils.py
+++ b/isort/utils.py
@@ -28,3 +28,26 @@ def chdir(path):
         yield
     finally:
         os.chdir(curdir)
+
+
+def union(a, b):
+    """ Return a list of items that are in `a` or `b`
+    """
+    u = []
+    for item in a:
+        if item not in u:
+            u.append(item)
+    for item in b:
+        if item not in u:
+            u.append(item)
+    return u
+
+
+def difference(a, b):
+    """ Return a list of items from `a` that are not in `b`.
+    """
+    d = []
+    for item in a:
+        if item not in b:
+            d.append(item)
+    return d
diff --git a/test_isort.py b/test_isort.py
index a21c38d4..cc829d27 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -31,7 +31,7 @@ import sysconfig
 
 import pytest
 
-from isort import finders
+from isort import finders, main, settings
 from isort.isort import SortImports
 from isort.utils import exists_case_sensitive
 from isort.main import is_python_file
@@ -406,6 +406,24 @@ def test_length_sort():
                            "import looooooooooooooooooooooooooooooooooooooong\n")
 
 
+def test_length_sort_section():
+    """Test setting isort to sort on length instead of alphabetically for a specific section."""
+    test_input = ("import medium_sizeeeeeeeeeeeeee\n"
+                  "import shortie\n"
+                  "import sys\n"
+                  "import os\n"
+                  "import looooooooooooooooooooooooooooooooooooooong\n"
+                  "import medium_sizeeeeeeeeeeeeea\n")
+    test_output = SortImports(file_contents=test_input, length_sort_stdlib=True).output
+    assert test_output == ("import os\n"
+                           "import sys\n"
+                           "\n"
+                           "import looooooooooooooooooooooooooooooooooooooong\n"
+                           "import medium_sizeeeeeeeeeeeeea\n"
+                           "import medium_sizeeeeeeeeeeeeee\n"
+                           "import shortie\n")
+
+
 def test_convert_hanging():
     """Ensure that isort will convert hanging indents to correct indent
     method."""
@@ -2512,6 +2530,8 @@ def test_new_lines_are_preserved():
     assert n_newline_contents == 'import os\nimport sys\n'
 
 
+@pytest.mark.skipif(not finders.RequirementsFinder.enabled, reason='RequirementsFinder not enabled (too old version of pip?)')
+@pytest.mark.skipif(not finders.pipreqs, reason='pipreqs is missing')
 def test_requirements_finder(tmpdir):
     subdir = tmpdir.mkdir('subdir').join("lol.txt")
     subdir.write("flask")
@@ -2546,6 +2566,31 @@ def test_requirements_finder(tmpdir):
     req_file.remove()
 
 
+def test_forced_separate_is_deterministic_issue_774(tmpdir):
+
+    config_file = tmpdir.join('setup.cfg')
+    config_file.write(
+        "[isort]\n"
+        "forced_separate:\n"
+        "   separate1\n"
+        "   separate2\n"
+        "   separate3\n"
+        "   separate4\n"
+    )
+
+    test_input = ('import time\n'
+                  '\n'
+                  'from separate1 import foo\n'
+                  '\n'
+                  'from separate2 import bar\n'
+                  '\n'
+                  'from separate3 import baz\n'
+                  '\n'
+                  'from separate4 import quux\n')
+
+    assert SortImports(file_contents=test_input, settings_path=config_file.strpath).output == test_input
+
+
 PIPFILE = """
 [[source]]
 url = "https://pypi.org/simple"
@@ -2563,6 +2608,8 @@ deal = {editable = true, git = "https://github.com/orsinium/deal.git"}
 """
 
 
+@pytest.mark.skipif(not finders.PipfileFinder.enabled, reason='PipfileFinder not enabled (missing requirementslib?)')
+@pytest.mark.skipif(not finders.pipreqs, reason='pipreqs is missing')
 def test_pipfile_finder(tmpdir):
     pipfile = tmpdir.join('Pipfile')
     pipfile.write(PIPFILE)
@@ -2617,4 +2664,47 @@ def test_path_finder(monkeypatch):
     assert finder.find("example_2") == finder.sections.THIRDPARTY
     assert finder.find("example_3") == finder.sections.THIRDPARTY
     assert finder.find("example_4") == finder.sections.THIRDPARTY
-    assert finder.find("example_5") == finder.sections.FIRSTPARTY
\ No newline at end of file
+    assert finder.find("example_5") == finder.sections.FIRSTPARTY
+
+
+def test_argument_parsing():
+    from isort.main import parse_args
+    args = parse_args(['-dt', '-t', 'foo', '--skip=bar', 'baz.py'])
+    assert args['order_by_type'] is False
+    assert args['force_to_top'] == ['foo']
+    assert args['skip'] == ['bar']
+    assert args['files'] == ['baz.py']
+
+
+@pytest.mark.parametrize('multiprocess', (False, True))
+def test_command_line(tmpdir, capfd, multiprocess):
+    from isort.main import main
+    tmpdir.join("file1.py").write("import re\nimport os\n\nimport contextlib\n\n\nimport isort")
+    tmpdir.join("file2.py").write("import collections\nimport time\n\nimport abc\n\n\nimport isort")
+    arguments = ["-rc", str(tmpdir)]
+    if multiprocess:
+        arguments.extend(['--jobs', '2'])
+    main(arguments)
+    assert tmpdir.join("file1.py").read() == "import contextlib\nimport os\nimport re\n\nimport isort\n"
+    assert tmpdir.join("file2.py").read() == "import abc\nimport collections\nimport time\n\nimport isort\n"
+    out, err = capfd.readouterr()
+    assert not err
+    # it informs us about fixing the files:
+    assert str(tmpdir.join("file1.py")) in out
+    assert str(tmpdir.join("file2.py")) in out
+
+
+@pytest.mark.parametrize('enabled', (False, True))
+def test_safety_excludes(tmpdir, enabled):
+    tmpdir.join("victim.py").write("# ...")
+    tmpdir.mkdir(".tox").join("verysafe.py").write("# ...")
+    tmpdir.mkdir("lib").mkdir("python3.7").join("importantsystemlibrary.py").write("# ...")
+    config = dict(settings.default.copy(), safety_excludes=enabled)
+    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'}
+        assert len(skipped) == 2
+    else:
+        assert file_names == {'.tox/verysafe.py', 'lib/python3.7/importantsystemlibrary.py', 'victim.py'}
+        assert not skipped
\ No newline at end of file