Add InputApi.AffectedSourceFile() and custom filtering support.

TEST=none
BUG=none

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

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

presubmit_canned_checks.py

@@ -6,6 +6,8 @@
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 
+### Description checks
+
 def CheckChangeHasTestField(input_api, output_api):
   """Requires that the changelist have a TEST= field."""
   if input_api.change.TEST:
@@ -51,40 +53,36 @@ def CheckDoNotSubmitInDescription(input_api, output_api):
     return []
 
 
+### Content checks
+
 def CheckDoNotSubmitInFiles(input_api, output_api):
   """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to any files."""
   keyword = 'DO NOT ' + 'SUBMIT'
-  for f, line_num, line in input_api.RightHandSideLines():
+  # We want to check every text files, not just source files.
+  for f, line_num, line in input_api.RightHandSideLines(lambda x: x):
     if keyword in line:
       text = 'Found ' + keyword + ' in %s, line %s' % (f.LocalPath(), line_num)
       return [output_api.PresubmitError(text)]
   return []
 
 
-def CheckDoNotSubmit(input_api, output_api):
+def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None):
-  return (
-      CheckDoNotSubmitInDescription(input_api, output_api) +
-      CheckDoNotSubmitInFiles(input_api, output_api)
-      )
-
-
-def CheckChangeHasNoCR(input_api, output_api):
   """Checks that there are no \r, \r\n (CR or CRLF) characters in any of the
-  text files to be submitted.
+  source files to be submitted.
   """
   outputs = []
-  for f in input_api.AffectedTextFiles():
+  for f in input_api.AffectedSourceFiles(source_file_filter):
     if '\r' in input_api.ReadFile(f, 'rb'):
       outputs.append(output_api.PresubmitPromptWarning(
           "Found a CR character in %s" % f.LocalPath()))
   return outputs
 
 
-def CheckChangeHasNoTabs(input_api, output_api):
+def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None):
   """Checks that there are no tab characters in any of the text files to be
   submitted.
   """
-  for f, line_num, line in input_api.RightHandSideLines():
+  for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
     if '\t' in line:
       return [output_api.PresubmitPromptWarning(
           "Found a tab character in %s, line %s" %
@@ -92,12 +90,12 @@ def CheckChangeHasNoTabs(input_api, output_api):
   return []
 
 
-def CheckLongLines(input_api, output_api, maxlen=80):
+def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
   """Checks that there aren't any lines longer than maxlen characters in any of
   the text files to be submitted.
   """
   bad = []
-  for f, line_num, line in input_api.RightHandSideLines():
+  for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
     # Allow lines with http://, https:// and #define/#pragma/#include/#if/#endif
     # to exceed the maxlen rule.
     if (len(line) > maxlen and
@@ -121,6 +119,15 @@ def CheckLongLines(input_api, output_api, maxlen=80):
     return []
 
 
+### Other checks
+
+def CheckDoNotSubmit(input_api, output_api):
+  return (
+      CheckDoNotSubmitInDescription(input_api, output_api) +
+      CheckDoNotSubmitInFiles(input_api, output_api)
+      )
+
+
 def CheckTreeIsOpen(input_api, output_api, url, closed):
   """Checks that an url's content doesn't match a regexp that would mean that
   the tree is closed."""
@@ -145,6 +152,7 @@ def _RunPythonUnitTests_LoadTests(input_api, module_name):
     module = getattr(module, part)
   return input_api.unittest.TestLoader().loadTestsFromModule(module)._tests
 
+
 def RunPythonUnitTests(input_api, output_api, unit_tests):
   """Imports the unit_tests modules and run them."""
   # We don't want to hinder users from uploading incomplete patches.

presubmit_support.py

@@ -137,6 +137,37 @@ class InputApi(object):
   know stuff about the change they're looking at.
   """
 
+  # File extensions that are considered source files from a style guide
+  # perspective. Don't modify this list from a presubmit script!
+  DEFAULT_WHITE_LIST = (
+      # C++ and friends
+      r".*\.c", r".*\.cc", r".*\.cpp", r".*\.h", r".*\.m", r".*\.mm",
+      r".*\.inl", r".*\.asm", r".*\.hxx", r".*\.hpp",
+      # Scripts
+      r".*\.js", r".*\.py", r".*\.json", r".*\.sh", r".*\.rb",
+      # No extension at all
+      r"(^|.*[\\\/])[^.]+$",
+      # Other
+      r".*\.java", r".*\.mk", r".*\.am",
+  )
+
+  # Path regexp that should be excluded from being considered containing source
+  # files. Don't modify this list from a presubmit script!
+  DEFAULT_BLACK_LIST = (
+      r".*\bexperimental[\\\/].*",
+      r".*\bthird_party[\\\/].*",
+      # Output directories (just in case)
+      r".*\bDebug[\\\/].*",
+      r".*\bRelease[\\\/].*",
+      r".*\bxcodebuild[\\\/].*",
+      r".*\bsconsbuild[\\\/].*",
+      # All caps files like README and LICENCE.
+      r".*\b[A-Z0-9_]+",
+      # SCM (can happen in dual SCM configuration)
+      r".*\b\.git[\\\/].*",
+      r".*\b\.svn[\\\/].*",
+  )
+
   def __init__(self, change, presubmit_path, is_committing):
     """Builds an InputApi object.
 
@@ -250,7 +281,35 @@ class InputApi(object):
     return filter(lambda x: x.IsTextFile(),
                   self.AffectedFiles(include_dirs=False, include_deletes=False))
 
-  def RightHandSideLines(self):
+  def FilterSourceFile(self, affected_file, white_list=None, black_list=None):
+    """Filters out files that aren't considered "source file".
+
+    If white_list or black_list is None, InputApi.DEFAULT_WHITE_LIST
+    and InputApi.DEFAULT_BLACK_LIST is used respectively.
+
+    The lists will be compiled as regular expression and
+    AffectedFile.LocalPath() needs to pass both list.
+
+    Note: Copy-paste this function to suit your needs or use a lambda function.
+    """
+    def Find(affected_file, list):
+      for item in list:
+        if self.re.match(item, affected_file.LocalPath()):
+          return True
+      return False
+    return (Find(affected_file, white_list or self.DEFAULT_WHITE_LIST) and
+            not Find(affected_file, black_list or self.DEFAULT_BLACK_LIST))
+
+  def AffectedSourceFiles(self, source_file):
+    """Filter the list of AffectedTextFiles by the function source_file.
+
+    If source_file is None, InputApi.FilterSourceFile() is used.
+    """
+    if not source_file:
+      source_file = self.FilterSourceFile
+    return filter(source_file, self.AffectedTextFiles())
+
+  def RightHandSideLines(self, source_file_filter=None):
     """An iterator over all text lines in "new" version of changed files.
 
     Only lists lines from new or modified text files in the change that are
@@ -267,9 +326,8 @@ class InputApi(object):
 
     Note: The cariage return (LF or CR) is stripped off.
     """
-    return InputApi._RightHandSideLinesImpl(
+    files = self.AffectedSourceFiles(source_file_filter)
-        filter(lambda x: x.IsTextFile(),
+    return InputApi._RightHandSideLinesImpl(files)
-               self.AffectedFiles(include_deletes=False)))
 
   def ReadFile(self, file, mode='r'):
     """Reads an arbitrary file.

tests/presubmit_unittest.py

@@ -60,17 +60,19 @@ def CheckChangeOnUpload(input_api, output_api):
   def setUp(self):
     mox.MoxTestBase.setUp(self)
     self.mox.StubOutWithMock(presubmit, 'warnings')
-    # Stub out 'os' but keep os.path.dirname/join/normpath and os.sep.
+    # Stub out 'os' but keep os.path.dirname/join/normpath/splitext and os.sep.
     os_sep = presubmit.os.sep
     os_path_join = presubmit.os.path.join
     os_path_dirname = presubmit.os.path.dirname
     os_path_normpath = presubmit.os.path.normpath
+    os_path_splitext = presubmit.os.path.splitext
     self.mox.StubOutWithMock(presubmit, 'os')
     self.mox.StubOutWithMock(presubmit.os, 'path')
     presubmit.os.sep = os_sep
     presubmit.os.path.join = os_path_join
     presubmit.os.path.dirname = os_path_dirname
     presubmit.os.path.normpath = os_path_normpath
+    presubmit.os.path.splitext = os_path_splitext
     self.mox.StubOutWithMock(presubmit, 'sys')
     # Special mocks.
     def MockAbsPath(f):
@@ -523,9 +525,12 @@ class InputApiUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles',
+      'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedSourceFiles',
-      'DepotToLocalPath', 'LocalPaths', 'LocalToDepotPath',
+      'AffectedTextFiles',
-      'PresubmitLocalPath', 'RightHandSideLines', 'ReadFile', 'ServerPaths',
+      'DEFAULT_BLACK_LIST', 'DEFAULT_WHITE_LIST',
+      'DepotToLocalPath', 'FilterSourceFile', 'LocalPaths',
+      'LocalToDepotPath',
+      'PresubmitLocalPath', 'ReadFile', 'RightHandSideLines', 'ServerPaths',
       'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change',
       'is_committing', 'marshal', 'os_path', 'pickle', 'platform',
       're', 'subprocess', 'tempfile', 'traceback', 'unittest', 'urllib2',
@@ -573,24 +578,30 @@ class InputApiUnittest(PresubmitTestsBase):
                          'BUG=123',
                          ' STORY =http://foo/  \t',
                          'and some more regular text')
+    blat = join('foo', 'blat.cc')
+    readme = join('foo', 'blat', 'READ_ME2')
+    binary = join('foo', 'blat', 'binary.dll')
+    weird = join('foo', 'blat', 'weird.xyz')
+    third_party = join('foo', 'third_party', 'third.cc')
+    another = join('foo', 'blat', 'another.h')
+    beingdeleted = join('foo', 'mat', 'beingdeleted.txt')
+    notfound = join('flop', 'notfound.txt')
+    flap = join('boo', 'flap.h')
     files = [
-      ['A', join('foo', 'blat.cc')],
+      ['A', blat],
-      ['M', join('foo', 'blat', 'binary.dll')],
+      ['M', readme],
+      ['M', binary],
+      ['M', weird],
+      ['M', another],
+      ['M', third_party],
       ['D', 'foo/mat/beingdeleted.txt'],
       ['M', 'flop/notfound.txt'],
       ['A', 'boo/flap.h'],
     ]
 
-  
+    for i in (blat, readme, binary, weird, another, third_party):
-    blat = join('foo', 'blat.cc')
+      presubmit.os.path.exists(i).AndReturn(True)
-    binary = join('foo', 'blat', 'binary.dll')
+      presubmit.os.path.isdir(i).AndReturn(False)
-    beingdeleted = join('foo', 'mat', 'beingdeleted.txt')
-    notfound = join('flop', 'notfound.txt')
-    flap = join('boo', 'flap.h')
-    presubmit.os.path.exists(blat).AndReturn(True)
-    presubmit.os.path.isdir(blat).AndReturn(False)
-    presubmit.os.path.exists(binary).AndReturn(True)
-    presubmit.os.path.isdir(binary).AndReturn(False)
     presubmit.os.path.exists(beingdeleted).AndReturn(False)
     presubmit.os.path.exists(notfound).AndReturn(False)
     presubmit.os.path.exists(flap).AndReturn(True)
@@ -598,30 +609,80 @@ class InputApiUnittest(PresubmitTestsBase):
     presubmit.gclient.CaptureSVNInfo(beingdeleted).AndReturn({})
     presubmit.gclient.CaptureSVNInfo(notfound).AndReturn({})
     presubmit.gcl.GetSVNFileProperty(blat, 'svn:mime-type').AndReturn(None)
+    presubmit.gcl.GetSVNFileProperty(readme, 'svn:mime-type').AndReturn(None)
     presubmit.gcl.GetSVNFileProperty(binary, 'svn:mime-type').AndReturn(
         'application/octet-stream')
+    presubmit.gcl.GetSVNFileProperty(weird, 'svn:mime-type').AndReturn(None)
+    presubmit.gcl.GetSVNFileProperty(another, 'svn:mime-type').AndReturn(None)
+    presubmit.gcl.GetSVNFileProperty(third_party, 'svn:mime-type'
+        ).AndReturn(None)
     presubmit.gcl.ReadFile(blat).AndReturn('whatever\ncookie')
+    presubmit.gcl.ReadFile(another).AndReturn('whatever\ncookie2')
     self.mox.ReplayAll()
 
     ci = presubmit.gcl.ChangeInfo(name='mychange', issue=0, patchset=0,
                                   description='\n'.join(description_lines),
                                   files=files)
     change = presubmit.GclChange(ci)
-    api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', False)
+    input_api = presubmit.InputApi(change, 'foo/PRESUBMIT.py', False)
-    affected_files = api.AffectedFiles()
+    # Doesn't filter much
-    self.assertEquals(len(affected_files), 3)
+    got_files = input_api.AffectedFiles()
-    self.assertEquals(affected_files[0].LocalPath(),
+    self.assertEquals(len(got_files), 7)
-                      presubmit.normpath('foo/blat.cc'))
+    self.assertEquals(got_files[0].LocalPath(), presubmit.normpath(blat))
-    self.assertEquals(affected_files[1].LocalPath(),
+    self.assertEquals(got_files[1].LocalPath(), presubmit.normpath(readme))
-                      presubmit.normpath('foo/blat/binary.dll'))
+    self.assertEquals(got_files[2].LocalPath(), presubmit.normpath(binary))
-    self.assertEquals(affected_files[2].LocalPath(),
+    self.assertEquals(got_files[3].LocalPath(), presubmit.normpath(weird))
-                      presubmit.normpath('foo/mat/beingdeleted.txt'))
+    self.assertEquals(got_files[4].LocalPath(), presubmit.normpath(another))
-    rhs_lines = []
+    self.assertEquals(got_files[5].LocalPath(), presubmit.normpath(third_party))
-    for line in api.RightHandSideLines():
+    self.assertEquals(got_files[6].LocalPath(),
-      rhs_lines.append(line)
+                      presubmit.normpath(beingdeleted))
-    self.assertEquals(len(rhs_lines), 2)
+    # Ignores weird because of whitelist, third_party because of blacklist,
-    self.assertEqual(rhs_lines[0][0].LocalPath(),
+    # binary isn't a text file and beingdeleted doesn't exist. The rest is
-                    presubmit.normpath('foo/blat.cc'))
+    # outside foo/.
+    rhs_lines = [x for x in input_api.RightHandSideLines(None)]
+    self.assertEquals(len(rhs_lines), 4)
+    self.assertEqual(rhs_lines[0][0].LocalPath(), presubmit.normpath(blat))
+    self.assertEqual(rhs_lines[1][0].LocalPath(), presubmit.normpath(blat))
+    self.assertEqual(rhs_lines[2][0].LocalPath(), presubmit.normpath(another))
+    self.assertEqual(rhs_lines[3][0].LocalPath(), presubmit.normpath(another))
+
+  def testCustomFilter(self):
+    def FilterSourceFile(affected_file):
+      return 'a' in affected_file.LocalPath()
+    files = [('A', 'eeaee'), ('M', 'eeabee'), ('M', 'eebcee')]
+    for (action, item) in files:
+      presubmit.os.path.exists(item).AndReturn(True)
+      presubmit.os.path.isdir(item).AndReturn(False)
+      presubmit.gcl.GetSVNFileProperty(item, 'svn:mime-type').AndReturn(None)
+    self.mox.ReplayAll()
+
+    ci = presubmit.gcl.ChangeInfo('mychange', 0, 0, '', files)
+    change = presubmit.GclChange(ci)
+    input_api = presubmit.InputApi(change, './PRESUBMIT.py', False)
+    got_files = input_api.AffectedSourceFiles(FilterSourceFile)
+    self.assertEquals(len(got_files), 2)
+    self.assertEquals(got_files[0].LocalPath(), 'eeaee')
+    self.assertEquals(got_files[1].LocalPath(), 'eeabee')
+
+  def testLambdaFilter(self):
+    white_list = presubmit.InputApi.DEFAULT_BLACK_LIST + (r".*?a.*?",)
+    black_list = [r".*?b.*?"]
+    files = [('A', 'eeaee'), ('M', 'eeabee'), ('M', 'eebcee'), ('M', 'eecaee')]
+    for (action, item) in files:
+      presubmit.os.path.exists(item).AndReturn(True)
+      presubmit.os.path.isdir(item).AndReturn(False)
+      presubmit.gcl.GetSVNFileProperty(item, 'svn:mime-type').AndReturn(None)
+    self.mox.ReplayAll()
+
+    ci = presubmit.gcl.ChangeInfo('mychange', 0, 0, '', files)
+    change = presubmit.GclChange(ci)
+    input_api = presubmit.InputApi(change, './PRESUBMIT.py', False)
+    # Sample usage of overiding the default white and black lists.
+    got_files = input_api.AffectedSourceFiles(
+        lambda x: input_api.FilterSourceFile(x, white_list, black_list))
+    self.assertEquals(len(got_files), 2)
+    self.assertEquals(got_files[0].LocalPath(), 'eeaee')
+    self.assertEquals(got_files[1].LocalPath(), 'eecaee')
 
   def testGetAbsoluteLocalPath(self):
     join = presubmit.os.path.join
@@ -905,7 +966,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
       (affected_file, 43, 'yer'),
       (affected_file, 23, 'ya'),
     ]
-    input_api1.RightHandSideLines().AndReturn(output1)
+    input_api1.RightHandSideLines(mox.IgnoreArg()).AndReturn(output1)
     input_api2 = self.MockInputApi()
     input_api2.change = self.MakeBasicChange('foo', 'Foo\n')
     output2 = [
@@ -913,12 +974,12 @@ class CannedChecksUnittest(PresubmitTestsBase):
       (affected_file, 43, 'yer'),
       (affected_file, 23, 'ya'),
     ]
-    input_api2.RightHandSideLines().AndReturn(output2)
+    input_api2.RightHandSideLines(mox.IgnoreArg()).AndReturn(output2)
     self.mox.ReplayAll()
 
-    results1 = check(input_api1, presubmit.OutputApi)
+    results1 = check(input_api1, presubmit.OutputApi, None)
     self.assertEquals(results1, [])
-    results2 = check(input_api2, presubmit.OutputApi)
+    results2 = check(input_api2, presubmit.OutputApi, None)
     self.assertEquals(len(results2), 1)
     self.assertEquals(results2[0].__class__, error_type)
 
@@ -948,31 +1009,32 @@ class CannedChecksUnittest(PresubmitTestsBase):
                          presubmit.OutputApi.PresubmitError)
 
   def testCannedCheckDoNotSubmitInFiles(self):
-    self.TestContent(presubmit_canned_checks.CheckDoNotSubmitInFiles,
+    self.TestContent(
-                     'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT',
+        lambda x,y,z: presubmit_canned_checks.CheckDoNotSubmitInFiles(x, y),
-                     presubmit.OutputApi.PresubmitError)
+        'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT',
+        presubmit.OutputApi.PresubmitError)
 
   def testCheckChangeHasNoCR(self):
     input_api1 = self.MockInputApi()
     self.mox.StubOutWithMock(input_api1, 'ReadFile')
     input_api1.change = self.MakeBasicChange('foo', 'Foo\n')
     affected_file1 = self.mox.CreateMock(presubmit.SvnAffectedFile)
-    input_api1.AffectedTextFiles().AndReturn([affected_file1])
+    input_api1.AffectedSourceFiles(None).AndReturn([affected_file1])
     input_api1.ReadFile(affected_file1, 'rb').AndReturn("Hey!\nHo!\n")
     input_api2 = self.MockInputApi()
     self.mox.StubOutWithMock(input_api2, 'ReadFile')
     input_api2.change = self.MakeBasicChange('foo', 'Foo\n')
     affected_file2 = self.mox.CreateMock(presubmit.SvnAffectedFile)
-    input_api2.AffectedTextFiles().AndReturn([affected_file2])
+    input_api2.AffectedSourceFiles(None).AndReturn([affected_file2])
     input_api2.ReadFile(affected_file2, 'rb').AndReturn("Hey!\r\nHo!\r\n")
     affected_file2.LocalPath().AndReturn('bar.cc')
     self.mox.ReplayAll()
 
     results = presubmit_canned_checks.CheckChangeHasNoCR(
-        input_api1, presubmit.OutputApi)
+        input_api1, presubmit.OutputApi, None)
     self.assertEquals(results, [])
     results2 = presubmit_canned_checks.CheckChangeHasNoCR(
-        input_api2, presubmit.OutputApi)
+        input_api2, presubmit.OutputApi, None)
     self.assertEquals(len(results2), 1)
     self.assertEquals(results2[0].__class__,
                       presubmit.OutputApi.PresubmitPromptWarning)
@@ -983,7 +1045,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
                      presubmit.OutputApi.PresubmitPromptWarning)
 
   def testCannedCheckLongLines(self):
-    check = lambda x,y: presubmit_canned_checks.CheckLongLines(x, y, 10)
+    check = lambda x,y,z: presubmit_canned_checks.CheckLongLines(x, y, 10, z)
     self.TestContent(check, '', 'blah blah blah',
                      presubmit.OutputApi.PresubmitPromptWarning)