Add RunPylint as a canned presubmit check.

Adding it as I figured out how to make it run correctly on ubuntu 10.4 and it's
used in enough places it warrants a canned check.

BUG=none
TEST=it self tests itself.

Review URL: http://codereview.chromium.org/5682011

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@69051 0039d316-1c4b-4281-b951-d872f2087c98

PRESUBMIT.py

@@ -19,27 +19,41 @@ UNIT_TESTS = [
   'tests.watchlists_unittest',
 ]
 
-def CheckChangeOnUpload(input_api, output_api):
+def CommonChecks(input_api, output_api):
   output = []
-  output.extend(input_api.canned_checks.RunPythonUnitTests(input_api,
-                                                           output_api,
-                                                           UNIT_TESTS))
+  output.extend(input_api.canned_checks.RunPythonUnitTests(
+      input_api,
+      output_api,
+      UNIT_TESTS))
   output.extend(WasGitClUploadHookModified(input_api, output_api))
-  output.extend(RunPylint(input_api, output_api))
+
+  def filter_python_sources(affected_file):
+    filepath = affected_file.LocalPath()
+    return ((filepath.endswith('.py') and
+             filepath != 'cpplint.py' and
+             not filepath.startswith('tests')) or
+            filepath == 'git-try')
+
+  output.extend(input_api.canned_checks.RunPylint(
+      input_api,
+      output_api,
+      source_file_filter=filter_python_sources))
   return output
 
 
+def CheckChangeOnUpload(input_api, output_api):
+  return CommonChecks(input_api, output_api)
+
+
 def CheckChangeOnCommit(input_api, output_api):
   output = []
-  output.extend(input_api.canned_checks.RunPythonUnitTests(input_api,
-                                                           output_api,
-                                                           UNIT_TESTS))
-  output.extend(input_api.canned_checks.CheckDoNotSubmit(input_api,
-                                                         output_api))
-  output.extend(WasGitClUploadHookModified(input_api, output_api))
-  output.extend(RunPylint(input_api, output_api))
+  output.extend(CommonChecks(input_api, output_api))
+  output.extend(input_api.canned_checks.CheckDoNotSubmit(
+      input_api,
+      output_api))
   return output
 
+
 def WasGitClUploadHookModified(input_api, output_api):
   for affected_file in input_api.AffectedSourceFiles(None):
     if (input_api.os_path.basename(affected_file.LocalPath()) ==
@@ -48,27 +62,3 @@ def WasGitClUploadHookModified(input_api, output_api):
           'Don\'t forget to fix git-cl to download the newest version of '
           'git-cl-upload-hook')]
   return []
-
-def RunPylint(input_api, output_api):
-  import glob
-  files = glob.glob('*.py')
-  # It's a python script
-  files.append('git-try')
-  # It uses non-standard pylint exceptions that makes pylint always fail.
-  files.remove('cpplint.py')
-  try:
-    proc = input_api.subprocess.Popen(['pylint'] + sorted(files))
-    proc.communicate()
-    if proc.returncode:
-      return [output_api.PresubmitError('Fix pylint errors first.')]
-    return []
-  except OSError:
-    if input_api.platform == 'win32':
-      return [output_api.PresubmitNotifyResult(
-        'Warning: Can\'t run pylint because it is not installed. Please '
-        'install manually\n'
-        'Cannot do static analysis of python files.')]
-    return [output_api.PresubmitError(
-        'Please install pylint with "sudo apt-get install python-setuptools; '
-        'sudo easy_install pylint"\n'
-        'Cannot do static analysis of python files.')]

presubmit_canned_checks.py

@@ -450,6 +450,36 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
   return []
 
 
+def RunPylint(input_api, output_api, source_file_filter=None):
+  """Run pylint on python files."""
+  import warnings
+  # On certain pylint/python version combination, running pylint throws a lot of
+  # warning messages.
+  warnings.filterwarnings('ignore', category=DeprecationWarning)
+  try:
+    if not source_file_filter:
+      source_file_filter = lambda f: f.LocalPath().endswith('.py')
+    files = [f.LocalPath()
+            for f in input_api.AffectedSourceFiles(source_file_filter)]
+    try:
+      from pylint import lint
+      if lint.Run(sorted(files)):
+        return [output_api.PresubmitPromptWarning('Fix pylint errors first.')]
+      return []
+    except ImportError:
+      if input_api.platform == 'win32':
+        return [output_api.PresubmitNotifyResult(
+          'Warning: Can\'t run pylint because it is not installed. Please '
+          'install manually\n'
+          'Cannot do static analysis of python files.')]
+      return [output_api.PresubmitError(
+          'Please install pylint with "sudo apt-get install python-setuptools; '
+          'sudo easy_install pylint"\n'
+          'Cannot do static analysis of python files.')]
+  finally:
+    warnings.filterwarnings('default', category=DeprecationWarning)
+
+
 def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms,
                                  owner):
   if not input_api.is_committing:

tests/presubmit_unittest.py

@@ -1137,6 +1137,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
       'CheckDoNotSubmit',
       'CheckDoNotSubmitInDescription', 'CheckDoNotSubmitInFiles',
       'CheckLongLines', 'CheckTreeIsOpen', 'RunPythonUnitTests',
+      'RunPylint',
       'CheckBuildbotPendingBuilds', 'CheckRietveldTryJobExecution',
     ]
     # If this test fails, you should add the relevant test.