Bug 1360595 - Add a git pre-commit hook for running ESLint. r?Mossop draft
authorMark Banner <standard8@mozilla.com>
Fri, 28 Apr 2017 12:19:15 +0100
changeset 572155 8b741664e70044c056cd97713c746194909b372d
parent 571858 aa248eeef420a9fd5075d5635e539d4b4514d58b
child 626958 c2e8ac6fbdcd9995ff37a810d9d047fd474055fc
push id57007
push usermbanner@mozilla.com
push dateWed, 03 May 2017 20:32:29 +0000
reviewersMossop
bugs1360595
milestone55.0a1
Bug 1360595 - Add a git pre-commit hook for running ESLint. r?Mossop MozReview-Commit-ID: 1YJL5Sd4dlb
tools/git/eslintvalidate.py
tools/git/git-lint-commit-hook.sh
tools/lint/eslint/hook_helper.py
tools/lint/flake8.lint.py
tools/mercurial/eslintvalidate.py
new file mode 100755
--- /dev/null
+++ b/tools/git/eslintvalidate.py
@@ -0,0 +1,35 @@
+#!/usr/bin/python
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+import os
+import sys
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "lint", "eslint"))
+from hook_helper import is_lintable, runESLint
+
+
+def output(message):
+    print >> sys.stderr, message
+
+
+def eslint():
+    f = os.popen('git diff --cached --name-only --diff-filter=ACM')
+
+    files = [file for file in f.read().splitlines() if is_lintable(file)]
+
+    if len(files) == 0:
+        return True
+
+    print "Running ESLint..."
+
+    return runESLint(output, files)
+
+
+if __name__ == '__main__':
+    if not eslint():
+        output("Note: ESLint failed, but the commit will still happen. "
+               "Please fix before pushing.")
+
+    # We output successfully as that seems to be a better model. See
+    # https://bugzilla.mozilla.org/show_bug.cgi?id=1230300#c4 for more
+    # information.
new file mode 100755
--- /dev/null
+++ b/tools/git/git-lint-commit-hook.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# Simple wrapper to try and ensure we get the right python version loaded.
+which python2.7 > /dev/null
+
+if [ $? -eq 0 ]
+then
+  python2.7 ./tools/git/eslintvalidate.py
+else
+  python ./tools/git/eslintvalidate.py
+fi
+
+exit $?
copy from tools/mercurial/eslintvalidate.py
copy to tools/lint/eslint/hook_helper.py
--- a/tools/mercurial/eslintvalidate.py
+++ b/tools/lint/eslint/hook_helper.py
@@ -1,75 +1,84 @@
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
+# This file provides helper functions for the repository hooks for git/hg.
+
 import os
 import re
 import json
 from subprocess import check_output, CalledProcessError
 
 lintable = re.compile(r'.+\.(?:js|jsm|jsx|xml|html)$')
 ignored = 'File ignored because of a matching ignore pattern. Use "--no-ignore" to override.'
 
 
 def is_lintable(filename):
+    """Determine if a file is lintable based on the file extension.
+
+    Keyword arguments:
+    filename -- the file to check.
+    """
     return lintable.match(filename)
 
 
-def display(ui, output):
-    results = json.loads(output)
+def display(print_func, output_json):
+    """Formats an ESLint result into a human readable message.
+
+    Keyword arguments:
+    print_func -- A function to call to print the output.
+    output_json -- the json ESLint results to format.
+    """
+    results = json.loads(output_json)
     for file in results:
         path = os.path.relpath(file["filePath"])
         for message in file["messages"]:
             if message["message"] == ignored:
                 continue
 
             if "line" in message:
-                ui.warn("%s:%d:%d %s\n" % (path, message["line"], message["column"],
-                        message["message"]))
+                print_func("%s:%d:%d %s\n" % (path, message["line"], message["column"],
+                           message["message"]))
             else:
-                ui.warn("%s: %s\n" % (path, message["message"]))
+                print_func("%s: %s\n" % (path, message["message"]))
 
 
-def eslinthook(ui, repo, node=None, **opts):
-    ctx = repo[node]
-    if len(ctx.parents()) > 1:
-        return 0
+def runESLint(print_func, files):
+    """Runs ESLint on the files that are passed.
 
-    deleted = repo.status(ctx.p1().node(), ctx.node()).deleted
-    files = [f for f in ctx.files() if f not in deleted and is_lintable(f)]
-
-    if len(files) == 0:
-        return
-
+    Keyword arguments:
+    print_func -- A function to call to print the output.
+    files -- A list of files to be checked.
+    """
     try:
         basepath = get_project_root()
 
         if not basepath:
             return
 
         dir = os.path.join(basepath, "node_modules", ".bin")
 
         eslint_path = os.path.join(dir, "eslint")
         if os.path.exists(os.path.join(dir, "eslint.cmd")):
             eslint_path = os.path.join(dir, "eslint.cmd")
         output = check_output([eslint_path,
                                "--format", "json", "--plugin", "html"] + files,
                               cwd=basepath)
-        display(ui, output)
+        display(print_func, output)
+        return True
     except CalledProcessError as ex:
-        display(ui, ex.output)
-        ui.warn("ESLint found problems in your changes, please correct them.\n")
-
-
-def reposetup(ui, repo):
-    ui.setconfig('hooks', 'commit.eslint', eslinthook)
+        display(print_func, ex.output)
+        return False
 
 
 def get_project_root():
+    """Returns the absolute path to the root of the project, by looking for the
+    mach executable.
+    """
     file_found = False
     folder = os.getcwd()
 
     while (folder):
         if os.path.exists(os.path.join(folder, 'mach')):
             file_found = True
             break
         else:
--- a/tools/lint/flake8.lint.py
+++ b/tools/lint/flake8.lint.py
@@ -182,16 +182,17 @@ LINTER = {
         'taskcluster',
         'testing/firefox-ui',
         'testing/marionette/client',
         'testing/marionette/harness',
         'testing/marionette/puppeteer',
         'testing/mozbase',
         'testing/mochitest',
         'testing/talos/',
+        'tools/git',
         'tools/lint',
         'tools/mercurial',
         'toolkit/components/telemetry',
     ],
     'exclude': ['testing/mochitest/pywebsocket'],
     'extensions': EXTENSIONS,
     'type': 'external',
     'payload': lint,
--- a/tools/mercurial/eslintvalidate.py
+++ b/tools/mercurial/eslintvalidate.py
@@ -1,81 +1,27 @@
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
 import os
-import re
-import json
-from subprocess import check_output, CalledProcessError
-
-lintable = re.compile(r'.+\.(?:js|jsm|jsx|xml|html)$')
-ignored = 'File ignored because of a matching ignore pattern. Use "--no-ignore" to override.'
-
-
-def is_lintable(filename):
-    return lintable.match(filename)
-
-
-def display(ui, output):
-    results = json.loads(output)
-    for file in results:
-        path = os.path.relpath(file["filePath"])
-        for message in file["messages"]:
-            if message["message"] == ignored:
-                continue
-
-            if "line" in message:
-                ui.warn("%s:%d:%d %s\n" % (path, message["line"], message["column"],
-                        message["message"]))
-            else:
-                ui.warn("%s: %s\n" % (path, message["message"]))
+import sys
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "lint", "eslint"))
+from hook_helper import is_lintable, runESLint
 
 
 def eslinthook(ui, repo, node=None, **opts):
     ctx = repo[node]
     if len(ctx.parents()) > 1:
         return 0
 
     deleted = repo.status(ctx.p1().node(), ctx.node()).deleted
     files = [f for f in ctx.files() if f not in deleted and is_lintable(f)]
 
     if len(files) == 0:
         return
 
-    try:
-        basepath = get_project_root()
-
-        if not basepath:
-            return
-
-        dir = os.path.join(basepath, "node_modules", ".bin")
-
-        eslint_path = os.path.join(dir, "eslint")
-        if os.path.exists(os.path.join(dir, "eslint.cmd")):
-            eslint_path = os.path.join(dir, "eslint.cmd")
-        output = check_output([eslint_path,
-                               "--format", "json", "--plugin", "html"] + files,
-                              cwd=basepath)
-        display(ui, output)
-    except CalledProcessError as ex:
-        display(ui, ex.output)
-        ui.warn("ESLint found problems in your changes, please correct them.\n")
+    if not runESLint(ui.warn, files):
+        ui.warn("Note: ESLint failed, but the commit will still happen. "
+                "Please fix before pushing.\n")
 
 
 def reposetup(ui, repo):
     ui.setconfig('hooks', 'commit.eslint', eslinthook)
-
-
-def get_project_root():
-    file_found = False
-    folder = os.getcwd()
-
-    while (folder):
-        if os.path.exists(os.path.join(folder, 'mach')):
-            file_found = True
-            break
-        else:
-            folder = os.path.dirname(folder)
-
-    if file_found:
-        return os.path.abspath(folder)
-
-    return None