Add CheckChangeHasOnlyOneEol and CheckChangeHasNoCrAndHasOnlyOneEol.

TEST=unit tests
BUG=none

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

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

presubmit_canned_checks.py

@@ -67,14 +67,56 @@ def CheckDoNotSubmitInFiles(input_api, output_api):
 
 
 def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None):
-  """Checks that there are no \r, \r\n (CR or CRLF) characters in any of the
-  source files to be submitted.
-  """
-  outputs = []
+  """Checks no '\r' (CR) character is in any source files."""
+  cr_files = []
   for f in input_api.AffectedSourceFiles(source_file_filter):
     if '\r' in input_api.ReadFile(f, 'rb'):
+      cr_files.append(f.LocalPath())
+  if cr_files:
+    return [output_api.PresubmitPromptWarning(
+        "Found a CR character in these files:", items=cr_files)]
+  return []
+
+
+def CheckChangeHasOnlyOneEol(input_api, output_api, source_file_filter=None):
+  """Checks the files ends with one and only one \n (LF)."""
+  eof_files = []
+  for f in input_api.AffectedSourceFiles(source_file_filter):
+    contents = input_api.ReadFile(f, 'rb')
+    # Check that the file ends in one and only one newline character.
+    if len(contents) > 1 and (contents[-1:] != "\n" or contents[-2:-1] == "\n"):
+      eof_files.append(f.LocalPath())
+
+  if eof_files:
+    return [output_api.PresubmitPromptWarning(
+      'These files should end in one (and only one) newline character:',
+      items=eof_files)]
+  return []
+
+
+def CheckChangeHasNoCrAndHasOnlyOneEol(input_api, output_api,
+                                       source_file_filter=None):
+  """Runs both CheckChangeHasNoCR and CheckChangeHasOnlyOneEOL in one pass.
+
+  It is faster because it is reading the file only once.
+  """
+  cr_files = []
+  eof_files = []
+  for f in input_api.AffectedSourceFiles(source_file_filter):
+    contents = input_api.ReadFile(f, 'rb')
+    if '\r' in contents:
+      cr_files.append(f.LocalPath())
+    # Check that the file ends in one and only one newline character.
+    if len(contents) > 1 and (contents[-1:] != "\n" or contents[-2:-1] == "\n"):
+      eof_files.append(f.LocalPath())
+  outputs = []
+  if cr_files:
     outputs.append(output_api.PresubmitPromptWarning(
-          "Found a CR character in %s" % f.LocalPath()))
+        "Found a CR character in these files:", items=cr_files))
+  if eof_files:
+    outputs.append(output_api.PresubmitPromptWarning(
+      'These files should end in one (and only one) newline character:',
+      items=eof_files))
   return outputs
 
 
@@ -82,11 +124,13 @@ 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.
   """
+  tabs = []
   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" %
-          (f.LocalPath(), line_num))]
+      tabs.append("%s, line %s" % (f.LocalPath(), line_num))
+  if tabs:
+    return [output_api.PresubmitPromptWarning("Found a tab character in:",
+                                              long_text="\n".join(tabs))]
   return []
 
 

tests/presubmit_unittest.py

@@ -937,7 +937,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-      'CheckChangeHasBugField', 'CheckChangeHasNoCR', 'CheckChangeHasNoTabs',
+      'CheckChangeHasBugField', 'CheckChangeHasOnlyOneEol',
+      'CheckChangeHasNoCR', 'CheckChangeHasNoCrAndHasOnlyOneEol',
+      'CheckChangeHasNoTabs',
       'CheckChangeHasQaField', 'CheckChangeHasTestedField',
       'CheckChangeHasTestField', 'CheckChangeSvnEolStyle',
       'CheckDoNotSubmit',
@@ -987,6 +989,28 @@ class CannedChecksUnittest(PresubmitTestsBase):
     self.assertEquals(len(results2), 1)
     self.assertEquals(results2[0].__class__, error_type)
 
+  def ReadFileTest(self, check, content1, content2, error_type):
+    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.AffectedSourceFiles(None).AndReturn([affected_file1])
+    input_api1.ReadFile(affected_file1, 'rb').AndReturn(content1)
+    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.AffectedSourceFiles(None).AndReturn([affected_file2])
+    input_api2.ReadFile(affected_file2, 'rb').AndReturn(content2)
+    affected_file2.LocalPath().AndReturn('bar.cc')
+    self.mox.ReplayAll()
+
+    results = check(input_api1, presubmit.OutputApi)
+    self.assertEquals(results, [])
+    results2 = check(input_api2, presubmit.OutputApi)
+    self.assertEquals(len(results2), 1)
+    self.assertEquals(results2[0].__class__, error_type)
+  
   def testCannedCheckChangeHasBugField(self):
     self.DescriptionTest(presubmit_canned_checks.CheckChangeHasBugField,
                          'BUG=1234', '',
@@ -1018,29 +1042,25 @@ class CannedChecksUnittest(PresubmitTestsBase):
         'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT',
         presubmit.OutputApi.PresubmitError)
 
+  def testCheckChangeHasOnlyOneEol(self):
+    self.ReadFileTest(presubmit_canned_checks.CheckChangeHasOnlyOneEol,
+                      "Hey!\nHo!\n", "Hey!\nHo!\n\n",
+                      presubmit.OutputApi.PresubmitPromptWarning)
+
   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.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.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()
+    self.ReadFileTest(presubmit_canned_checks.CheckChangeHasNoCR,
+                      "Hey!\nHo!\n", "Hey!\r\nHo!\r\n",
+                      presubmit.OutputApi.PresubmitPromptWarning)
  
-    results = presubmit_canned_checks.CheckChangeHasNoCR(
-        input_api1, presubmit.OutputApi, None)
-    self.assertEquals(results, [])
-    results2 = presubmit_canned_checks.CheckChangeHasNoCR(
-        input_api2, presubmit.OutputApi, None)
-    self.assertEquals(len(results2), 1)
-    self.assertEquals(results2[0].__class__,
+  def testCheckChangeHasNoCrAndHasOnlyOneEol(self):
+    self.ReadFileTest(
+        presubmit_canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol,
+        "Hey!\nHo!\n", "Hey!\nHo!\n\n",
+        presubmit.OutputApi.PresubmitPromptWarning)
+    self.mox.VerifyAll()
+    self.ReadFileTest(
+        presubmit_canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol,
+        "Hey!\nHo!\n", "Hey!\r\nHo!\r\n",
         presubmit.OutputApi.PresubmitPromptWarning)
 
   def testCannedCheckChangeHasNoTabs(self):