Add flake8 linter to project and travis ci (#46)

Author: ggilmore

.travis.yml

@@ -13,4 +13,5 @@ before_install:
 install: "make"
 
 script:
+    - make lint
     - make test

Makefile

@@ -4,6 +4,9 @@ init:
 	pip install pipenv
 	pipenv install --dev
 
+lint:
+	pipenv run flake8
+
 test:
 	pipenv run pytest test_langserver.py
 	cd ./test && pipenv run pytest test_*.py

Pipfile

@@ -18,6 +18,9 @@ lightstep = "*"
 py = "==1.4.34"
 pytest = "==3.2.3"
 "autopep8" = "*"
+"flake8" = "*"
+autoflake = "*"
+docformatter = "*"
 
 
 [requires]

Pipfile.lock

@@ -1,7 +1,6 @@
-import os
-import os.path
 import distutils
 
+
 class GlobalConfig:
 
     # TODO: allow different Python stdlib versions per workspace?

langserver/config.py

@@ -8,14 +8,15 @@
 
 
 class TargetedSymbolVisitor:
-    """
-    The purpose of this class is to take a SymbolDescriptor (typically the result of a preceding x-definition request)
-    and perform a more precise symbol search based on the extra metadata available in the descriptor.
-    """
+    """The purpose of this class is to take a SymbolDescriptor (typically the
+    result of a preceding x-definition request) and perform a more precise
+    symbol search based on the extra metadata available in the descriptor."""
+
     def __init__(self, name, kind, path):
         self.name = name
         self.kind = kind
-        # the AST nodes for modules don't contain their name, so we have to infer it from the file or folder name
+        # the AST nodes for modules don't contain their name, so we have to
+        # infer it from the file or folder name
         folder, file = os.path.split(path)
         if file == "__init__.py":
             self.module = os.path.basename(folder)
@@ -38,7 +39,8 @@ def visit_Module(self, node, container):
             yield from self.generic_visit(node)
 
     def visit_ImportFrom(self, node, container):
-        # this handles the case where imported symbols are re-exported; an xj2d sometimes needs to land here
+        # this handles the case where imported symbols are re-exported; an xj2d
+        # sometimes needs to land here
         for n in node.names:
             if n.name == self.name:
                 yield Symbol(
@@ -116,8 +118,8 @@ def visit_Assign(self, assign_node, container):
                 return
 
     def visit_If(self, node, container):
-        # If is often used provide different implementations for the same var. To avoid duplicate names, we only visit
-        # the true body.
+        # If is often used provide different implementations for the same var. To avoid duplicate
+        # names, we only visit the true body.
         for child in node.body:
             yield from self.visit(child, container)
 
@@ -171,9 +173,11 @@ def targeted_symbol(symbol_descriptor, fs, root_path, parent_span):
         try:
             tree = ast.parse(source, path)
         except SyntaxError as e:
-            log.error("Error parsing Python file %s:%s -- %s: %s", path, e.lineno, e.msg, e.text)
+            log.error("Error parsing Python file %s:%s -- %s: %s",
+                      path, e.lineno, e.msg, e.text)
             continue
-        visitor = TargetedSymbolVisitor(symbol_descriptor["name"], symbol_descriptor["kind"], path)
+        visitor = TargetedSymbolVisitor(
+            symbol_descriptor["name"], symbol_descriptor["kind"], path)
         for sym in visitor.visit(tree):
             sym.file = path
             symbols.append(sym.json_object())

langserver/definitions.py

@@ -11,39 +11,50 @@
 
 
 def fetch_dependency(module_name: str, specifier: str, install_path: str):
-    """
-    Shells out to PIP in order to download and unzip the named package into the specified path. This method only runs
-    `pip download`, NOT `pip install`, so it's presumably safe.
+    """Shells out to PIP in order to download and unzip the named package into
+    the specified path. This method only runs `pip download`, NOT `pip
+    install`, so it's presumably safe.
+
     :param module_name: the name of the package to download
     :param specifier: the version specifier for the package
     :param install_path: the path in which to install the downloaded package
     """
     with tempfile.TemporaryDirectory() as download_folder:
-        log.info("Attempting to download package %s to %s", module_name, download_folder, exc_info=True)
+        log.info("Attempting to download package %s to %s",
+                 module_name, download_folder, exc_info=True)
         # TODO: check the result status
 
         index_url = os.environ.get('INDEX_URL')
         if index_url is not None:
-            result = pip.main(["download", "--no-deps", "-i", index_url, "-d", download_folder, module_name+specifier])
+            result = pip.main(["download", "--no-deps", "-i", index_url,
+                               "-d", download_folder, module_name + specifier])
         else:
-            result = pip.main(["download", "--no-deps", "-d", download_folder, module_name+specifier])
+            result = pip.main(["download", "--no-deps", "-d",
+                               download_folder, module_name + specifier])
         if result != pip.status_codes.SUCCESS:
             log.error("Unable to fetch package %s", module_name)
             return
         for thing in os.listdir(download_folder):
             thing_abs = os.path.join(download_folder, thing)
             if os.path.isdir(thing_abs):
-                log.debug("Moving %s to %s", thing, install_path, exc_info=True)
+                log.debug("Moving %s to %s", thing,
+                          install_path, exc_info=True)
                 shutil.move(thing_abs, install_path)
             elif thing.endswith(".whl") or thing.endswith(".zip"):
-                log.debug("Unzipping %s to %s", thing, install_path, exc_info=True)
-                result = subprocess.run(["unzip", "-o", "-d", install_path, thing_abs])
+                log.debug("Unzipping %s to %s", thing,
+                          install_path, exc_info=True)
+                result = subprocess.run(
+                    ["unzip", "-o", "-d", install_path, thing_abs])
             elif thing.endswith(".tar.gz"):
-                log.debug("Untarring %s to %s", thing, install_path, exc_info=True)
-                result = subprocess.run(["tar", "-C", install_path, "-xzf", thing_abs])
+                log.debug("Untarring %s to %s", thing,
+                          install_path, exc_info=True)
+                result = subprocess.run(
+                    ["tar", "-C", install_path, "-xzf", thing_abs])
             elif thing.endswith(".tar.bz2"):
-                log.debug("Untarring %s to %s", thing, install_path, exc_info=True)
-                result = subprocess.run(["tar", "-C", install_path, "-xjf", thing_abs])
+                log.debug("Untarring %s to %s", thing,
+                          install_path, exc_info=True)
+                result = subprocess.run(
+                    ["tar", "-C", install_path, "-xjf", thing_abs])
             else:
-                log.warning("Unrecognized package file: %s", thing, exc_info=True)
-
+                log.warning("Unrecognized package file: %s",
+                            thing, exc_info=True)

langserver/fetch.py

@@ -1,4 +1,3 @@
-import base64
 import os
 import os.path
 from abc import ABC, abstractmethod
@@ -54,7 +53,6 @@ def open(self, path, parent_span=None):
             return open_file.read()
 
     def listdir(self, path, parent_span=None):
-        entries = []
         names = os.listdir(path)
         # TODO: prepend `path` to each name?
         return names
@@ -124,7 +122,7 @@ def walk(self, path):
 
     def batch_open(self, paths, parent_span):
         with opentracing.start_child_span(
-                parent_span, "RemoteFileSystem.batch_open") as batch_open_span:
+                parent_span, "RemoteFileSystem.batch_open"):
             # We need to read the iterator paths twice, so convert to list
             paths = list(paths)
             responses = self.conn.send_request_batch(("textDocument/xcontent",
@@ -136,7 +134,8 @@ def batch_open(self, paths, parent_span):
                                                       }) for path in paths)
             for path, resp in zip(paths, responses):
                 if "error" in resp:
-                    # Consume rest of generator to ensure resources are shutdown
+                    # Consume rest of generator to ensure resources are
+                    # shutdown
                     for _ in responses:
                         pass
                     raise FileException(resp["error"])
@@ -174,8 +173,9 @@ def listdir(self, path: str, parent_span) -> List[Entry]:
         return entries.values()
 
 
-# TODO(aaron): determine whether this extra filesystem is really necessary, or if we could have just used a local fs
-# I suspect not, because the new workspace indexing/importing code wasn't written with a local fs in mind
+# TODO(aaron): determine whether this extra filesystem is really necessary, or if we could have
+# just used a local fs I suspect not, because the new workspace indexing/importing code wasn't
+# written with a local fs in mind
 class TestFileSystem(FileSystem):
     def __init__(self, local_root_path: str):
         self.root = os.path.abspath(local_root_path)
@@ -194,7 +194,9 @@ def batch_open(self, paths, parent_span):
 
     def listdir(self, path: str, parent_span=None):
         path = os.path.abspath(path)
-        if not path.startswith(self.root):  # need this check for namespace imports, for which we get a relative path
+        # need this check for namespace imports, for which we get a relative
+        # path
+        if not path.startswith(self.root):
             if path.startswith("/"):
                 path = path[1:]
             path = os.path.join(self.root, path)

langserver/fs.py

@@ -4,13 +4,15 @@
 
 
 def get_imports(fs, root_path, parent_span):
-    # TODO: consider crawling over the main project files only; ignore examples, tests, etc
+    # TODO: consider crawling over the main project files only; ignore
+    # examples, tests, etc
     py_paths = (path for path in fs.walk(root_path) if path.endswith(".py"))
     py_srces = fs.batch_open(py_paths, parent_span)
     with multiprocessing.Pool() as p:
         import_chunks = p.imap_unordered(
             _imap_extract_imports, py_srces, chunksize=10)
-        imports = {i.split(".")[0] for i in itertools.chain.from_iterable(import_chunks)}
+        imports = {i.split(".")[0]
+                   for i in itertools.chain.from_iterable(import_chunks)}
     return imports
 
 
@@ -41,7 +43,9 @@ def visit_Import(self, node, container):
             yield n.name
 
     def visit_ImportFrom(self, node, container):
-        if not node.level:  # we only care about top-level imports, and definitely want to ignore internal imports
+        # we only care about top-level imports, and definitely want to
+        # ignore internal imports
+        if not node.level:
             yield node.module
 
     # Based on ast.NodeVisitor.visit