mirror of
https://github.com/mollyim/webrtc.git
synced 2025-05-13 05:40:42 +01:00
Add presubmit check to guard against assert() usage.
Example of error reporting: Usage of assert() has been detected in the following files, please use RTC_DCHECK() instead. Files: rtc_base/thread.cc Bug: webrtc:6779 Change-Id: Iae08c3d7ddcc0449073752cadca19b3cf662892c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225549 Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/master@{#34532}
This commit is contained in:
parent
b0ed12099f
commit
a6395132ae
3 changed files with 95 additions and 2 deletions
24
PRESUBMIT.py
24
PRESUBMIT.py
|
@ -1041,6 +1041,8 @@ def CommonChecks(input_api, output_api):
|
||||||
results.extend(CheckApiDepsFileIsUpToDate(input_api, output_api))
|
results.extend(CheckApiDepsFileIsUpToDate(input_api, output_api))
|
||||||
results.extend(
|
results.extend(
|
||||||
CheckAbslMemoryInclude(input_api, output_api, non_third_party_sources))
|
CheckAbslMemoryInclude(input_api, output_api, non_third_party_sources))
|
||||||
|
results.extend(
|
||||||
|
CheckAssertUsage(input_api, output_api, non_third_party_sources))
|
||||||
results.extend(
|
results.extend(
|
||||||
CheckBannedAbslMakeUnique(input_api, output_api,
|
CheckBannedAbslMakeUnique(input_api, output_api,
|
||||||
non_third_party_sources))
|
non_third_party_sources))
|
||||||
|
@ -1158,6 +1160,28 @@ def CheckObjcApiSymbols(input_api, output_api, source_file_filter):
|
||||||
return []
|
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):
|
def CheckAbslMemoryInclude(input_api, output_api, source_file_filter):
|
||||||
pattern = input_api.re.compile(r'^#include\s*"absl/memory/memory.h"',
|
pattern = input_api.re.compile(r'^#include\s*"absl/memory/memory.h"',
|
||||||
input_api.re.MULTILINE)
|
input_api.re.MULTILINE)
|
||||||
|
|
|
@ -271,5 +271,69 @@ class CheckNoMixingSourcesTest(unittest.TestCase):
|
||||||
f.write(content)
|
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__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|
|
@ -24,13 +24,18 @@ class MockInputApi(object):
|
||||||
self.change = MockChange([], [])
|
self.change = MockChange([], [])
|
||||||
self.files = []
|
self.files = []
|
||||||
self.presubmit_local_path = os.path.dirname(__file__)
|
self.presubmit_local_path = os.path.dirname(__file__)
|
||||||
|
self.re = re # pylint: disable=invalid-name
|
||||||
|
|
||||||
def AffectedSourceFiles(self, file_filter=None):
|
def AffectedSourceFiles(self, file_filter=None):
|
||||||
return self.AffectedFiles(file_filter=file_filter)
|
return self.AffectedFiles(file_filter=file_filter)
|
||||||
|
|
||||||
def AffectedFiles(self, file_filter=None, include_deletes=False):
|
def AffectedFiles(self, file_filter=None, include_deletes=False):
|
||||||
# pylint: disable=unused-argument
|
for f in self.files:
|
||||||
return self.files
|
if file_filter and not file_filter(f):
|
||||||
|
continue
|
||||||
|
if not include_deletes and f.Action() == 'D':
|
||||||
|
continue
|
||||||
|
yield f
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def FilterSourceFile(cls,
|
def FilterSourceFile(cls,
|
||||||
|
|
Loading…
Reference in a new issue