firmed up new tools api.

imhotep/tools.py

@@ -7,34 +7,17 @@
 
 
 class Tool(object):
+    """
+    Tool represents a program that runs over source code. It returns a nested
+    dictionary structure like:
+
+      {'relative_filename': {'line_number': [error1, error2]}}
+      eg: {'imhotep/main.py': {'103': ['line too long']}}
+    """
     def __init__(self, command_executor, filenames=set()):
         self.executor = command_executor
         self.filenames = filenames
 
-    def process_line(self, dirname, line):
-        """
-        Processes a line return a 3-element tuple representing (filename,
-        line_number, error_messages) or None to indicate no error.
-
-        :param: dirname - directory the code is running in
-        """
-        raise NotImplementedError()
-
-    def get_file_extensions(self):
-        """
-        Returns a list of file extensions this tool should run against.
-
-        eg: ['.py', '.js']
-        """
-        raise NotImplementedError()
-
-    def get_command(self, dirname):
-        """
-        Returns the command to run for linting. It is piped a list of files to
-        run on over stdin.
-        """
-        raise NotImplementedError()
-
     def invoke(self, dirname, filenames=set()):
         """
         Main entrypoint for all plugins.
@@ -61,9 +44,35 @@ def invoke(self, dirname, filenames=set()):
             output = self.process_line(dirname, line)
             if output is not None:
                 filename, lineno, messages = output
+                if filename.startswith(dirname):
+                    filename = filename[len(dirname)+1:]
                 retval[filename][lineno].append(messages)
         return retval
 
+    def process_line(self, dirname, line):
+        """
+        Processes a line return a 3-element tuple representing (filename,
+        line_number, error_messages) or None to indicate no error.
+
+        :param: dirname - directory the code is running in
+        """
+        raise NotImplementedError()
+
+    def get_file_extensions(self):
+        """
+        Returns a list of file extensions this tool should run against.
+
+        eg: ['.py', '.js']
+        """
+        raise NotImplementedError()
+
+    def get_command(self, dirname):
+        """
+        Returns the command to run for linting. It is piped a list of files to
+        run on over stdin.
+        """
+        raise NotImplementedError()
+
 
 class JSHint(Tool):
     response_format = re.compile(r'^(?P<filename>.*): " \
@@ -102,42 +111,17 @@ def process_line(self, dirname, line):
             if len(self.filenames) != 0:
                 if match.group('filename') not in self.filenames:
                     return
-            return match.groups()
+            filename, line, messages = match.groups()
+            # If you run pylint on /foo/bar/baz and you are in the /foo/bar
+            # directory, it will spit out paths that look like: ./baz To fix
+            # this, we run it through `os.path.abspath` which will give it a
+            # full, absolute path.
+            filename = os.path.abspath(filename)
+            return filename, line, messages
 
     def get_command(self, dirname):
         cmd = 'pylint --output-format=parseable -rn'
         if os.path.exists(os.path.join(dirname, self.pylintrc_filename)):
             cmd += " --rcfile=%s" % os.path.join(
                 dirname, self.pylintrc_filename)
         return cmd
-
-
-    # def invoke(self, dirname, filenames=set()):
-    #     to_return = defaultdict(lambda: defaultdict(list))
-    #     log.debug("Running pylint on %s", dirname)
-    #     cmd = 'find %s -name "*.py" | ' \
-    #           'xargs pylint --output-format=parseable -rn'
-
-    #     if os.path.exists(os.path.join(dirname, self.pylintrc_filename)):
-    #         cmd += " --rcfile=%s" % os.path.join(
-    #             dirname, self.pylintrc_filename)
-    #     result = self.executor(cmd % dirname)
-
-
-    #     # pylint is stupid, this should fix relative path linting
-    #     # if repo is checked out relative to where imhotep is called.
-    #     if os.path.abspath('.') in dirname:
-    #         dirname = dirname[len(os.path.abspath('.'))+1:]
-
-    #     # splitting based on newline + dirname and trailing slash will make
-    #     # beginning of line until first colon the relative filename. It also
-    #     # has the nice side effect of allowing us multi-line output from the
-    #     # tool without things breaking.
-    #     for line in result.split("\n%s/" % dirname):
-    #         if len(line) == 0:
-    #             continue
-    #         filename, line_num, error = line.split(':', 2)
-    #         if len(filenames) != 0 and filename not in filenames:
-    #             continue
-    #         to_return[filename][line_num].append(error)
-    #     return to_return

imhotep/tools_test.py

@@ -85,10 +85,22 @@ def test_appends_process_line_results_to_results():
     m = mock.Mock()
     m.return_value = ""
     process_mock = mock.Mock()
-    process_mock.return_value = (1, 2, 3)
+    process_mock.return_value = ('filename', 2, 3)
     t = TestTool(m)
     t.process_line = process_mock
     retval = t.invoke('/woobie/')
 
     assert 1 == len(retval.keys())
-    assert retval[1][2][0] == 3
+    assert retval['filename'][2][0] == 3
+
+
+def test_invoke_removes_dirname_prefix():
+    m = mock.Mock()
+    m.return_value = ""
+    process_mock = mock.Mock()
+    process_mock.return_value = ('/my/full/path/and/extras', 2, 3)
+    t = TestTool(m)
+    t.process_line = process_mock
+    retval = t.invoke('/my/full/path')
+
+    assert 'and/extras' in retval.keys()