Multiple Projects per Repo, Eslint Enhancements, and More...

.dockerignore

@@ -0,0 +1,14 @@
+*.pyc
+build/
+dist/
+*.egg-info
+tags
+/env
+/settings.py
+/workspace
+/ca.crt
+/node_modules
+/vendor
+/bundle
+/.bundle
+output.log

.travis.yml

@@ -20,6 +20,7 @@ before_install:
 
 install:
   - pip install -r requirements-dev.txt
+  - pip install -r requirements-linters.txt
   - pip install .
   - pip install codecov
   - pip install coverage

Dockerfile

@@ -36,8 +36,8 @@ ENV BUNDLE_SILENCE_ROOT_WARNING 1
 ADD Gemfile Gemfile.lock /code/
 RUN bundler install --system
 
-ADD requirements.txt /code/
-RUN pip install -r requirements.txt
+ADD requirements.txt requirements-linters.txt /code/
+RUN pip install -r requirements.txt -r requirements-linters.txt
 ADD . /code
 RUN pip install .
 RUN cp /code/settings.sample.py /code/settings.py

README.mdown

@@ -178,6 +178,23 @@ are used to find and exclude files when doing a review. Ignore patterns use glob
 to find files. The patterns start at the reviewed repository root. If you need to ignore mulitple
 patterns separate them with new lines.
 
+### Multiple projects in one repo
+
+You can list multiple `tool_` sections as follows (**note the _# suffixes**):
+
+    [tool_eslint_#client]
+    config = client/.eslintrc.json
+    working_dir = client
+    install = true
+
+    [tool_eslint_#server]
+    config = server/.eslintrc.json
+    working_dir = server
+    install = true
+
+Each section must have a unique name, but anything after and including `_#` is ignored when determining the linter for which it applies. This allows you to run the same linter multiple times with different settings for each run. This can be useful if, as in the example above, you have two different projects (e.g. client and server) in the same repository.
+
+To avoid duplicate or conflicting comments, make sure that the each project ignores the files of the other. Using the example above, there should be a .eslintignore file in the `client` directory that ignores files in `server`, and vice versa. Depending on the tool, you might also be able to do this using an ignore pattern in the .lintrc. The documentation for each linter outlines which settings are available.
 
 ## Running Lint Review
 
@@ -254,6 +271,15 @@ to do style checks on PHP, Javascript and or CSS files.
 * `exclude` A comma separated list of sniffs to not apply.
 * `tab_width` The number of spaces to convert tabs into, this is useful for projects using
   tabs for indentation.
+* `working_dir` Change the working directory before linting. 
+* `install` Run composer install before linting. Runs in `working_dir`, if specified. Otherwise, runs in the root directory of this linter. Any non-empty value for this option will trigger an install.
+
+`working_dir` and `install` can be used together to avoid having to pre-install linting packages inside this linter. Instead, set `working_dir` to the location of your project's composer.json and set `install = true`.
+
+**NOTE** If `install` isn't specified, you'll need to install the PHPCS globall:
+
+    cd path/to/lintreview
+    composer require "squizlabs/php_codesniffer=*"
 
 ### Javascript:
 
@@ -284,15 +310,39 @@ you can use this linter you'll need to install nodejs and the jshint npm package
 
 #### ESLint
 
-Uses the [eslint](http://eslint.org/) npm module to check javascript files. Before
-you can use this linter you'll need to install the eslint npm package:
+Uses the [eslint](http://eslint.org/) npm module to check javascript files. 
+
+*Options*
+
+* `config` Provide a path to the json config file for eslint.
+* `extensions` The extensions to check. By default only `.js` and `.jsx` files will be checked.
+* `working_dir` Change the working directory before linting. 
+* `install` Run npm install before linting. Runs in `working_dir`, if specified. Otherwise, runs in the root directory of this linter. Any non-empty value for this option will trigger an install.
+
+`working_dir` and `install` can be used together to avoid having to pre-install linting packages inside this linter. For example, if one of your projects requires `eslint-plugin-vue` while the other needs `eslint-plugin-react`, you don't need to install them both inside the linter. Instead, set `working_dir` to the location of your project's package.json and set `install = true`. This will also help if you're having problems with the linter being unable to resolve required packages.
+
+**NOTE** If `install` isn't specified, you'll need to install the eslint npm package:
 
     cd path/to/lintreview
     npm install eslint
 
-*Options*
+If you have multiple eslint configurations listed, and if your projects are laid out such that one project's `working_dir` is *inside* the other's, then you should do one of two things:
+1. Add `root: true` to the .eslintrc of the project in the child directory
+2. Run the project with the top-most `working_dir` first.
 
-* `config` Provide a path to the json config file for eslint.
+For example, consider the following setup:
+
+    [tool_eslint_#server]
+    config = app/.eslintrc.json
+    working_dir = app
+    install = true
+
+    [tool_eslint_#client]
+    config = app/public/.eslintrc.json
+    working_dir = app/public
+    install = true
+
+The client is in `app/public`, which is inside `app`. If you are intentionally extending the server eslint config in the client directory, then the server should be run first by renaming the sections `tool_eslint_#1_server` and `tool_eslint_#2_client`. Otherwiser, if the client's eslint config should be completely independent of the server's, then `root: true` should be added to `app/public/.eslintrc.json`.
 
 #### StandardJs
 

alpine.Dockerfile

@@ -0,0 +1,21 @@
+FROM alpine:3.7
+RUN apk add --no-cache \
+        curl=7.58.0-r1 \
+        gcc=6.4.0-r5 \
+        git=2.15.0-r1 \
+        libffi-dev=3.2.1-r4 \
+        libxml2=2.9.7-r0 \
+        musl-dev=1.1.18-r3 \
+        openssl-dev=1.0.2n-r0 \
+        openssl=1.0.2n-r0 \
+        python2=2.7.14-r2 \
+        python2-dev=2.7.14-r2 \
+        py2-pip=9.0.1-r1 \
+        zlib-dev=1.2.11-r1
+
+WORKDIR /code
+ADD requirements.txt ./
+RUN pip install -r requirements.txt
+ADD . ./
+RUN pip install . \
+ && cp settings.sample.py settings.py

docker_healthcheck.sh

@@ -1,10 +1,10 @@
-#!/bin/bash
+#!/bin/sh
 set -eo pipefail
 
-if [ "$1" == "web" ]; then
+if [ "$1" = "web" ]; then
   curl -f http://localhost:5000/ping
 fi
 
-if [ "$1" == "worker" ]; then
+if [ "$1" = "worker" ]; then
   celery inspect ping -A lintreview.tasks -d "celery@$HOSTNAME"
 fi

lintreview/tasks.py

@@ -7,13 +7,12 @@
 from lintreview.repo import GithubRepository
 from lintreview.processor import Processor
 
-config = load_config()
 celery = Celery('lintreview.tasks')
-celery.config_from_object(config)
+celery.config_from_object('settings')
 
+config = load_config()
 log = logging.getLogger(__name__)
 
-
 @celery.task(ignore_result=True)
 def process_pull_request(user, repo_name, number, lintrc):
     """

lintreview/tools/__init__.py

@@ -23,6 +23,12 @@ def __init__(self, problems, options=None, base_path=None):
         if isinstance(options, dict):
             self.options = options
 
+    def get_working_dir(self):
+        working_dir = self.options.get('working_dir')
+        if working_dir:
+            return os.path.join(self.base_path, working_dir)
+        return os.getcwd()
+
     def check_dependencies(self):
         """
         Used to check for a tools commandline
@@ -196,6 +202,7 @@ def factory(config, problems, base_path):
     for linter in config.linters():
         linter_config = config.linter_config(linter)
         try:
+            linter = linter.split('_#')[0]
             classname = linter.capitalize()
             log.debug("Attempting to import 'lintreview.tools.%s'", linter)
             mod = __import__('lintreview.tools.' + linter, fromlist='*')
@@ -263,6 +270,8 @@ def process_checkstyle(problems, xml, filename_converter):
             message = err.get('message')
             if ',' in line:
                 lines = [int(x) for x in line.split(',')]
+            elif not line.isdigit():
+                continue
             else:
                 lines = [int(line)]
             list(map(lambda x: problems.add(filename, x, message), lines))

lintreview/tools/eslint.py

@@ -6,6 +6,7 @@
 from lintreview.review import IssueComment
 from lintreview.tools import Tool, run_command, process_checkstyle
 from lintreview.utils import in_path, npm_exists
+from lintreview.config import comma_value
 
 log = logging.getLogger(__name__)
 
@@ -17,14 +18,17 @@ class Eslint(Tool):
     def check_dependencies(self):
         """See if ESLint is on the system path.
         """
-        return in_path('eslint') or npm_exists('eslint')
+        working_dir = self.get_working_dir()
+        return in_path('eslint') or npm_exists('eslint', cwd=working_dir)
 
     def match_file(self, filename):
         """Check if a file should be linted using ESLint.
         """
         base = os.path.basename(filename)
         name, ext = os.path.splitext(base)
-        return ext == '.js' or ext == '.jsx'
+        extensions = comma_value(self.options.get('extensions', '.js,.jsx'))
+        log.debug('Using extensions %s', extensions)
+        return ext in extensions
 
     def has_fixer(self):
         """Eslint has a fixer that can be enabled
@@ -36,12 +40,25 @@ def process_files(self, files):
         """Run code checks with ESLint.
         """
         log.debug('Processing %s files with %s', files, self.name)
+
+        working_dir = self.get_working_dir()
+        log.debug('Working in dir %s', working_dir)
+
+        if self.options.get('install'):
+            output = run_command(
+                ['npm', 'install'],
+                ignore_error=True,
+                cwd=working_dir)
+            log.debug('Install output: %s', output)
+
         command = self._create_command()
 
         command += files
         output = run_command(
             command,
-            ignore_error=True)
+            ignore_error=True,
+            cwd=working_dir)
+
         self._process_output(output, files)
 
     def process_fixer(self, files):
@@ -51,7 +68,8 @@ def process_fixer(self, files):
         output = run_command(
             command,
             ignore_error=True,
-            include_errors=False)
+            include_errors=False,
+            cwd=self.get_working_dir())
         log.debug(output)
 
     def create_fixer_command(self, files):
@@ -62,8 +80,9 @@ def create_fixer_command(self, files):
 
     def _create_command(self):
         cmd = 'eslint'
-        if npm_exists('eslint'):
-            cmd = os.path.join(os.getcwd(), 'node_modules', '.bin', 'eslint')
+        working_dir = self.get_working_dir()
+        if npm_exists('eslint', cwd=working_dir):
+            cmd = os.path.join(working_dir, 'node_modules', '.bin', 'eslint')
         command = [cmd, '--format', 'checkstyle']
 
         # Add config file or default to recommended linters
@@ -72,6 +91,7 @@ def _create_command(self):
         return command
 
     def _process_output(self, output, files):
+        log.debug("Linting output:\n%s", output)
         if '<?xml' not in output:
             return self._config_error(output)
 

lintreview/tools/phpcs.py

@@ -22,7 +22,8 @@ def check_dependencies(self):
         """
         See if PHPCS is on the system path.
         """
-        return in_path('phpcs') or composer_exists('phpcs')
+        working_dir = self.get_working_dir()
+        return in_path('phpcs') or composer_exists('phpcs', cwd=working_dir)
 
     def match_file(self, filename):
         base = os.path.basename(filename)
@@ -36,11 +37,24 @@ def process_files(self, files):
         to save resources.
         """
         log.debug('Processing %s files with %s', files, self.name)
+
+        working_dir = self.get_working_dir()
+        log.debug('Working in dir %s', working_dir)
+
+        if self.options.get('install'):
+            output = run_command(
+                ['composer', 'install'],
+                ignore_error=True,
+                cwd=working_dir)
+            log.debug('Install output: %s', output)
+
         command = self.create_command(files)
         output = run_command(
             command,
             ignore_error=True,
-            include_errors=False)
+            include_errors=False,
+            cwd=working_dir)
+
         filename_converter = functools.partial(
             self._relativize_filename,
             files)
@@ -66,7 +80,7 @@ def apply_base(self, path):
 
     def create_command(self, files):
         command = ['phpcs']
-        if composer_exists('phpcs'):
+        if composer_exists('phpcs', cwd=self.get_working_dir()):
             command = ['vendor/bin/phpcs']
         command += ['--report=checkstyle']
         command = self._apply_options(command)
@@ -106,11 +120,12 @@ def process_fixer(self, files):
         run_command(
             command,
             ignore_error=True,
-            include_errors=False)
+            include_errors=False,
+            cwd=self.get_working_dir())
 
     def create_fixer_command(self, files):
         command = ['phpcbf']
-        if composer_exists('phpcbf'):
+        if composer_exists('phpcbf', cwd=self.get_working_dir()):
             command = ['vendor/bin/phpcbf']
         command = self._apply_options(command)
         command += files

lintreview/utils.py

@@ -19,26 +19,28 @@ def in_path(name):
     return False
 
 
-def npm_exists(name):
+def npm_exists(name, cwd=None):
     """
     Check whether or not a cli tool exists in a node_modules/.bin
     dir in os.cwd
 
     @return boolean
     """
-    cwd = os.getcwd()
+    if cwd is None:
+        cwd = os.getcwd()
     path = os.path.join(cwd, 'node_modules', '.bin', name)
     return os.path.exists(path)
 
 
-def composer_exists(name):
+def composer_exists(name, cwd=None):
     """
     Check whether or not a cli tool exists in vendor/bin/{name}
     relative to os.cwd
 
     @return boolean
     """
-    cwd = os.getcwd()
+    if cwd is None:
+        cwd = os.getcwd()
     path = os.path.join(cwd, 'vendor', 'bin', name)
     return os.path.exists(path)
 

lintreview/web.py

@@ -54,6 +54,8 @@ def start_review():
     try:
         lintrc = get_lintrc(gh, head_repo_ref)
         log.debug("lintrc file contents '%s'", lintrc)
+        if not lintrc:
+            raise Exception('lintrc does not exist or is empty')
     except Exception as e:
         log.warn("Cannot download .lintrc file for '%s', "
                  "skipping lint checks.", base_repo_url)

requirements-linters.txt

@@ -0,0 +1,8 @@
+jinja2<2.9
+pep8>=1.7.0,<2.0.0
+flake8>=3.3.0,<3.4.0
+yamllint>=1.6.1,<1.7.0
+demjson>=2.2.3,<2.3.0
+ansible-lint>=3.4.11,<3.5.0
+pylint>=1.7.4,<2.0.0
+autopep8>=1.3.0,<2.0.0