Restore CheckNoStrCatRedefines check and speed up it a little.
CheckNoStrCatRedefines was not used since its inception. This CL enables the check and adds header/impl filters to make it run faster. Bug: 1330868 Change-Id: Ie9b57474675a57cb6d6b5fddbf1945c8081c9888 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3679759 Commit-Queue: Bruce Dawson <brucedawson@chromium.org> Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com> Reviewed-by: Bruce Dawson <brucedawson@chromium.org> Cr-Commit-Position: refs/heads/main@{#1010614}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
b45f989d28
commit
9b28c039ae
18
PRESUBMIT.py
18
PRESUBMIT.py
@ -70,6 +70,13 @@ _IMPLEMENTATION_EXTENSIONS = r'\.(cc|cpp|cxx|mm)$'
|
||||
_HEADER_EXTENSIONS = r'\.(h|hpp|hxx)$'
|
||||
|
||||
|
||||
# Paths with sources that don't use //base.
|
||||
_NON_BASE_DEPENDENT_PATHS = (
|
||||
r"^chrome[\\/]browser[\\/]browser_switcher[\\/]bho[\\/]",
|
||||
r"^tools[\\/]win[\\/]",
|
||||
)
|
||||
|
||||
|
||||
# Regular expression that matches code only used for test binaries
|
||||
# (best effort).
|
||||
_TEST_CODE_EXCLUDED_PATHS = (
|
||||
@ -1381,15 +1388,22 @@ def CheckNoIOStreamInHeaders(input_api, output_api):
|
||||
return []
|
||||
|
||||
|
||||
def _CheckNoStrCatRedefines(input_api, output_api):
|
||||
def CheckNoStrCatRedefines(input_api, output_api):
|
||||
"""Checks no windows headers with StrCat redefined are included directly."""
|
||||
files = []
|
||||
files_to_check = (r'.+%s' % _HEADER_EXTENSIONS,
|
||||
r'.+%s' % _IMPLEMENTATION_EXTENSIONS)
|
||||
files_to_skip = (input_api.DEFAULT_FILES_TO_SKIP +
|
||||
_NON_BASE_DEPENDENT_PATHS)
|
||||
sources_filter = lambda f: input_api.FilterSourceFile(
|
||||
f, files_to_check=files_to_check, files_to_skip=files_to_skip)
|
||||
|
||||
pattern_deny = input_api.re.compile(
|
||||
r'^#include\s*[<"](shlwapi|atlbase|propvarutil|sphelper).h[">]',
|
||||
input_api.re.MULTILINE)
|
||||
pattern_allow = input_api.re.compile(
|
||||
r'^#include\s"base/win/windows_defines.inc"', input_api.re.MULTILINE)
|
||||
for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
|
||||
for f in input_api.AffectedSourceFiles(sources_filter):
|
||||
contents = input_api.ReadFile(f)
|
||||
if pattern_deny.search(
|
||||
contents) and not pattern_allow.search(contents):
|
||||
|
@ -3044,7 +3044,7 @@ class CheckNoDirectIncludesHeadersWhichRedefineStrCat(unittest.TestCase):
|
||||
MockFile('dir/baz.h', ['#include <atlbase.h>']),
|
||||
MockFile('dir/jumbo.h', ['#include "sphelper.h"']),
|
||||
]
|
||||
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
results = PRESUBMIT.CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
self.assertEqual(1, len(results))
|
||||
self.assertEqual(4, len(results[0].items))
|
||||
self.assertTrue('StrCat' in results[0].message)
|
||||
@ -3059,7 +3059,7 @@ class CheckNoDirectIncludesHeadersWhichRedefineStrCat(unittest.TestCase):
|
||||
MockFile('dir/baz_win.cc', ['#include "base/win/shlwapi.h"']),
|
||||
MockFile('dir/baz-win.h', ['#include "base/win/atl.h"']),
|
||||
]
|
||||
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
results = PRESUBMIT.CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
self.assertEqual(0, len(results))
|
||||
|
||||
def testAllowsToCreateWrapper(self):
|
||||
@ -3069,7 +3069,16 @@ class CheckNoDirectIncludesHeadersWhichRedefineStrCat(unittest.TestCase):
|
||||
'#include <shlwapi.h>',
|
||||
'#include "base/win/windows_defines.inc"']),
|
||||
]
|
||||
results = PRESUBMIT._CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
results = PRESUBMIT.CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
self.assertEqual(0, len(results))
|
||||
|
||||
def testIgnoresNonImplAndHeaders(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_input_api.files = [
|
||||
MockFile('dir/foo_win.txt', ['#include "shlwapi.h"']),
|
||||
MockFile('dir/bar.asm', ['#include <propvarutil.h>']),
|
||||
]
|
||||
results = PRESUBMIT.CheckNoStrCatRedefines(mock_input_api, MockOutputApi())
|
||||
self.assertEqual(0, len(results))
|
||||
|
||||
|
||||
|
Reference in New Issue
Block a user