Modify presubmit checks to work on changed lines only.

Unittest is being also updated.

TEST=manual
. run ./presubmit_unittest.py, observe success
.create a CL with code style violations (long lines, traling spaces) and observe the violations reported by 'git cl presubmit'

Patch contributed by Vadim

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

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

presubmit_support.py

@@ -86,11 +86,9 @@ def PromptYesNo(input_stream, output_stream, prompt):
 def _RightHandSideLinesImpl(affected_files):
   """Implements RightHandSideLines for InputApi and GclChange."""
   for af in affected_files:
-    lines = af.NewContents()
-    line_number = 0
+    lines = af.ChangedContents()
     for line in lines:
-      line_number += 1
-      yield (af, line_number, line)
+      yield (af, line[0], line[1])
 
 
 class OutputApi(object):
@@ -188,7 +186,7 @@ class InputApi(object):
   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$",
+      r".*\.inl$", r".*\.asm$", r".*\.hxx$", r".*\.hpp$", r".*\.s$", r".*\.S$",
       # Scripts
       r".*\.js$", r".*\.py$", r".*\.sh$", r".*\.rb$", r".*\.pl$", r".*\.pm$",
       # No extension at all, note that ALL CAPS files are black listed in
@@ -209,7 +207,7 @@ class InputApi(object):
       r".*\bxcodebuild[\\\/].*",
       r".*\bsconsbuild[\\\/].*",
       # All caps files like README and LICENCE.
-      r".*\b[A-Z0-9_]+$",
+      r".*\b[A-Z0-9_]{2,}$",
       # SCM (can happen in dual SCM configuration). (Slightly over aggressive)
       r"(|.*[\\\/])\.git[\\\/].*",
       r"(|.*[\\\/])\.svn[\\\/].*",
@@ -345,8 +343,8 @@ class InputApi(object):
     Note: Copy-paste this function to suit your needs or use a lambda function.
     """
     def Find(affected_file, items):
+      local_path = affected_file.LocalPath()
       for item in items:
-        local_path = affected_file.LocalPath()
         if self.re.match(item, local_path):
           logging.debug("%s matched %s" % (item, local_path))
           return True
@@ -481,9 +479,36 @@ class AffectedFile(object):
     """
     raise NotImplementedError()  # Implement if/when needed.
 
+  def ChangedContents(self):
+    """Returns a list of tuples (line number, line text) of all new lines.
+
+     This relies on the scm diff output describing each changed code section
+     with a line of the form
+
+     ^@@ <old line num>,<old size> <new line num>,<new size> @@$
+    """
+    new_lines = []
+    line_num = 0
+
+    if self.IsDirectory():
+      return []
+
+    for line in self.GenerateScmDiff().splitlines():
+      m = re.match(r'^@@ [0-9\,\+\-]+ \+([0-9]+)\,[0-9]+ @@', line)
+      if m:
+        line_num = int(m.groups(1)[0])
+        continue
+      if line.startswith('+') and not line.startswith('++'):
+        new_lines.append((line_num, line[1:]))
+      if not line.startswith('-'):
+        line_num += 1
+    return new_lines
+
   def __str__(self):
     return self.LocalPath()
 
+  def GenerateScmDiff(self):
+    raise NotImplementedError()  # Implemented in derived classes.
 
 class SvnAffectedFile(AffectedFile):
   """Representation of a file in a change out of a Subversion checkout."""
@@ -532,6 +557,8 @@ class SvnAffectedFile(AffectedFile):
         self._is_text_file = (not mime_type or mime_type.startswith('text/'))
     return self._is_text_file
 
+  def GenerateScmDiff(self):
+    return scm.SVN.GenerateDiff(self.AbsoluteLocalPath())
 
 class GitAffectedFile(AffectedFile):
   """Representation of a file in a change out of a git checkout."""
@@ -577,6 +604,8 @@ class GitAffectedFile(AffectedFile):
         self._is_text_file = os.path.isfile(self.AbsoluteLocalPath())
     return self._is_text_file
 
+  def GenerateScmDiff(self):
+    return scm.GIT.GenerateDiff(self._local_root, files=[self.LocalPath(),])
 
 class Change(object):
   """Describe a change.

tests/presubmit_unittest.py

@@ -37,6 +37,65 @@ def GetPreferredTrySlaves():
   return %s
 """
 
+  presubmit_diffs = """
+--- file1       2011-02-09 10:38:16.517224845 -0800
++++ file2       2011-02-09 10:38:53.177226516 -0800
+@@ -1,6 +1,5 @@
+ this is line number 0
+ this is line number 1
+-this is line number 2 to be deleted
+ this is line number 3
+ this is line number 4
+ this is line number 5
+@@ -8,7 +7,7 @@
+ this is line number 7
+ this is line number 8
+ this is line number 9
+-this is line number 10 to be modified
++this is line number 10
+ this is line number 11
+ this is line number 12
+ this is line number 13
+@@ -21,9 +20,8 @@
+ this is line number 20
+ this is line number 21
+ this is line number 22
+-this is line number 23
+-this is line number 24
+-this is line number 25
++this is line number 23.1
++this is line number 25.1
+ this is line number 26
+ this is line number 27
+ this is line number 28
+@@ -31,6 +29,7 @@
+ this is line number 30
+ this is line number 31
+ this is line number 32
++this is line number 32.1
+ this is line number 33
+ this is line number 34
+ this is line number 35
+@@ -38,14 +37,14 @@
+ this is line number 37
+ this is line number 38
+ this is line number 39
+-
+ this is line number 40
+-this is line number 41
++this is line number 41.1
+ this is line number 42
+ this is line number 43
+ this is line number 44
+ this is line number 45
++
+ this is line number 46
+ this is line number 47
+-this is line number 48
++this is line number 48.1
+ this is line number 49
+"""
+
   def setUp(self):
     SuperMoxTestBase.setUp(self)
     self.mox.StubOutWithMock(presubmit, 'random')
@@ -56,6 +115,7 @@ def GetPreferredTrySlaves():
     self.mox.StubOutWithMock(presubmit.scm.SVN, 'GetFileProperty')
     self.mox.StubOutWithMock(presubmit.gclient_utils, 'FileRead')
     self.mox.StubOutWithMock(presubmit.gclient_utils, 'FileWrite')
+    self.mox.StubOutWithMock(presubmit.scm.SVN, 'GenerateDiff')
 
 
 class PresubmitUnittest(PresubmitTestsBase):
@@ -197,8 +257,9 @@ class PresubmitUnittest(PresubmitTestsBase):
     presubmit.scm.SVN.CaptureInfo(notfound).AndReturn({})
     presubmit.scm.SVN.CaptureInfo(flap).AndReturn(
             {'URL': 'svn:/foo/boo/flap.h'})
-    presubmit.gclient_utils.FileRead(blat, 'rU').AndReturn('boo!\nahh?')
-    presubmit.gclient_utils.FileRead(notfound, 'rU').AndReturn('look!\nthere?')
+    presubmit.scm.SVN.GenerateDiff(blat).AndReturn(self.presubmit_diffs)
+    presubmit.scm.SVN.GenerateDiff(notfound).AndReturn(self.presubmit_diffs)
+
     self.mox.ReplayAll()
 
     change = presubmit.SvnChange('mychange', '\n'.join(description_lines),
@@ -242,20 +303,24 @@ class PresubmitUnittest(PresubmitTestsBase):
     for line in change.RightHandSideLines():
       rhs_lines.append(line)
     self.assertEquals(rhs_lines[0][0].LocalPath(), files[0][1])
-    self.assertEquals(rhs_lines[0][1], 1)
-    self.assertEquals(rhs_lines[0][2],'boo!')
+    self.assertEquals(rhs_lines[0][1], 10)
+    self.assertEquals(rhs_lines[0][2],'this is line number 10')
+
+    self.assertEquals(rhs_lines[3][0].LocalPath(), files[0][1])
+    self.assertEquals(rhs_lines[3][1], 32)
+    self.assertEquals(rhs_lines[3][2], 'this is line number 32.1')
 
-    self.assertEquals(rhs_lines[1][0].LocalPath(), files[0][1])
-    self.assertEquals(rhs_lines[1][1], 2)
-    self.assertEquals(rhs_lines[1][2], 'ahh?')
+    self.assertEquals(rhs_lines[8][0].LocalPath(), files[3][1])
+    self.assertEquals(rhs_lines[8][1], 23)
+    self.assertEquals(rhs_lines[8][2], 'this is line number 23.1')
 
-    self.assertEquals(rhs_lines[2][0].LocalPath(), files[3][1])
-    self.assertEquals(rhs_lines[2][1], 1)
-    self.assertEquals(rhs_lines[2][2], 'look!')
+    self.assertEquals(rhs_lines[12][0].LocalPath(), files[3][1])
+    self.assertEquals(rhs_lines[12][1], 46)
+    self.assertEquals(rhs_lines[12][2], '')
 
-    self.assertEquals(rhs_lines[3][0].LocalPath(), files[3][1])
-    self.assertEquals(rhs_lines[3][1], 2)
-    self.assertEquals(rhs_lines[3][2], 'there?')
+    self.assertEquals(rhs_lines[13][0].LocalPath(), files[3][1])
+    self.assertEquals(rhs_lines[13][1], 49)
+    self.assertEquals(rhs_lines[13][2], 'this is line number 48.1')
 
   def testExecPresubmitScript(self):
     description_lines = ('Hello there',
@@ -711,10 +776,9 @@ class InputApiUnittest(PresubmitTestsBase):
     presubmit.scm.SVN.GetFileProperty(another, 'svn:mime-type').AndReturn(None)
     presubmit.scm.SVN.GetFileProperty(third_party, 'svn:mime-type'
         ).AndReturn(None)
-    presubmit.gclient_utils.FileRead(blat, 'rU'
-                                     ).AndReturn('whatever\ncookie')
-    presubmit.gclient_utils.FileRead(another, 'rU'
-                                     ).AndReturn('whatever\ncookie2')
+    presubmit.scm.SVN.GenerateDiff(blat).AndReturn(self.presubmit_diffs)
+    presubmit.scm.SVN.GenerateDiff(another).AndReturn(self.presubmit_diffs)
+
     self.mox.ReplayAll()
 
     change = presubmit.SvnChange('mychange', '\n'.join(description_lines),
@@ -737,14 +801,14 @@ class InputApiUnittest(PresubmitTestsBase):
     # binary isn't a text file and beingdeleted doesn't exist. The rest is
     # outside foo/.
     rhs_lines = [x for x in input_api.RightHandSideLines(None)]
-    self.assertEquals(len(rhs_lines), 4)
+    self.assertEquals(len(rhs_lines), 14)
     self.assertEqual(rhs_lines[0][0].LocalPath(),
                      presubmit.normpath(files[0][1]))
-    self.assertEqual(rhs_lines[1][0].LocalPath(),
+    self.assertEqual(rhs_lines[3][0].LocalPath(),
                      presubmit.normpath(files[0][1]))
-    self.assertEqual(rhs_lines[2][0].LocalPath(),
+    self.assertEqual(rhs_lines[7][0].LocalPath(),
                      presubmit.normpath(files[4][1]))
-    self.assertEqual(rhs_lines[3][0].LocalPath(),
+    self.assertEqual(rhs_lines[13][0].LocalPath(),
                      presubmit.normpath(files[4][1]))
 
   def testDefaultWhiteListBlackListFilters(self):
@@ -758,11 +822,13 @@ class InputApiUnittest(PresubmitTestsBase):
           f('experimental/b'),
           f('a/experimental'),
           f('a/experimental.cc'),
+          f('a/experimental.S'),
         ],
         [
           # Expected.
           'a/experimental',
           'a/experimental.cc',
+          'a/experimental.S',
         ],
       ),
       (
@@ -808,7 +874,7 @@ class InputApiUnittest(PresubmitTestsBase):
     input_api = presubmit.InputApi(None, './PRESUBMIT.py', False)
     self.mox.ReplayAll()
 
-    self.assertEqual(len(input_api.DEFAULT_WHITE_LIST), 20)
+    self.assertEqual(len(input_api.DEFAULT_WHITE_LIST), 22)
     self.assertEqual(len(input_api.DEFAULT_BLACK_LIST), 9)
     for item in files:
       results = filter(input_api.FilterSourceFile, item[0])
@@ -1018,8 +1084,9 @@ class AffectedFileUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'AbsoluteLocalPath', 'Action', 'IsDirectory', 'IsTextFile', 'LocalPath',
-      'NewContents', 'OldContents', 'OldFileTempPath', 'Property', 'ServerPath',
+      'AbsoluteLocalPath', 'Action', 'ChangedContents', 'GenerateScmDiff',
+      'IsDirectory', 'IsTextFile', 'LocalPath', 'NewContents', 'OldContents',
+      'OldFileTempPath', 'Property', 'ServerPath',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(presubmit.AffectedFile('a', 'b'), members)