Invalidate cache if linter path or arguments change (#19)

Author: sieberst

gitlint/linters.py

@@ -55,15 +55,14 @@ def missing_requirements_command(missing_programs, installation_string,
 
 
 # TODO(skreft): add test case for result already in cache.
-def lint_command(name, program, arguments, filter_regex, filename, lines):
+def lint_command(program, arguments, filter_regex, filename, lines):
     """Executes a lint program and filter the output.
 
     Executes the lint tool 'program' with arguments 'arguments' over the file
     'filename' returning only those lines matching the regular expression
     'filter_regex'.
 
     Args:
-      name: string: the name of the linter.
       program: string: lint program.
       arguments: list[string]: extra arguments for the program.
       filter_regex: string: regular expression to filter lines.
@@ -73,7 +72,8 @@ def lint_command(name, program, arguments, filter_regex, filename, lines):
 
     Returns: dict: a dict with the extracted info from the message.
     """
-    output = utils.get_output_from_cache(name, filename)
+    linter_hash = utils.calculate_hash(program, arguments)
+    output = utils.get_output_from_cache(linter_hash, filename)
 
     if output is None:
         call_arguments = [program] + arguments + [filename]
@@ -91,7 +91,7 @@ def lint_command(name, program, arguments, filter_regex, filename, lines):
                 }
             }
         output = output.decode('utf-8')
-        utils.save_output_in_cache(name, filename, output)
+        utils.save_output_in_cache(linter_hash, filename, output)
 
     output_lines = output.split(os.linesep)
 
@@ -137,7 +137,7 @@ def parse_yaml_config(yaml_config, repo_home):
         'REPO_HOME': repo_home,
     }
 
-    for name, data in yaml_config.items():
+    for _, data in yaml_config.items():
         command = _replace_variables([data['command']], variables)[0]
         requirements = _replace_variables(
             data.get('requirements', []), variables)
@@ -149,7 +149,7 @@ def parse_yaml_config(yaml_config, repo_home):
             linter_command = Partial(missing_requirements_command,
                                      not_found_programs, data['installation'])
         else:
-            linter_command = Partial(lint_command, name, command, arguments,
+            linter_command = Partial(lint_command, command, arguments,
                                      data['filter'])
         for extension in data['extensions']:
             config[extension].append(linter_command)

gitlint/utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 """Common function used across modules."""
 
+import hashlib
 import io
 import os
 import re
@@ -72,29 +73,45 @@ def _open_for_write(filename):
     return io.open(filename, 'w')
 
 
-def _get_cache_filename(name, filename):
-    """Returns the cache location for filename and linter name."""
+def _get_cache_filename(linter_hash, filename):
+    """Returns the cache location for filename and linter hash."""
     filename = os.path.abspath(filename)[1:]
     home_folder = os.path.expanduser('~')
     base_cache_dir = os.path.join(home_folder, '.git-lint', 'cache')
 
-    return os.path.join(base_cache_dir, name, filename)
+    return os.path.join(base_cache_dir, linter_hash, filename)
 
 
-def get_output_from_cache(name, filename):
+def calculate_hash(program, arguments):
+    """Calculate sha256 hash as hex string over program and arguments.
+
+    Args:
+      program: string: lint program.
+      arguments: list[string]: extra arguments for the program.
+
+    Returns: string: a string with the calculated sha256 hex value.
+    """
+    algorithm = hashlib.sha256()
+    algorithm.update(program)
+    for argument in arguments:
+        algorithm.update(argument)
+    return algorithm.hexdigest()
+
+
+def get_output_from_cache(linter_hash, filename):
     """Returns the output from the cache if still valid.
 
     It checks that the cache file is defined and that its modification time is
     after the modification time of the original file.
 
     Args:
-      name: string: name of the linter.
+      linter_hash: string: hash representing linter binary and arguments.
       filename: string: path of the filename for which we are retrieving the
         output.
 
     Returns: a string with the output, if it is still valid, or None otherwise.
     """
-    cache_filename = _get_cache_filename(name, filename)
+    cache_filename = _get_cache_filename(linter_hash, filename)
     if (os.path.exists(cache_filename)
             and os.path.getmtime(filename) < os.path.getmtime(cache_filename)):
         with io.open(cache_filename) as f:
@@ -103,14 +120,14 @@ def get_output_from_cache(name, filename):
     return None
 
 
-def save_output_in_cache(name, filename, output):
+def save_output_in_cache(linter_hash, filename, output):
     """Saves output in the cache location.
 
     Args:
-      name: string: name of the linter.
+      linter_hash: string: hash representing linter binary and arguments.
       filename: string: path of the filename for which we are saving the output.
       output: string: full output (not yet filetered) of the lint command.
     """
-    cache_filename = _get_cache_filename(name, filename)
+    cache_filename = _get_cache_filename(linter_hash, filename)
     with _open_for_write(cache_filename) as f:
         f.write(output)

test/unittest/test_linters.py

@@ -36,7 +36,7 @@ def test_lint_command_success(self):
                 ['Line 1:1: 1', 'Line 5:2: 5 ·', 'Line 7:3: 7',
                  'Line 9:4: 9']).encode('utf-8')
             command = functools.partial(
-                linters.lint_command, 'l', 'linter', ['-f', '--compact'],
+                linters.lint_command, 'linter', ['-f', '--compact'],
                 r'^Line (?P<line>{lines}):(?P<column>\d+): (?P<message>.*)$')
             filename = 'foo.txt'
             self.assertEqual({
@@ -97,7 +97,7 @@ def test_lint_command_all_fields(self):
             check_output.return_value = os.linesep.join(
                 ['ERROR: line 1, col 1: (W32) missing foo']).encode('utf-8')
             command = functools.partial(
-                linters.lint_command, 'l', 'linter', ['-f', '--compact'],
+                linters.lint_command, 'linter', ['-f', '--compact'],
                 r'^(?P<severity>.*): line (?P<line>{lines})(, col ' +
                 r'(?P<column>\d+)): \((?P<message_id>.*)\) (?P<message>.*)$')
             filename = 'foo.txt'
@@ -124,7 +124,7 @@ def test_lint_command_error(self):
                             1, 'linter', output)) as check_output, \
                 mock.patch('os.path.getmtime', side_effect=[1, 0, 1, 0]):
             command = functools.partial(
-                linters.lint_command, 'l', 'linter', ['-f', '--compact'],
+                linters.lint_command, 'linter', ['-f', '--compact'],
                 '^Line (?P<line>{lines}): (?P<message>.*)$')
             filename = 'foo.txt'
             self.assertEqual({
@@ -177,7 +177,7 @@ def test_lint_command_not_found(self):
         with mock.patch('subprocess.check_output',
                         side_effect=OSError('Not found')) as check_output, \
                 mock.patch('os.path.getmtime', side_effect=[1, 0]):
-            command = functools.partial(linters.lint_command, 'l', 'linter',
+            command = functools.partial(linters.lint_command, 'linter',
                                         ['-f', '--compact'],
                                         '^Line ({lines}):')
             filename = 'foo.txt'
@@ -194,10 +194,10 @@ def test_lint_command_not_found(self):
 
     def test_lint(self):
         linter1 = functools.partial(
-            linters.lint_command, 'l1', 'linter1', ['-f'],
+            linters.lint_command, 'linter1', ['-f'],
             '^Line (?P<line>{lines}): (?P<message>.*)$')
         linter2 = functools.partial(
-            linters.lint_command, 'l2', 'linter2', [],
+            linters.lint_command, 'linter2', [],
             '^ line (?P<line>{lines}): (?P<message>.*)$')
         config = {'.txt': [linter1, linter2]}
         outputs = [
@@ -231,15 +231,15 @@ def test_lint(self):
 
     def test_lint_output_is_sorted(self):
         linter1 = functools.partial(
-            linters.lint_command, 'l1', 'linter1', ['-f'],
+            linters.lint_command, 'linter1', ['-f'],
             '^Line (?P<line>{lines}): (?P<message>.*)$')
         linter2 = functools.partial(
-            linters.lint_command, 'l2', 'linter2', [],
+            linters.lint_command, 'linter2', [],
             '^ line (?P<line>{lines}): (?P<message>.*)$')
-        linter3 = functools.partial(linters.lint_command, 'l3', 'linter3', [],
+        linter3 = functools.partial(linters.lint_command, 'linter3', [],
                                     '^(?P<message>.*)$')
         linter4 = functools.partial(
-            linters.lint_command, 'l4', 'linter4', [],
+            linters.lint_command, 'linter4', [],
             r'^(?P<line>{lines}):(?P<column>\d+): (?P<message>.*)$')
         config = {'.txt': [linter1, linter2, linter3, linter4]}
         outputs = [
@@ -285,10 +285,10 @@ def test_lint_output_is_sorted(self):
 
     def test_lint_one_empty_lint(self):
         linter1 = functools.partial(
-            linters.lint_command, 'l1', 'linter1', ['-f'],
+            linters.lint_command, 'linter1', ['-f'],
             '^Line (?P<line>{lines}): (?P<message>.*)$')
         linter2 = functools.partial(
-            linters.lint_command, 'l2', 'linter2', [],
+            linters.lint_command, 'linter2', [],
             '^ line (?P<line>{lines}): (?P<message>.*)$')
         config = {'.txt': [linter1, linter2]}
         outputs = [b'', os.linesep.join([' line 4: 4']).encode('utf-8')]
@@ -314,9 +314,9 @@ def test_lint_one_empty_lint(self):
             self.assertEqual(expected_calls, check_output.call_args_list)
 
     def test_lint_all_empty_lint(self):
-        linter1 = functools.partial(linters.lint_command, 'l1', 'linter1',
+        linter1 = functools.partial(linters.lint_command, 'linter1',
                                     ['-f'], '^Line {lines}:')
-        linter2 = functools.partial(linters.lint_command, 'l2', 'linter2', [],
+        linter2 = functools.partial(linters.lint_command, 'linter2', [],
                                     '^ line {lines}:')
         config = {'.txt': [linter1, linter2]}
         outputs = [b'', b'']