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."""