PRESUBMIT check for JavaScript style errors

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 06c92efa89f1cdef939cd6818d8f0ee615b9042e..c70e458917fb2b28b157f5e5a6e124bce2b67269 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -9,8 +9,11 @@ for more details about the presubmit API built into gcl.
 """
 
 
+import closure_linter.checker
+import closure_linter.common.errorhandler
+import closure_linter.errors
 import re
-
+import os.path
 
 _EXCLUDED_PATHS = (
     r"^breakpad[\\\/].*",
@@ -29,7 +32,98 @@ _TEST_ONLY_WARNING = (
     'not perfect.  The commit queue will not block on this warning.\n'
     'Email joi@chromium.org if you have questions.')
 
+class ErrorHandlerImpl(closure_linter.common.errorhandler.ErrorHandler):
+  '''Implementation of ErrorHandler that collects all errors except those
+     that don't apply for Chromium JavaScript code.
+  '''
+
+  def __init__(self):
+    self._errors = []
+
+  def HandleFile(self, filename, first_token):
+    self._filename = filename
+
+  def HandleError(self, error):
+    if (self._valid(error)):
+      error.filename = self._filename
+      self._errors.append(error)
+
+  def GetErrors(self):
+    return self._errors
+
+  def HasErrors(self):
+    return not self._errors.empty
+
+  def _valid(self, error):
+    '''Check whether an error is valid. Most errors are valid, with a few
+       exceptions which are listed here.
+    '''
+    return error.code not in [
+        closure_linter.errors.COMMA_AT_END_OF_LITERAL,
+        closure_linter.errors.JSDOC_ILLEGAL_QUESTION_WITH_PIPE,
+        closure_linter.errors.JSDOC_TAG_DESCRIPTION_ENDS_WITH_INVALID_CHARACTER
+    ]
+
+def _CheckJavaScriptStyle(input_api, output_api):
+  """Check for JavaScript style violations."""
 
+  # Only check the following folders. OWNERS of folders containing JavaScript
+  # code can opt-in to this check by adding the folder here.
+  checked_folders = [
+      os.path.join('chrome', 'browser', 'resources', 'ntp4'),
+      os.path.join('chrome', 'browser', 'resources', 'options2'),
+  ]
+
+  def inCheckedFolder(affected_file):
+    return any(affected_file.LocalPath().startswith(cf)
+        for cf in checked_folders)
+
+  def jsOrHtml(affected_file):
+    return re.search('\.(js|html?)$', affected_file.LocalPath())
+
+  def fileFilter(affected_file):
+    return jsOrHtml(affected_file) and inCheckedFolder(affected_file)
+
+  results = []
+
+  for f in input_api.change.AffectedFiles(file_filter=fileFilter):
+    errorLines = []
+
+    # check for getElementById()
+    for i, line in enumerate(f.NewContents()):
+      if 'getElementById' in line:
+        errorLines.append('  line %d: %s\n%s' % (
+            i,
+            'Use $() instead of document.getElementById()',
+            line))
+
+    # Use closure_linter to check for several different errors
+    error_handler = ErrorHandlerImpl()
+    checker = closure_linter.checker.JavaScriptStyleChecker(error_handler)
+    checker.Check(f.LocalPath())
+
+    for error in error_handler.GetErrors():
+      errorMsg = '  line %d: E%04d: %s\n%s' % (
+          error.token.line_number,
+          error.code,
+          error.message,
+          error.token.line)
+      errorLines.append(errorMsg)
+
+    if errorLines:
+      errorLines = [
+          'Found JavaScript style violations in %s:' %
+          f.LocalPath()] + errorLines
+      results.append(output_api.PresubmitError('\n'.join(errorLines)))
+
+  if results:
+    results.append(output_api.PresubmitNotifyResult(
+        'See the JavaScript style guide at '
+        'http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml'
+        ' and contact tbreisacher@chromium.org for feedback on this'
+        ' PRESUBMIT check.'))
+
+  return results
 
 def _CheckNoInterfacesInBase(input_api, output_api):
   """Checks to make sure no files in libbase.a have |@interface|."""
@@ -214,6 +308,7 @@ def _CheckNoNewOldCallback(input_api, output_api):
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
+  results.extend(_CheckJavaScriptStyle(input_api, output_api))

[Dan Beam, 2012/02/03 03:09:13]

Damn, I guess I should be doing my checks in this file instead of in
depot_tools, d'oh!

   results.extend(input_api.canned_checks.PanProjectChecks(
       input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
   results.extend(_CheckNoInterfacesInBase(input_api, output_api))