PRESUBMIT check for JavaScript style errors

PRESUBMIT.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Top-level presubmit script for Chromium.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 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[\\\/].*",
     r"^native_client_sdk[\\\/].*",
     r"^net[\\\/]tools[\\\/]spdyshark[\\\/].*",
     r"^skia[\\\/].*",
     r"^v8[\\\/].*",
     r".*MakeFile$",
 )
 
 
 _TEST_ONLY_WARNING = (
     'You might be calling functions intended only for testing from\n'
     'production code.  It is OK to ignore this warning if you know what\n'
     'you are doing, as the heuristics used to detect the situation are\n'
     '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|."""
   pattern = input_api.re.compile(r'^\s*@interface', input_api.re.MULTILINE)
   files = []
   for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
     if (f.LocalPath().startswith('base/') and
         not f.LocalPath().endswith('_unittest.mm')):
       contents = input_api.ReadFile(f)
       if pattern.search(contents):
@@ skipping 164 matching lines @@
   if not problems:
     return []
   return [output_api.PresubmitPromptWarning('The old callback system is '
       'deprecated. If possible, use base::Bind and base::Callback instead.\n' +
       '\n'.join(problems))]
 
 
 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))
   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
   results.extend(
     _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
   results.extend(_CheckNoNewWStrings(input_api, output_api))
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
@@ skipping 109 matching lines @@
 def GetPreferredTrySlaves(project, change):
   only_objc_files = all(
       f.LocalPath().endswith(('.mm', '.m')) for f in change.AffectedFiles())
   if only_objc_files:
     return ['mac_rel']
   preferred = ['win_rel', 'linux_rel', 'mac_rel']
   aura_re = '_aura[^/]*[.][^/]*'
   if any(re.search(aura_re, f.LocalPath()) for f in change.AffectedFiles()):
     preferred.append('linux_chromeos_aura:compile')
   return preferred