diff options
| -rw-r--r-- | PRESUBMIT.py | 55 | ||||
| -rwxr-xr-x | PRESUBMIT_test.py | 47 |
2 files changed, 64 insertions, 38 deletions
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index f16d273..252b836 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -542,7 +542,7 @@ def _CheckIncludeOrderForScope(scope, input_api, file_path, changed_linenums): return warnings -def _CheckIncludeOrderInFile(input_api, f, is_source, changed_linenums): +def _CheckIncludeOrderInFile(input_api, f, changed_linenums): """Checks the #include order for the given file f.""" system_include_pattern = input_api.re.compile(r'\s*#include \<.*') @@ -550,34 +550,34 @@ def _CheckIncludeOrderInFile(input_api, f, is_source, changed_linenums): # often need to appear in a specific order. excluded_include_pattern = input_api.re.compile(r'\s*#include \<.*/.*') custom_include_pattern = input_api.re.compile(r'\s*#include "(?P<FILE>.*)"') - if_pattern = input_api.re.compile(r'\s*#\s*(if|elif|else|endif).*') + if_pattern = ( + input_api.re.compile(r'\s*#\s*(if|elif|else|endif|define|undef).*')) contents = f.NewContents() warnings = [] line_num = 0 - # Handle the special first include for source files. If the header file is - # some/path/file.h, the corresponding source file can be some/path/file.cc, - # some/other/path/file.cc, some/path/file_platform.cc etc. It's also possible - # that no special first include exists. - if is_source: - for line in contents: - line_num += 1 - if system_include_pattern.match(line): + # Handle the special first include. If the first include file is + # some/path/file.h, the corresponding including file can be some/path/file.cc, + # some/other/path/file.cc, some/path/file_platform.cc, some/path/file-suffix.h + # etc. It's also possible that no special first include exists. + for line in contents: + line_num += 1 + if system_include_pattern.match(line): + # No special first include -> process the line again along with normal + # includes. + line_num -= 1 + break + match = custom_include_pattern.match(line) + if match: + match_dict = match.groupdict() + header_basename = input_api.os_path.basename( + match_dict['FILE']).replace('.h', '') + if header_basename not in input_api.os_path.basename(f.LocalPath()): # No special first include -> process the line again along with normal # includes. line_num -= 1 - break - match = custom_include_pattern.match(line) - if match: - match_dict = match.groupdict() - header_basename = input_api.os_path.basename( - match_dict['FILE']).replace('.h', '') - if header_basename not in input_api.os_path.basename(f.LocalPath()): - # No special first include -> process the line again along with normal - # includes. - line_num -= 1 - break + break # Split into scopes: Each region between #if and #endif is its own scope. scopes = [] @@ -607,18 +607,15 @@ def _CheckIncludeOrder(input_api, output_api): 3. C++ system files in alphabetical order 4. Project's .h files in alphabetical order - Each region between #if and #endif follows these rules separately. + Each region separated by #if, #elif, #else, #endif, #define and #undef follows + these rules separately. """ warnings = [] for f in input_api.AffectedFiles(): - changed_linenums = set([line_num for line_num, _ in f.ChangedContents()]) - if f.LocalPath().endswith('.cc'): - warnings.extend(_CheckIncludeOrderInFile(input_api, f, True, - changed_linenums)) - elif f.LocalPath().endswith('.h'): - warnings.extend(_CheckIncludeOrderInFile(input_api, f, False, - changed_linenums)) + if f.LocalPath().endswith(('.cc', '.h')): + changed_linenums = set(line_num for line_num, _ in f.ChangedContents()) + warnings.extend(_CheckIncludeOrderInFile(input_api, f, changed_linenums)) results = [] if warnings: diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index debe33b..9bc7db5 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -28,13 +28,16 @@ class MockOutputApi(object): self.long_text = long_text class PresubmitError(PresubmitResult): - pass + def __init__(self, message, items, long_text=''): + MockOutputApi.PresubmitResult.__init__(self, message, items, long_text) class PresubmitPromptWarning(PresubmitResult): - pass + def __init__(self, message, items, long_text=''): + MockOutputApi.PresubmitResult.__init__(self, message, items, long_text) class PresubmitNotifyResult(PresubmitResult): - pass + def __init__(self, message, items, long_text=''): + MockOutputApi.PresubmitResult.__init__(self, message, items, long_text) class MockFile(object): @@ -120,7 +123,7 @@ class IncludeOrderTest(unittest.TestCase): '#include "a/header.h"'] mock_file = MockFile('some/path/foo.cc', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(0, len(warnings)) def testSpecialFirstInclude2(self): @@ -129,7 +132,7 @@ class IncludeOrderTest(unittest.TestCase): '#include "a/header.h"'] mock_file = MockFile('some/path/foo.cc', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(0, len(warnings)) def testSpecialFirstInclude3(self): @@ -138,7 +141,7 @@ class IncludeOrderTest(unittest.TestCase): '#include "a/header.h"'] mock_file = MockFile('some/path/foo_platform.cc', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(0, len(warnings)) def testSpecialFirstInclude4(self): @@ -147,10 +150,19 @@ class IncludeOrderTest(unittest.TestCase): '#include "a/header.h"'] mock_file = MockFile('some/path/foo_platform.cc', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(1, len(warnings)) self.assertTrue('2' in warnings[0]) + def testSpecialFirstInclude5(self): + mock_input_api = MockInputApi() + contents = ['#include "some/other/path/foo.h"', + '#include "a/header.h"'] + mock_file = MockFile('some/path/foo-suffix.h', contents) + warnings = PRESUBMIT._CheckIncludeOrderInFile( + mock_input_api, mock_file, range(1, len(contents) + 1)) + self.assertEqual(0, len(warnings)) + def testOrderAlreadyWrong(self): scope = [(1, '#include "b.h"'), (2, '#include "a.h"'), @@ -183,6 +195,10 @@ class IncludeOrderTest(unittest.TestCase): def testIfElifElseEndif(self): mock_input_api = MockInputApi() contents = ['#include "e.h"', + '#define foo', + '#include "f.h"', + '#undef foo', + '#include "e.h"', '#if foo', '#include "d.h"', '#elif bar', @@ -193,7 +209,7 @@ class IncludeOrderTest(unittest.TestCase): '#include "a.h"'] mock_file = MockFile('', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(0, len(warnings)) def testSysIncludes(self): @@ -203,9 +219,22 @@ class IncludeOrderTest(unittest.TestCase): '#include <sys/a.h>'] mock_file = MockFile('', contents) warnings = PRESUBMIT._CheckIncludeOrderInFile( - mock_input_api, mock_file, True, range(1, len(contents) + 1)) + mock_input_api, mock_file, range(1, len(contents) + 1)) self.assertEqual(0, len(warnings)) + def testCheckOnlyCFiles(self): + mock_input_api = MockInputApi() + mock_output_api = MockOutputApi() + contents = ['#include <b.h>', + '#include <a.h>'] + mock_file_cc = MockFile('something.cc', contents) + mock_file_h = MockFile('something.h', contents) + mock_file_other = MockFile('something.py', contents) + mock_input_api.files = [mock_file_cc, mock_file_h, mock_file_other] + warnings = PRESUBMIT._CheckIncludeOrder(mock_input_api, mock_output_api) + self.assertEqual(1, len(warnings)) + self.assertEqual(2, len(warnings[0].items)) + class VersionControlerConflictsTest(unittest.TestCase): def testTypicalConflict(self): |