pre-upload: Add a check for tabbed indents for ebuild,eclass.

I have :set expandtab in vim. Maybe you do too!
ebuild/eclass files want tabbed indents. So let's catch that in
pre-upload rather than annoying reviewers.

BUG=chromium:676699
TEST=(1) (new) unittest.
     (2) Upload a CL in some ebuild/eclass with/without space indent.

Change-Id: If9d9b3e3cfd253960a1d593e9374b4e8664bc8c8
Reviewed-on: https://chromium-review.googlesource.com/423003
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index 7677605..be40f96 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python2
+#!/usr/bin/env python2
 # Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
@@ -394,6 +394,29 @@
   return _run_command(['git', 'log', '--format=%s%n%n%b', commit + '^!'])
 
 
+def _check_lines_in_diff(commit, files, check_callable, error_description):
+  """Checks given file for errors via the given check.
+
+  This is a convenience function for common per-line checks. It goes through all
+  files and returns a HookFailure with the error description listing all the
+  failures.
+
+  Args:
+    commit: The commit we're working on.
+    files: The files to check.
+    check_callable: A callable that takes a line and returns True if this line
+        _fails_ the check.
+    error_description: A string describing the error.
+  """
+  errors = []
+  for afile in files:
+    for line_num, line in _get_file_diff(afile, commit):
+      if check_callable(line):
+        errors.append('%s, line %s' % (afile, line_num))
+  if errors:
+    return HookFailure(error_description, errors)
+
+
 # Common Hooks
 
 
@@ -427,16 +450,12 @@
 
 def _check_no_stray_whitespace(_project, commit):
   """Checks that there is no stray whitespace at source lines end."""
-  errors = []
   files = _filter_files(_get_affected_files(commit),
                         COMMON_INCLUDED_PATHS,
                         COMMON_EXCLUDED_PATHS)
-  for afile in files:
-    for line_num, line in _get_file_diff(afile, commit):
-      if line.rstrip() != line:
-        errors.append('%s, line %s' % (afile, line_num))
-    if errors:
-      return HookFailure('Found line ending with white space in:', errors)
+  return _check_lines_in_diff(commit, files,
+                              lambda line: line.rstrip() != line,
+                              'Found line ending with white space in:')
 
 
 def _check_no_tabs(_project, commit):
@@ -449,17 +468,29 @@
       r".*\.mk$"
   ]
 
-  errors = []
   files = _filter_files(_get_affected_files(commit),
                         COMMON_INCLUDED_PATHS,
                         COMMON_EXCLUDED_PATHS + TAB_OK_PATHS)
+  return _check_lines_in_diff(commit, files,
+                              lambda line: '\t' in line,
+                              'Found a tab character in:')
 
-  for afile in files:
-    for line_num, line in _get_file_diff(afile, commit):
-      if '\t' in line:
-        errors.append('%s, line %s' % (afile, line_num))
-  if errors:
-    return HookFailure('Found a tab character in:', errors)
+
+def _check_tabbed_indents(_project, commit):
+  """Checks that indents use tabs only."""
+  TABS_REQUIRED_PATHS = [
+      r".*\.ebuild$",
+      r".*\.eclass$",
+  ]
+  LEADING_SPACE_RE = re.compile('[\t]* ')
+
+  files = _filter_files(_get_affected_files(commit),
+                        TABS_REQUIRED_PATHS,
+                        COMMON_EXCLUDED_PATHS)
+  return _check_lines_in_diff(
+      commit, files,
+      lambda line: LEADING_SPACE_RE.match(line) is not None,
+      'Found a space in indentation (must be all tabs):')
 
 
 def _check_gofmt(_project, commit):
@@ -1293,6 +1324,7 @@
     _check_no_long_lines,
     _check_no_stray_whitespace,
     _check_no_tabs,
+    _check_tabbed_indents,
     _check_portage_make_use_var,
 ]
 
@@ -1316,6 +1348,7 @@
     'cros_license_check': _check_cros_license,
     'aosp_license_check': _check_aosp_license,
     'tab_check': _check_no_tabs,
+    'tabbed_indent_required_check': _check_tabbed_indents,
     'branch_check': _check_change_has_branch_field,
     'signoff_check': _check_change_has_signoff_field,
     'bug_field_check': _check_change_has_bug_field,
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 830b46e..c8efbc1 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python2
+#!/usr/bin/env python2
 # -*- coding: utf-8 -*-
 # Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -78,6 +78,42 @@
                       failure.items)
 
 
+class CheckTabbedIndentsTest(cros_test_lib.MockTestCase):
+  """Tests for _check_tabbed_indents."""
+  def setUp(self):
+    self.PatchObject(pre_upload, '_get_affected_files', return_value=['x.py'])
+    self.PatchObject(pre_upload, '_filter_files', return_value=['x.py'])
+    self.diff_mock = self.PatchObject(pre_upload, '_get_file_diff')
+
+  def test_good_cases(self):
+    self.diff_mock.return_value = [
+        (1, u'no_tabs_anywhere'),
+        (2, u'	leading_tab_only'),
+        (3, u'	leading_tab another_tab'),
+        (4, u'	leading_tab trailing_too	'),
+        (5, u'	leading_tab  then_spaces_trailing  '),
+    ]
+    failure = pre_upload._check_tabbed_indents(ProjectNamed('PROJECT'),
+                                               'COMMIT')
+    self.assertIsNone(failure)
+
+  def test_bad_cases(self):
+    self.diff_mock.return_value = [
+        (1, u' leading_space'),
+        (2, u'	 tab_followed_by_space'),
+        (3, u'  			space_followed_by_tab'),
+        (4, u'   			  mix_em_up'),
+        (5, u'	  		  			mix_on_both_sides			   '),
+    ]
+    failure = pre_upload._check_tabbed_indents(ProjectNamed('PROJECT'),
+                                               'COMMIT')
+    self.assertTrue(failure)
+    self.assertEquals('Found a space in indentation (must be all tabs):',
+                      failure.msg)
+    self.assertEquals(['x.py, line %d' % line for line in xrange(1, 6)],
+                      failure.items)
+
+
 class CheckProjectPrefix(cros_test_lib.MockTempDirTestCase):
   """Tests for _check_project_prefix."""