Fix quality violations; enable quality checks via tox/make (#99)

.gitignore

@@ -15,3 +15,6 @@ reports/*
 
 # OS detritus
 *.DS_Store
+
+# Virtual environment
+/venv/

Makefile

@@ -24,3 +24,8 @@ upgrade: ## update the requirements/*.txt files with the latest packages satisfy
 	pip-compile --upgrade -o requirements/tox.txt requirements/tox.in
 	pip-compile --upgrade -o requirements/testing.txt requirements/testing.in
 	pip-compile --upgrade -o requirements/sandbox.txt requirements/sandbox.in
+
+quality: ## check coding style with pycodestyle and pylint
+	pycodestyle codejail *.py
+	isort --check-only --diff --recursive codejail *.py
+	pylint codejail *.py

codejail/django_integration.py

@@ -4,8 +4,10 @@
 
 """
 
-from django.core.exceptions import MiddlewareNotUsed
+# pylint: skip-file
+
 from django.conf import settings
+from django.core.exceptions import MiddlewareNotUsed
 from django.utils.deprecation import MiddlewareMixin
 
 from . import django_integration_utils

codejail/jail_code.py

@@ -1,13 +1,11 @@
 """Run code in a jail."""
 
-from __future__ import absolute_import
 import logging
 import os
 import os.path
 import resource
 import shutil
 import sys
-
 from builtins import bytes
 
 from .proxy import run_subprocess_through_proxy
@@ -16,7 +14,7 @@
 
 log = logging.getLogger("codejail")
 
-# TODO: limit too much stdout data?
+# TODO: limit too much stdout data?  # pylint: disable=fixme
 
 # Configure the commands
 
@@ -60,12 +58,15 @@ def is_configured(command):
     """
     return command in COMMANDS
 
+
 # By default, look where our current Python is, and maybe there's a
 # python-sandbox alongside.  Only do this if running in a virtualenv.
 # The check for sys.real_prefix covers virtualenv
 # the equality of non-empty sys.base_prefix with sys.prefix covers venv
-running_in_virtualenv = (hasattr(sys, 'real_prefix') or
-    (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix))
+running_in_virtualenv = (
+    hasattr(sys, 'real_prefix') or
+    (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix)
+)
 
 if running_in_virtualenv:
     # On jenkins
@@ -176,7 +177,7 @@ def override_limit(limit_name, value, limit_overrides_context):
     LIMIT_OVERRIDES[limit_overrides_context][limit_name] = value
 
 
-class JailResult(object):
+class JailResult:
     """
     A passive object for us to return from jail_code.
     """

codejail/proxy.py

@@ -1,17 +1,17 @@
 """A proxy subprocess-making process for CodeJail."""
 
-from __future__ import absolute_import
 import ast
 import logging
 import os
 import os.path
 import subprocess
 import sys
 import time
+
 import six
+from six.moves import range
 
 from .subproc import run_subprocess
-from six.moves import range
 
 log = logging.getLogger("codejail")
 
@@ -47,7 +47,7 @@ def deserialize_out(bstr):
 
 
 ##
-## Client code, runs in the parent CodeJail process.
+# Client code, runs in the parent CodeJail process.
 ##
 
 def run_subprocess_through_proxy(*args, **kwargs):
@@ -58,7 +58,7 @@ def run_subprocess_through_proxy(*args, **kwargs):
 
     """
     last_exception = None
-    for tries in range(3):
+    for _tries in range(3):
         try:
             proxy = get_proxy()
 
@@ -79,7 +79,7 @@ def run_subprocess_through_proxy(*args, **kwargs):
             for level, msg, args in log_calls:
                 log.log(level, msg, *args)
             return status, stdout, stderr
-        except Exception as e:
+        except Exception:  # pylint: disable=broad-except
             log.exception("Proxy process failed")
             # Give the proxy process a chance to die completely if it is dying.
             time.sleep(.001)
@@ -94,8 +94,10 @@ def run_subprocess_through_proxy(*args, **kwargs):
 # There is one global proxy process.
 PROXY_PROCESS = None
 
+
 def get_proxy():
-    global PROXY_PROCESS
+    # pylint: disable=missing-function-docstring
+    global PROXY_PROCESS  # pylint: disable=global-statement
 
     # If we had a proxy process, but it died, clean up.
     if PROXY_PROCESS is not None:
@@ -132,7 +134,7 @@ def get_proxy():
     return PROXY_PROCESS
 
 ##
-## Proxy process code
+# Proxy process code
 ##
 
 
@@ -145,6 +147,9 @@ class CapturingHandler(logging.Handler):
     the caller, the current exception, the current time, etc.
 
     """
+    # pylint wants us to override emit().
+    # pylint: disable=abstract-method
+
     def __init__(self):
         super(CapturingHandler, self).__init__()
         self.log_calls = []
@@ -156,6 +161,7 @@ def handle(self, record):
         self.log_calls.append((record.levelno, record.msg, record.args))
 
     def get_log_calls(self):
+        # pylint: disable=missing-function-docstring
         retval = self.log_calls
         self.log_calls = []
         return retval
@@ -194,17 +200,20 @@ def proxy_main(argv):
     try:
         while True:
             stdin = sys.stdin.readline()
-            log.debug("proxy stdin: %r" % stdin)
+            log.debug("proxy stdin: %r", stdin)
             if not stdin:
                 break
             args, kwargs = deserialize_in(stdin.rstrip())
             status, stdout, stderr = run_subprocess(*args, **kwargs)
-            log.debug("run_subprocess result: status=%r\nstdout=%r\nstderr=%r" % (status, stdout, stderr))
+            log.debug(
+                "run_subprocess result: status=%r\nstdout=%r\nstderr=%r",
+                status, stdout, stderr,
+            )
             log_calls = capture_log.get_log_calls()
             stdout = serialize_out((status, stdout, stderr, log_calls))
             sys.stdout.write(stdout+"\n")
             sys.stdout.flush()
-    except Exception:
+    except Exception:  # pylint: disable=broad-except
         # Note that this log message will not get back to the parent, because
         # we are dying and not communicating back to the parent. This will be
         # useful only if you add another handler at the top of this function.

codejail/safe_exec.py

@@ -1,21 +1,22 @@
 """Safe execution of untrusted Python code."""
 
-from __future__ import absolute_import
+import inspect
 import logging
 import os.path
 import shutil
 import sys
 import textwrap
+
 import six
-import inspect
+
+from codejail import jail_code
+from codejail.util import change_directory, temp_directory
 
 try:
     import simplejson as json
 except ImportError:
     import json
 
-from codejail import jail_code
-from codejail.util import temp_directory, change_directory
 
 log = logging.getLogger("codejail")
 
@@ -36,11 +37,17 @@ class SafeExecException(Exception):
     contain the original exception message.
 
     """
-    pass
 
 
-def safe_exec(code, globals_dict, files=None, python_path=None,
-              limit_overrides_context=None, slug=None, extra_files=None):
+def safe_exec(
+        code,
+        globals_dict,
+        files=None,
+        python_path=None,
+        limit_overrides_context=None,
+        slug=None,
+        extra_files=None,
+):
     """
     Execute code as "exec" does, but safely.
 
@@ -170,8 +177,9 @@ def json_safe(d):
     Used to emulate reading data through a serialization straw.
 
     """
+    # pylint: disable=invalid-name
 
-    # six.binary_type is here because bytes are sometimes ok if they represent valid utf8 
+    # six.binary_type is here because bytes are sometimes ok if they represent valid utf8
     # so we consider them valid for now and try to decode them with decode_object.  If that
     # doesn't work they'll get dropped later in the process.
     ok_types = (type(None), int, float, six.binary_type, six.text_type, list, tuple, dict)
@@ -190,21 +198,20 @@ def decode_object(obj):
         """
         if isinstance(obj, bytes):
             return obj.decode('utf-8')
-        elif isinstance(obj, (list,tuple)):
+        if isinstance(obj, (list, tuple)):
             new_list = []
             for i in obj:
                 new_obj = decode_object(i)
                 new_list.append(new_obj)
             return new_list
-        elif isinstance(obj, dict):
+        if isinstance(obj, dict):
             new_dict = {}
-            for k,v in six.iteritems(obj):
+            for k, v in six.iteritems(obj):
                 new_key = decode_object(k)
                 new_value = decode_object(v)
                 new_dict[new_key] = new_value
             return new_dict
-        else:
-            return obj
+        return obj
 
     bad_keys = ("__builtins__",)
     jd = {}
@@ -223,16 +230,22 @@ def decode_object(obj):
 
             # Also ensure that the keys encode/decode correctly
             k = json.loads(json.dumps(decode_object(k)))
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
             continue
         else:
             jd[k] = v
     return json.loads(json.dumps(jd))
 
 
-def not_safe_exec(code, globals_dict, files=None, python_path=None,
-                  limit_overrides_context=None,  # pylint: disable=unused-argument
-                  slug=None, extra_files=None):
+def not_safe_exec(
+        code,
+        globals_dict,
+        files=None,
+        python_path=None,
+        limit_overrides_context=None,  # pylint: disable=unused-argument
+        slug=None,  # pylint: disable=unused-argument
+        extra_files=None,
+):
     """
     Another implementation of `safe_exec`, but not safe.
 
@@ -248,6 +261,7 @@ def not_safe_exec(code, globals_dict, files=None, python_path=None,
 
     with temp_directory() as tmpdir:
         with change_directory(tmpdir):
+            # pylint: disable=invalid-name
             # Copy the files here.
             for filename in files or ():
                 dest = os.path.join(tmpdir, os.path.basename(filename))
@@ -261,7 +275,7 @@ def not_safe_exec(code, globals_dict, files=None, python_path=None,
             if python_path:
                 sys.path.extend(python_path)
             try:
-                exec(code, g_dict)
+                exec(code, g_dict)  # pylint: disable=exec-used
             except Exception as e:
                 # Wrap the exception in a SafeExecException, but we don't
                 # try here to include the traceback, since this is just a

codejail/subproc.py

@@ -1,6 +1,5 @@
 """Subprocess helpers for CodeJail."""
 
-from __future__ import absolute_import
 import functools
 import logging
 import os
@@ -13,8 +12,8 @@
 
 
 def run_subprocess(
-    cmd, stdin=None, cwd=None, env=None, rlimits=None, realtime=None,
-    slug=None,
+        cmd, stdin=None, cwd=None, env=None, rlimits=None, realtime=None,
+        slug=None,
 ):
     """
     A helper to make a limited subprocess.
@@ -37,11 +36,11 @@ def run_subprocess(
     the stdout and stderr of the process, as strings.
 
     """
-    subproc = subprocess.Popen(
+    subproc = subprocess.Popen(  # pylint: disable=subprocess-popen-preexec-fn
         cmd, cwd=cwd, env=env,
         preexec_fn=functools.partial(set_process_limits, rlimits or ()),
         stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-        )
+    )
 
     if slug:
         log.info("Executed jailed code %s in %s, with PID %s", slug, cwd, subproc.pid)

codejail/tests/doit.py

@@ -1,5 +1,6 @@
-from __future__ import absolute_import
-from __future__ import print_function
+# pylint: skip-file isort:skip_file
+from __future__ import absolute_import, print_function
+
 import sys
 
 print("This is doit.py!")

codejail/tests/test_django_integration_utils.py

@@ -4,7 +4,6 @@
 
 from .. import jail_code
 from ..django_integration_utils import apply_django_settings
-
 from .util import ResetJailCodeStateMixin