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 re
-
-
 _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.')
 
 
+def _CheckJavaScriptStyle(input_api, output_api):
+  """Check for JavaScript style violations."""
+
+  import closure_linter.common.errorhandler
+
+  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 bool(self._errors)
+
+    def _valid(self, error):
+      """Check whether an error is valid. Most errors are valid, with a few
+         exceptions which are listed here.
+      """
+
+      import closure_linter.errors
+
+      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

[Tyler Breisacher (Chromium), 2012/02/06 22:00:13]

What is typically done in cases like this to avoid the 80char issue? 'import
closure_linter.errors as cl_errors' or something like that?

+      ]
+
+  import closure_linter.checker
+  import os.path
+
+  # 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 input_api.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(), start=1):
+      if 'getElementById' in line:
+        errorLines.append('  line %d: %s\n%s' % (
+            i,
+            'Use $() instead of document.getElementById()',
+            line))
+
+      const_re = input_api.re.compile(r'\bconst\b')
+      if const_re.search(line):
+        errorLines.append('  line %d: %s\n%s' % (
+            i,
+            'Use |var| instead of |const|. See http://crbug.com/80149',
+            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))
   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 106 matching lines @@
   return results
 
 
 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()):
+  if any(input_api.re.search(aura_re, f.LocalPath())
+      for f in change.AffectedFiles()):
     preferred.append('linux_chromeos')
   return preferred