diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 21875f61af..6c4a04ce89 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -1041,6 +1041,8 @@ def CommonChecks(input_api, output_api): results.extend(CheckApiDepsFileIsUpToDate(input_api, output_api)) results.extend( CheckAbslMemoryInclude(input_api, output_api, non_third_party_sources)) + results.extend( + CheckAssertUsage(input_api, output_api, non_third_party_sources)) results.extend( CheckBannedAbslMakeUnique(input_api, output_api, non_third_party_sources)) @@ -1158,6 +1160,28 @@ def CheckObjcApiSymbols(input_api, output_api, source_file_filter): return [] +def CheckAssertUsage(input_api, output_api, source_file_filter): + pattern = input_api.re.compile(r'\bassert\(') + file_filter = lambda f: (f.LocalPath().endswith(('.cc', '.h', '.m', '.mm')) + and source_file_filter(f)) + + files = [] + for f in input_api.AffectedFiles(include_deletes=False, + file_filter=file_filter): + for _, line in f.ChangedContents(): + if pattern.search(line): + files.append(f.LocalPath()) + break + + if len(files): + return [ + output_api.PresubmitError( + 'Usage of assert() has been detected in the following files, ' + 'please use RTC_DCHECK() instead.\n Files:', files) + ] + return [] + + def CheckAbslMemoryInclude(input_api, output_api, source_file_filter): pattern = input_api.re.compile(r'^#include\s*"absl/memory/memory.h"', input_api.re.MULTILINE) diff --git a/presubmit_test.py b/presubmit_test.py index bb93765f28..e7879f99f7 100755 --- a/presubmit_test.py +++ b/presubmit_test.py @@ -271,5 +271,69 @@ class CheckNoMixingSourcesTest(unittest.TestCase): f.write(content) +class CheckAssertUsageTest(unittest.TestCase): + def setUp(self): + self.input_api = MockInputApi() + self.output_api = MockOutputApi() + self._content_with_assert = [ + 'void Foo() {', + ' assert(true);', + '}' + ] + self._content_without_assert = [ + 'void Foo() {', + ' RTC_CHECK(true);', + '}' + ] + + def testDetectsAssertInCcFile(self): + self.input_api.files = [ + MockFile('with_assert.cc', self._content_with_assert), + MockFile('without_assert.cc', self._content_without_assert), + ] + errors = PRESUBMIT.CheckAssertUsage( + self.input_api, self.output_api, lambda x: True) + self.assertEqual(1, len(errors)) + self.assertEqual('with_assert.cc', errors[0].items[0]) + + def testDetectsAssertInHeaderFile(self): + self.input_api.files = [ + MockFile('with_assert.h', self._content_with_assert), + MockFile('without_assert.h', self._content_without_assert), + ] + errors = PRESUBMIT.CheckAssertUsage( + self.input_api, self.output_api, lambda x: True) + self.assertEqual(1, len(errors)) + self.assertEqual('with_assert.h', errors[0].items[0]) + + def testDetectsAssertInObjCFile(self): + self.input_api.files = [ + MockFile('with_assert.m', self._content_with_assert), + MockFile('without_assert.m', self._content_without_assert), + ] + errors = PRESUBMIT.CheckAssertUsage( + self.input_api, self.output_api, lambda x: True) + self.assertEqual(1, len(errors)) + self.assertEqual('with_assert.m', errors[0].items[0]) + + def testDetectsAssertInObjCppFile(self): + self.input_api.files = [ + MockFile('with_assert.mm', self._content_with_assert), + MockFile('without_assert.mm', self._content_without_assert), + ] + errors = PRESUBMIT.CheckAssertUsage( + self.input_api, self.output_api, lambda x: True) + self.assertEqual(1, len(errors)) + self.assertEqual('with_assert.mm', errors[0].items[0]) + + def testDoesntDetectAssertInOtherFiles(self): + self.input_api.files = [ + MockFile('with_assert.cpp', self._content_with_assert), + ] + errors = PRESUBMIT.CheckAssertUsage( + self.input_api, self.output_api, lambda x: True) + self.assertEqual(0, len(errors)) + + if __name__ == '__main__': unittest.main() diff --git a/presubmit_test_mocks.py b/presubmit_test_mocks.py index b15eb74dd8..4ed7947530 100644 --- a/presubmit_test_mocks.py +++ b/presubmit_test_mocks.py @@ -24,13 +24,18 @@ class MockInputApi(object): self.change = MockChange([], []) self.files = [] self.presubmit_local_path = os.path.dirname(__file__) + self.re = re # pylint: disable=invalid-name def AffectedSourceFiles(self, file_filter=None): return self.AffectedFiles(file_filter=file_filter) def AffectedFiles(self, file_filter=None, include_deletes=False): - # pylint: disable=unused-argument - return self.files + for f in self.files: + if file_filter and not file_filter(f): + continue + if not include_deletes and f.Action() == 'D': + continue + yield f @classmethod def FilterSourceFile(cls,