Merge pull request #352 from mcarton/slow

Fix slowness problems II

tests/rules/test_brew_install.py

@@ -1,6 +1,6 @@
 import pytest
 from thefuck.rules.brew_install import match, get_new_command
-from thefuck.rules.brew_install import brew_formulas
+from thefuck.rules.brew_install import _get_formulas
 from tests.utils import Command
 
 
@@ -20,9 +20,7 @@ def brew_already_installed():
 
 
 def _is_not_okay_to_test():
-    if 'elasticsearch' not in brew_formulas:
-        return True
-    return False
+    return 'elasticsearch' not in _get_formulas()
 
 
 @pytest.mark.skipif(_is_not_okay_to_test(),

tests/rules/test_brew_unknown_command.py

@@ -1,6 +1,6 @@
 import pytest
 from thefuck.rules.brew_unknown_command import match, get_new_command
-from thefuck.rules.brew_unknown_command import brew_commands
+from thefuck.rules.brew_unknown_command import _brew_commands
 from tests.utils import Command
 
 
@@ -16,7 +16,7 @@ def brew_unknown_cmd2():
 
 def test_match(brew_unknown_cmd):
     assert match(Command('brew inst', stderr=brew_unknown_cmd), None)
-    for command in brew_commands:
+    for command in _brew_commands():
         assert not match(Command('brew ' + command), None)
 
 
@@ -24,5 +24,6 @@ def test_get_new_command(brew_unknown_cmd, brew_unknown_cmd2):
     assert get_new_command(Command('brew inst', stderr=brew_unknown_cmd),
                            None) == ['brew list', 'brew install', 'brew uninstall']
 
-    assert get_new_command(Command('brew instaa', stderr=brew_unknown_cmd2),
-                           None) == ['brew install', 'brew uninstall', 'brew list']
+    cmds = get_new_command(Command('brew instaa', stderr=brew_unknown_cmd2), None)
+    assert 'brew install' in cmds
+    assert 'brew uninstall' in cmds

thefuck/corrector.py

@@ -1,15 +1,17 @@
-import sys
+from . import conf, types, logs
 from imp import load_source
 from pathlib import Path
-from . import conf, types, logs
+from thefuck.types import CorrectedCommand, Rule
+import sys
 
 
 def load_rule(rule, settings):
     """Imports rule module and returns it."""
     name = rule.name[:-3]
+    with logs.debug_time(u'Importing rule: {};'.format(name), settings):
         rule_module = load_source(name, str(rule))
         priority = getattr(rule_module, 'priority', conf.DEFAULT_PRIORITY)
-    return types.Rule(name, rule_module.match,
+    return Rule(name, rule_module.match,
                 rule_module.get_new_command,
                 getattr(rule_module, 'enabled_by_default', True),
                 getattr(rule_module, 'side_effect', None),
@@ -55,7 +57,7 @@ def is_rule_match(command, rule, settings):
 def make_corrected_commands(command, rule, settings):
     new_commands = rule.get_new_command(command, settings)
     if not isinstance(new_commands, list):
-        new_commands = [new_commands]
+        new_commands = (new_commands,)
     for n, new_command in enumerate(new_commands):
         yield types.CorrectedCommand(script=new_command,
                                      side_effect=rule.side_effect,

thefuck/rules/brew_install.py

@@ -1,38 +1,38 @@
 import os
 import re
-from subprocess import check_output
-from thefuck.utils import get_closest, replace_argument
+from thefuck.utils import get_closest, replace_argument, which
+from thefuck.specific.brew import get_brew_path_prefix
 
-# Formulars are base on each local system's status
 
-brew_formulas = []
+enabled_by_default = bool(which('brew'))
+
+
+def _get_formulas():
+    # Formulas are based on each local system's status
     try:
-    brew_path_prefix = check_output(['brew', '--prefix'],
-                                    universal_newlines=True).strip()
+        brew_path_prefix = get_brew_path_prefix()
         brew_formula_path = brew_path_prefix + '/Library/Formula'
 
         for file_name in os.listdir(brew_formula_path):
             if file_name.endswith('.rb'):
-            brew_formulas.append(file_name.replace('.rb', ''))
+                yield file_name[:-3]
     except:
         pass
 
 
 def _get_similar_formula(formula_name):
-    return get_closest(formula_name, brew_formulas, 1, 0.85)
+    return get_closest(formula_name, _get_formulas(), 1, 0.85)
 
 
 def match(command, settings):
     is_proper_command = ('brew install' in command.script and
                          'No available formula' in command.stderr)
 
-    has_possible_formulas = False
     if is_proper_command:
         formula = re.findall(r'Error: No available formula for ([a-z]+)',
                              command.stderr)[0]
-        has_possible_formulas = bool(_get_similar_formula(formula))
-
-    return has_possible_formulas
+        return bool(_get_similar_formula(formula))
+    return False
 
 
 def get_new_command(command, settings):

thefuck/rules/brew_unknown_command.py

@@ -1,31 +1,20 @@
 import os
 import re
-import subprocess
 from thefuck.utils import get_closest, replace_command
+from thefuck.specific.brew import get_brew_path_prefix
 
 BREW_CMD_PATH = '/Library/Homebrew/cmd'
 TAP_PATH = '/Library/Taps'
 TAP_CMD_PATH = '/%s/%s/cmd'
 
 
-def _get_brew_path_prefix():
-    """To get brew path"""
-    try:
-        return subprocess.check_output(['brew', '--prefix'],
-                                       universal_newlines=True).strip()
-    except:
-        return None
-
-
 def _get_brew_commands(brew_path_prefix):
     """To get brew default commands on local environment"""
     brew_cmd_path = brew_path_prefix + BREW_CMD_PATH
 
-    commands = [name.replace('.rb', '') for name in os.listdir(brew_cmd_path)
+    return [name[:-3] for name in os.listdir(brew_cmd_path)
             if name.endswith('.rb')]
 
-    return commands
-
 
 def _get_brew_tap_specific_commands(brew_path_prefix):
     """To get tap's specific commands
@@ -51,10 +40,7 @@ def _get_brew_tap_specific_commands(brew_path_prefix):
 
 
 def _is_brew_tap_cmd_naming(name):
-    if name.startswith('brew-') and name.endswith('.rb'):
-        return True
-
-    return False
+    return name.startswith('brew-') and name.endswith('.rb')
 
 
 def _get_directory_names_only(path):
@@ -62,35 +48,33 @@ def _get_directory_names_only(path):
             if os.path.isdir(os.path.join(path, d))]
 
 
-brew_path_prefix = _get_brew_path_prefix()
-
-# Failback commands for testing (Based on Homebrew 0.9.5)
-brew_commands = ['info', 'home', 'options', 'install', 'uninstall',
-                 'search', 'list', 'update', 'upgrade', 'pin', 'unpin',
-                 'doctor', 'create', 'edit']
-
+def _brew_commands():
+    brew_path_prefix = get_brew_path_prefix()
     if brew_path_prefix:
         try:
-        brew_commands = _get_brew_commands(brew_path_prefix) \
+            return _get_brew_commands(brew_path_prefix) \
                  + _get_brew_tap_specific_commands(brew_path_prefix)
         except OSError:
             pass
 
+    # Failback commands for testing (Based on Homebrew 0.9.5)
+    return ['info', 'home', 'options', 'install', 'uninstall',
+            'search', 'list', 'update', 'upgrade', 'pin', 'unpin',
+            'doctor', 'create', 'edit']
+
 
 def match(command, settings):
     is_proper_command = ('brew' in command.script and
                          'Unknown command' in command.stderr)
 
-    has_possible_commands = False
     if is_proper_command:
         broken_cmd = re.findall(r'Error: Unknown command: ([a-z]+)',
                                 command.stderr)[0]
-        has_possible_commands = bool(get_closest(broken_cmd, brew_commands))
-
-    return has_possible_commands
+        return bool(get_closest(broken_cmd, _brew_commands()))
+    return False
 
 
 def get_new_command(command, settings):
     broken_cmd = re.findall(r'Error: Unknown command: ([a-z]+)',
                             command.stderr)[0]
-    return replace_command(command, broken_cmd, brew_commands)
+    return replace_command(command, broken_cmd, _brew_commands())

thefuck/rules/fix_file.py

@@ -35,17 +35,17 @@ patterns = (
 
 # for the sake of readability do not use named groups above
 def _make_pattern(pattern):
-    pattern = pattern.replace('{file}', '(?P<file>[^:\n]+)')
-    pattern = pattern.replace('{line}', '(?P<line>[0-9]+)')
-    pattern = pattern.replace('{col}',  '(?P<col>[0-9]+)')
+    pattern = pattern.replace('{file}', '(?P<file>[^:\n]+)') \
+                     .replace('{line}', '(?P<line>[0-9]+)') \
+                     .replace('{col}',  '(?P<col>[0-9]+)')
     return re.compile(pattern, re.MULTILINE)
-patterns = [_make_pattern(p) for p in patterns]
+patterns = [_make_pattern(p).search for p in patterns]
 
 
 @memoize
 def _search(stderr):
     for pattern in patterns:
-        m = re.search(pattern, stderr)
+        m = pattern(stderr)
         if m and os.path.isfile(m.group('file')):
             return m
 

thefuck/rules/git_add.py

@@ -1,5 +1,5 @@
 import re
-from thefuck import utils, shells
+from thefuck import shells
 from thefuck.specific.git import git_support
 
 

thefuck/rules/git_branch_delete.py

@@ -1,4 +1,3 @@
-from thefuck import utils
 from thefuck.utils import replace_argument
 from thefuck.specific.git import git_support
 

thefuck/rules/git_branch_list.py

@@ -1,4 +1,4 @@
-from thefuck import utils, shells
+from thefuck import shells
 from thefuck.specific.git import git_support
 
 

thefuck/rules/git_diff_staged.py

@@ -1,4 +1,3 @@
-from thefuck import utils
 from thefuck.utils import replace_argument
 from thefuck.specific.git import git_support
 

thefuck/rules/git_pull.py

@@ -1,4 +1,4 @@
-from thefuck import shells, utils
+from thefuck import shells
 from thefuck.specific.git import git_support
 
 

thefuck/rules/git_pull_clone.py

@@ -1,4 +1,3 @@
-from thefuck import utils
 from thefuck.utils import replace_argument
 from thefuck.specific.git import git_support
 

thefuck/rules/git_push.py

@@ -1,4 +1,3 @@
-from thefuck import utils
 from thefuck.specific.git import git_support
 
 

thefuck/rules/git_push_pull.py

@@ -1,4 +1,4 @@
-from thefuck import utils, shells
+from thefuck import shells
 from thefuck.utils import replace_argument
 from thefuck.specific.git import git_support
 

thefuck/rules/git_stash.py

@@ -1,4 +1,4 @@
-from thefuck import shells, utils
+from thefuck import shells
 from thefuck.specific.git import git_support
 
 

thefuck/rules/ssh_known_hosts.py

@@ -1,27 +1,23 @@
 import re
 from thefuck.utils import for_app
 
-patterns = [
-    r'WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!',
-    r'WARNING: POSSIBLE DNS SPOOFING DETECTED!',
-    r"Warning: the \S+ host key for '([^']+)' differs from the key for the IP address '([^']+)'",
-]
-offending_pattern = re.compile(
-    r'(?:Offending (?:key for IP|\S+ key)|Matching host key) in ([^:]+):(\d+)',
-    re.MULTILINE)
-
-commands = ['ssh', 'scp']
+commands = ('ssh', 'scp')
 
 
 @for_app(*commands)
 def match(command, settings):
     if not command.script:
         return False
-    if not command.script.split()[0] in commands:
+    if not command.script.startswith(commands):
         return False
-    if not any([re.findall(pattern, command.stderr) for pattern in patterns]):
-        return False
-    return True
+
+    patterns = (
+        r'WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!',
+        r'WARNING: POSSIBLE DNS SPOOFING DETECTED!',
+        r"Warning: the \S+ host key for '([^']+)' differs from the key for the IP address '([^']+)'",
+    )
+
+    return any(re.findall(pattern, command.stderr) for pattern in patterns)
 
 
 def get_new_command(command, settings):
@@ -29,6 +25,9 @@ def get_new_command(command, settings):
 
 
 def side_effect(old_cmd, command, settings):
+    offending_pattern = re.compile(
+        r'(?:Offending (?:key for IP|\S+ key)|Matching host key) in ([^:]+):(\d+)',
+        re.MULTILINE)
     offending = offending_pattern.findall(old_cmd.stderr)
     for filepath, lineno in offending:
         with open(filepath, 'r') as fh:

thefuck/rules/unknown_command.py

@@ -1,5 +1,5 @@
 import re
-from thefuck.utils import (replace_command, get_all_matched_commands)
+from thefuck.utils import replace_command
 
 def match(command, settings):
     return (re.search(r"([^:]*): Unknown command.*", command.stderr) != None

thefuck/shells.py

@@ -86,8 +86,7 @@ class Bash(Generic):
 
     @memoize
     def get_aliases(self):
-        proc = Popen('bash -ic alias', stdout=PIPE, stderr=DEVNULL,
-                     shell=True)
+        proc = Popen(['bash', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
         return dict(
             self._parse_alias(alias)
             for alias in proc.stdout.read().decode('utf-8').split('\n')
@@ -131,8 +130,7 @@ class Fish(Generic):
     @memoize
     def get_aliases(self):
         overridden = self._get_overridden_aliases()
-        proc = Popen('fish -ic functions', stdout=PIPE, stderr=DEVNULL,
-                     shell=True)
+        proc = Popen(['fish', '-ic', 'functions'], stdout=PIPE, stderr=DEVNULL)
         functions = proc.stdout.read().decode('utf-8').strip().split('\n')
         return {func: func for func in functions if func not in overridden}
 
@@ -172,8 +170,7 @@ class Zsh(Generic):
 
     @memoize
     def get_aliases(self):
-        proc = Popen('zsh -ic alias', stdout=PIPE, stderr=DEVNULL,
-                     shell=True)
+        proc = Popen(['zsh', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
         return dict(
             self._parse_alias(alias)
             for alias in proc.stdout.read().decode('utf-8').split('\n')
@@ -205,8 +202,7 @@ class Tcsh(Generic):
 
     @memoize
     def get_aliases(self):
-        proc = Popen('tcsh -ic alias', stdout=PIPE, stderr=DEVNULL,
-                     shell=True)
+        proc = Popen(['tcsh', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
         return dict(
             self._parse_alias(alias)
             for alias in proc.stdout.read().decode('utf-8').split('\n')

thefuck/specific/brew.py

@@ -0,0 +1,15 @@
+import subprocess
+from thefuck.utils import memoize, which
+
+
+enabled_by_default = bool(which('brew'))
+
+
+@memoize
+def get_brew_path_prefix():
+    """To get brew path"""
+    try:
+        return subprocess.check_output(['brew', '--prefix'],
+                                       universal_newlines=True).strip()
+    except:
+        return None

thefuck/utils.py

@@ -18,6 +18,23 @@ else:
     from shlex import quote
 
 
+def memoize(fn):
+    """Caches previous calls to the function."""
+    memo = {}
+
+    @wraps(fn)
+    def wrapper(*args, **kwargs):
+        key = pickle.dumps((args, kwargs))
+        if key not in memo or memoize.disabled:
+            memo[key] = fn(*args, **kwargs)
+
+        return memo[key]
+
+    return wrapper
+memoize.disabled = False
+
+
+@memoize
 def which(program):
     """Returns `program` path or `None`."""
 
@@ -53,22 +70,6 @@ def wrap_settings(params):
     return decorator(_wrap_settings)
 
 
-def memoize(fn):
-    """Caches previous calls to the function."""
-    memo = {}
-
-    @wraps(fn)
-    def wrapper(*args, **kwargs):
-        key = pickle.dumps((args, kwargs))
-        if key not in memo or memoize.disabled:
-            memo[key] = fn(*args, **kwargs)
-
-        return memo[key]
-
-    return wrapper
-memoize.disabled = False
-
-
 def get_closest(word, possibilities, n=3, cutoff=0.6, fallback_to_first=True):
     """Returns closest match or just first from possibilities."""
     possibilities = list(possibilities)