Merge pull request #934 from mkurnikov/extract-skipping-logic

Extract file-related logic from _SortImports

3 files changed, 226 insertions, 220 deletions

diff --git a/isort/compat.py b/isort/compat.py
index e24f84de..6ea27d90 100644
--- a/isort/compat.py
+++ b/isort/compat.py
@@ -1,11 +1,61 @@
+import locale
 import os
-from typing import Any, Optional
+import re
+import sys
+from typing import Any, Optional, Tuple
 
 from isort import settings
+from isort.format import ask_whether_to_apply_changes_to_file, show_unified_diff
 from isort.isort import _SortImports
 
 
+def determine_file_encoding(fname: str, default: str = 'utf-8') -> str:
+    # see https://www.python.org/dev/peps/pep-0263/
+    pattern = re.compile(br'coding[:=]\s*([-\w.]+)')
+
+    coding = default
+    with open(fname, 'rb') as f:
+        for line_number, line in enumerate(f, 1):
+            groups = re.findall(pattern, line)
+            if groups:
+                coding = groups[0].decode('ascii')
+                break
+            if line_number > 2:
+                break
+
+    return coding
+
+
+def read_file_contents(file_path: str, encoding: str, fallback_encoding: str) -> Tuple[Optional[str], Optional[str]]:
+    with open(file_path, encoding=encoding, newline='') as file_to_import_sort:
+        try:
+            file_contents = file_to_import_sort.read()
+            return file_contents, encoding
+        except UnicodeDecodeError:
+            pass
+
+    with open(file_path, encoding=fallback_encoding, newline='') as file_to_import_sort:
+        try:
+            file_contents = file_to_import_sort.read()
+            return file_contents, fallback_encoding
+        except UnicodeDecodeError:
+            return None, None
+
+
+def get_settings_path(settings_path: Optional[str], current_file_path: str) -> str:
+    if settings_path:
+        return settings_path
+
+    if current_file_path:
+        return os.path.dirname(os.path.abspath(current_file_path))
+    else:
+        return os.getcwd()
+
+
 class SortImports(object):
+    incorrectly_sorted = False
+    skipped = False
+
     def __init__(
             self,
             file_path: Optional[str] = None,
@@ -19,45 +69,118 @@ class SortImports(object):
             check_skip: bool = True,
             **setting_overrides: Any
     ):
-        _settings_path = settings_path
-        if _settings_path is None:
-            if file_path:
-                _settings_path = os.path.dirname(os.path.abspath(file_path))
+        self.config = settings.prepare_config(get_settings_path(settings_path, file_path),
+                                              **setting_overrides)
+        self.output = None
+
+        file_encoding = 'utf-8'
+        file_name = file_path
+
+        self.file_path = file_path or ""
+        if file_path:
+            file_path = os.path.abspath(file_path)
+            if check_skip:
+                if run_path and file_path.startswith(run_path):
+                    file_name = os.path.relpath(file_path, run_path)
+                else:
+                    file_name = file_path
+                    run_path = ''
+
+                if settings.file_should_be_skipped(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:
+                preferred_encoding = determine_file_encoding(file_path)
+
+                # default encoding for open(mode='r') on the system
+                fallback_encoding = locale.getpreferredencoding(False)
+
+                file_contents, used_encoding = read_file_contents(file_path,
+                                                                  encoding=preferred_encoding,
+                                                                  fallback_encoding=fallback_encoding)
+                if used_encoding is None:
+                    self.skipped = True
+                    if self.config['verbose']:
+                        print("WARNING: {} was skipped as it couldn't be opened with the given "
+                              "{} encoding or {} fallback encoding".format(file_path,
+                                                                           file_encoding,
+                                                                           fallback_encoding))
+                else:
+                    file_encoding = used_encoding
+
+        if file_contents is None or ("isort:" + "skip_file") in file_contents:
+            self.skipped = True
+            if write_to_stdout and file_contents:
+                sys.stdout.write(file_contents)
+            return
+
+        self.sorted_imports = _SortImports(file_contents=file_contents,
+                                           config=self.config)
+        self.output = self.sorted_imports.output
+
+        if self.config['atomic']:
+            try:
+                out_lines_without_top_comment = self.sorted_imports.get_out_lines_without_top_comment()
+                compile(out_lines_without_top_comment, self.file_path, 'exec', 0, 1)
+            except SyntaxError:
+                self.output = file_contents
+                self.incorrectly_sorted = True
+                try:
+                    in_lines_without_top_comment = self.sorted_imports.get_in_lines_without_top_comment()
+                    compile(in_lines_without_top_comment, self.file_path, 'exec', 0, 1)
+                    print("ERROR: {0} isort would have introduced syntax errors, please report to the project!".
+                          format(self.file_path))
+                except SyntaxError:
+                    print("ERROR: {0} File contains syntax errors.".format(self.file_path))
+
+                return
+
+        if check:
+            check_output = self.output
+            check_against = file_contents
+            if self.config['ignore_whitespace']:
+                check_output = check_output.replace(self.sorted_imports.line_separator, "").replace(" ", "").replace("\x0c", "")
+                check_against = check_against.replace(self.sorted_imports.line_separator, "").replace(" ", "").replace("\x0c", "")
+
+            current_input_sorted_correctly = self.sorted_imports.check_if_input_already_sorted(check_output, check_against,
+                                                                                               current_file_path=self.file_path)
+            if current_input_sorted_correctly:
+                return
             else:
-                _settings_path = os.getcwd()
+                self.incorrectly_sorted = True
 
-        config = settings.prepare_config(_settings_path, **setting_overrides)
+        if show_diff or self.config['show_diff']:
+            show_unified_diff(file_input=file_contents, file_output=self.output,
+                              file_path=self.file_path)
 
-        self.sorted_imports = _SortImports(file_path=file_path,
-                                           file_contents=file_contents,
-                                           write_to_stdout=write_to_stdout,
-                                           check=check,
-                                           show_diff=show_diff,
-                                           ask_to_apply=ask_to_apply,
-                                           run_path=run_path,
-                                           check_skip=check_skip,
-                                           config=config)
+        elif write_to_stdout:
+            sys.stdout.write(self.output)
 
-    @property
-    def config(self):
-        return self.sorted_imports.config
+        elif file_name and not check:
+            if self.output == file_contents:
+                return
 
-    @property
-    def sections(self):
-        return self.sorted_imports.sections
+            if ask_to_apply:
+                show_unified_diff(file_input=file_contents, file_output=self.output,
+                                  file_path=self.file_path)
+                apply_changes = ask_whether_to_apply_changes_to_file(self.file_path)
+                if not apply_changes:
+                    return
 
-    @property
-    def incorrectly_sorted(self):
-        return self.sorted_imports.incorrectly_sorted
+            with open(self.file_path, 'w', encoding=file_encoding, newline='') as output_file:
+                if not self.config['quiet']:
+                    print("Fixing {0}".format(self.file_path))
+
+                output_file.write(self.output)
 
     @property
-    def skipped(self) -> bool:
-        return self.sorted_imports.skipped
+    def sections(self):
+        return self.sorted_imports.sections
 
     @property
     def length_change(self) -> int:
         return self.sorted_imports.length_change
-
-    @property
-    def output(self):
-        return self.sorted_imports.output
diff --git a/isort/format.py b/isort/format.py
new file mode 100644
index 00000000..9aa81bea
--- /dev/null
+++ b/isort/format.py
@@ -0,0 +1,52 @@
+import os
+import sys
+from datetime import datetime
+from difflib import unified_diff
+
+
+def format_simplified(import_line: str) -> str:
+    import_line = import_line.strip()
+    if import_line.startswith("from "):
+        import_line = import_line.replace("from ", "")
+        import_line = import_line.replace(" import ", ".")
+    elif import_line.startswith("import "):
+        import_line = import_line.replace("import ", "")
+
+    return import_line
+
+
+def format_natural(import_line: str) -> str:
+    import_line = import_line.strip()
+    if not import_line.startswith("from ") and not import_line.startswith("import "):
+        if "." not in import_line:
+            return "import {0}".format(import_line)
+        parts = import_line.split(".")
+        end = parts.pop(-1)
+        return "from {0} import {1}".format(".".join(parts), end)
+
+    return import_line
+
+
+def show_unified_diff(*, file_input: str, file_output: str, file_path: str) -> None:
+    unified_diff_lines = unified_diff(
+        file_input.splitlines(keepends=True),
+        file_output.splitlines(keepends=True),
+        fromfile=file_path + ':before',
+        tofile=file_path + ':after',
+        fromfiledate=str(datetime.fromtimestamp(os.path.getmtime(file_path))
+                         if file_path else datetime.now()),
+        tofiledate=str(datetime.now())
+    )
+    for line in unified_diff_lines:
+        sys.stdout.write(line)
+
+
+def ask_whether_to_apply_changes_to_file(file_path: str) -> bool:
+    answer = None
+    while answer not in ('yes', 'y', 'no', 'n', 'quit', 'q'):
+        answer = input("Apply suggested changes to '{0}' [y/n/q]? ".format(file_path)).lower()
+        if answer in ('no', 'n'):
+            return False
+        if answer in ('quit', 'q'):
+            sys.exit(1)
+    return True
diff --git a/isort/isort.py b/isort/isort.py
index 370c6b72..a240a015 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -26,16 +26,12 @@ OTHER DEALINGS IN THE SOFTWARE.
 """
 import copy
 import itertools
-import locale
-import os
 import re
-import sys
 from collections import OrderedDict, namedtuple
-from datetime import datetime
-from difflib import unified_diff
 from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple
 
 from isort import utils
+from isort.format import format_natural, format_simplified
 
 from . import settings
 from .finders import FindersManager
@@ -58,74 +54,16 @@ if TYPE_CHECKING:
 
 
 class _SortImports(object):
-    incorrectly_sorted = False
-    skipped = False
-
-    def __init__(
-            self, *,
-            config: Dict[str, Any],
-            file_path: Optional[str] = None,
-            file_contents: Optional[str] = None,
-            write_to_stdout: bool = False,
-            check: bool = False,
-            show_diff: bool = False,
-            ask_to_apply: bool = False,
-            run_path: str = '',
-            check_skip: bool = True
-    ) -> None:
+    def __init__(self, file_contents: str, config: Dict[str, Any]) -> None:
         self.config = config
 
         self.place_imports = {}  # type: Dict[str, List[str]]
         self.import_placements = {}  # type: Dict[str, str]
-        self.remove_imports = [self._format_simplified(removal) for removal in self.config['remove_imports']]
-        self.add_imports = [self._format_natural(addition) for addition in self.config['add_imports']]
+        self.remove_imports = [format_simplified(removal) for removal in self.config['remove_imports']]
+        self.add_imports = [format_natural(addition) for addition in self.config['add_imports']]
         self._section_comments = ["# " + value for key, value in self.config.items()
                                   if key.startswith('import_heading') and value]
 
-        self.file_encoding = 'utf-8'
-        file_name = file_path
-        self.file_path = file_path or ""
-        if file_path:
-            file_path = os.path.abspath(file_path)
-            if check_skip:
-                if run_path and file_path.startswith(run_path):
-                    file_name = os.path.relpath(file_path, run_path)
-                else:
-                    file_name = file_path
-                    run_path = ''
-
-                if settings.file_should_be_skipped(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:
-                preferred_encoding = determine_file_encoding(file_path)
-                # default encoding for open(mode='r') on the system
-                fallback_encoding = locale.getpreferredencoding(False)
-
-                file_contents, used_encoding = self.read_file_contents(file_path,
-                                                                       encoding=preferred_encoding,
-                                                                       fallback_encoding=fallback_encoding)
-                if used_encoding is None:
-                    self.skipped = True
-                    if self.config['verbose']:
-                        print("WARNING: {} was skipped as it couldn't be opened with the given "
-                              "{} encoding or {} fallback encoding".format(file_path,
-                                                                           self.file_encoding,
-                                                                           fallback_encoding))
-                else:
-                    self.file_encoding = used_encoding
-
-        if file_contents is None or ("isort:" + "skip_file") in file_contents:
-            self.skipped = True
-            self.output = None
-            if write_to_stdout and file_contents:
-                sys.stdout.write(file_contents)
-            return
-
         self.line_separator = self.determine_line_separator(file_contents)
 
         self.in_lines = file_contents.split(self.line_separator)
@@ -159,58 +97,22 @@ class _SortImports(object):
             self.out_lines.pop(-1)
         self.out_lines.append("")
         self.output = self.line_separator.join(self.out_lines)
-        if self.config['atomic']:
-            try:
-                out_lines_without_top_comment = self._strip_top_comments(self.out_lines, self.line_separator)
-                compile(out_lines_without_top_comment, self.file_path, 'exec', 0, 1)
-            except SyntaxError:
-                self.output = file_contents
-                self.incorrectly_sorted = True
-                try:
-                    in_lines_without_top_comment = self._strip_top_comments(self.in_lines, self.line_separator)
-                    compile(in_lines_without_top_comment, self.file_path, 'exec', 0, 1)
-                    print("ERROR: {0} isort would have introduced syntax errors, please report to the project!".
-                          format(self.file_path))
-                except SyntaxError:
-                    print("ERROR: {0} File contains syntax errors.".format(self.file_path))
-
-                return
-        if check:
-            check_output = self.output
-            check_against = file_contents
-            if self.config['ignore_whitespace']:
-                check_output = check_output.replace(self.line_separator, "").replace(" ", "").replace("\x0c", "")
-                check_against = check_against.replace(self.line_separator, "").replace(" ", "").replace("\x0c", "")
-
-            if check_output.strip() == check_against.strip():
-                if self.config['verbose']:
-                    print("SUCCESS: {0} Everything Looks Good!".format(self.file_path))
-                return
-
-            print("ERROR: {0} Imports are incorrectly sorted.".format(self.file_path))
-            self.incorrectly_sorted = True
-        if show_diff or self.config['show_diff']:
-            self._show_diff(file_contents)
-        elif write_to_stdout:
-            sys.stdout.write(self.output)
-        elif file_name and not check:
-            if self.output == file_contents:
-                return
-
-            if ask_to_apply:
-                self._show_diff(file_contents)
-                answer = None
-                while answer not in ('yes', 'y', 'no', 'n', 'quit', 'q'):
-                    answer = input("Apply suggested changes to '{0}' [y/n/q]? ".format(self.file_path)).lower()
-                    if answer in ('no', 'n'):
-                        return
-                    if answer in ('quit', 'q'):
-                        sys.exit(1)
-
-            with open(self.file_path, 'w', encoding=self.file_encoding, newline='') as output_file:
-                if not self.config['quiet']:
-                    print("Fixing {0}".format(self.file_path))
-                output_file.write(self.output)
+
+    def get_out_lines_without_top_comment(self) -> str:
+        return self._strip_top_comments(self.out_lines, self.line_separator)
+
+    def get_in_lines_without_top_comment(self) -> str:
+        return self._strip_top_comments(self.in_lines, self.line_separator)
+
+    def check_if_input_already_sorted(self, output: str, check_against: str,
+                                      *, current_file_path) -> bool:
+        if output.strip() == check_against.strip():
+            if self.config['verbose']:
+                print("SUCCESS: {0} Everything Looks Good!".format(current_file_path))
+            return True
+
+        print("ERROR: {0} Imports are incorrectly sorted.".format(current_file_path))
+        return False
 
     def determine_line_separator(self, file_contents: str) -> str:
         if self.config['line_ending']:
@@ -218,37 +120,6 @@ class _SortImports(object):
         else:
             return utils.infer_line_separator(file_contents)
 
-    def read_file_contents(self, file_path: str, encoding: str, fallback_encoding: str) -> Tuple[Optional[str], Optional[str]]:
-        with open(file_path, encoding=encoding, newline='') as file_to_import_sort:
-            try:
-                file_contents = file_to_import_sort.read()
-                return file_contents, encoding
-            except UnicodeDecodeError:
-                pass
-
-        with open(file_path, encoding=fallback_encoding, newline='') as file_to_import_sort:
-            try:
-                file_contents = file_to_import_sort.read()
-                return file_contents, fallback_encoding
-            except UnicodeDecodeError:
-                return None, None
-
-    @property
-    def correctly_sorted(self) -> bool:
-        return not self.incorrectly_sorted
-
-    def _show_diff(self, file_contents: str) -> None:
-        for line in unified_diff(
-            file_contents.splitlines(keepends=True),
-            self.output.splitlines(keepends=True),
-            fromfile=self.file_path + ':before',
-            tofile=self.file_path + ':after',
-            fromfiledate=str(datetime.fromtimestamp(os.path.getmtime(self.file_path))
-                             if self.file_path else datetime.now()),
-            tofiledate=str(datetime.now())
-        ):
-            sys.stdout.write(line)
-
     @staticmethod
     def _strip_top_comments(lines: Sequence[str], line_separator: str) -> str:
         """Strips # comments that exist at the top of the given lines"""
@@ -864,29 +735,6 @@ class _SortImports(object):
 
         return line, comments, new_comments
 
-    @staticmethod
-    def _format_simplified(import_line: str) -> str:
-        import_line = import_line.strip()
-        if import_line.startswith("from "):
-            import_line = import_line.replace("from ", "")
-            import_line = import_line.replace(" import ", ".")
-        elif import_line.startswith("import "):
-            import_line = import_line.replace("import ", "")
-
-        return import_line
-
-    @staticmethod
-    def _format_natural(import_line: str) -> str:
-        import_line = import_line.strip()
-        if not import_line.startswith("from ") and not import_line.startswith("import "):
-            if "." not in import_line:
-                return "import {0}".format(import_line)
-            parts = import_line.split(".")
-            end = parts.pop(-1)
-            return "from {0} import {1}".format(".".join(parts), end)
-
-        return import_line
-
     def _skip_line(self, line: str) -> bool:
         skip_line = self._in_quote
         if self.index == 1 and line.startswith("#"):
@@ -1096,20 +944,3 @@ class _SortImports(object):
                                 " Do you need to define a default section?".format(import_from, line)
                             )
                         self.imports[placed_module][import_type][module] = None
-
-
-def determine_file_encoding(fname: str, default: str = 'utf-8') -> str:
-    # see https://www.python.org/dev/peps/pep-0263/
-    pattern = re.compile(br'coding[:=]\s*([-\w.]+)')
-
-    coding = default
-    with open(fname, 'rb') as f:
-        for line_number, line in enumerate(f, 1):
-            groups = re.findall(pattern, line)
-            if groups:
-                coding = groups[0].decode('ascii')
-                break
-            if line_number > 2:
-                break
-
-    return coding