Fix unique_ptr<>() PRESUBMIT.
Three directories had a PRESUBMIT designed to catch "std::unique_ptr<T>()" and
require nullptr instead. In two directories, the match was overbroad, catching
things like "std::vector<std::unique_ptr<T>>()". In ui/, ricea attempted a fix
(in https://codereview.chromium.org/2311783002 ) that made the regex too
conservative, failing to catch std::unique_ptr<Foo<T>>().
Instead, fail to match std::unique_ptr<T>() if it's immediately preceded by '<'.
This moves the check to the global PRESUBMIT to avoid duplication.
BUG=none
TEST=none
TBR=sky
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ia8888408683b2de3bbe60e0e43409d72475ea169
Reviewed-on: https://chromium-review.googlesource.com/933547
Reviewed-by: Peter Kasting <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Peter Kasting <[email protected]>
Cr-Commit-Position: refs/heads/master@{#538723}diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 927ec2b..48493fc 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1400,6 +1400,40 @@
return []
+def _CheckUniquePtr(input_api, output_api):
+ file_inclusion_pattern = r'.+%s' % _IMPLEMENTATION_EXTENSIONS
+ sources = lambda affected_file: input_api.FilterSourceFile(
+ affected_file,
+ black_list=(_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS +
+ input_api.DEFAULT_BLACK_LIST),
+ white_list=(file_inclusion_pattern,))
+ return_construct_pattern = input_api.re.compile(
+ r'(=|\breturn)\s*std::unique_ptr<.*?(?<!])>\([^)]+\)')
+ null_construct_pattern = input_api.re.compile(
+ r'\b(?<!<)std::unique_ptr<.*?>\(\)')
+ errors = []
+ for f in input_api.AffectedSourceFiles(sources):
+ for line_number, line in f.ChangedContents():
+ # Disallow:
+ # return std::unique_ptr<T>(foo);
+ # bar = std::unique_ptr<T>(foo);
+ # But allow:
+ # return std::unique_ptr<T[]>(foo);
+ # bar = std::unique_ptr<T[]>(foo);
+ if return_construct_pattern.search(line):
+ errors.append(output_api.PresubmitError(
+ ('%s:%d uses explicit std::unique_ptr constructor. ' +
+ 'Use std::make_unique<T>() instead.') %
+ (f.LocalPath(), line_number)))
+ # Disallow:
+ # std::unique_ptr<T>()
+ if null_construct_pattern.search(line):
+ errors.append(output_api.PresubmitError(
+ '%s:%d uses std::unique_ptr<T>(). Use nullptr instead.' %
+ (f.LocalPath(), line_number)))
+ return errors
+
+
def _CheckUserActionUpdate(input_api, output_api):
"""Checks if any new user action has been added."""
if any('actions.xml' == input_api.os_path.basename(f) for f in
@@ -2531,6 +2565,7 @@
source_file_filter=lambda x: x.LocalPath().endswith('.grd')))
results.extend(_CheckSpamLogging(input_api, output_api))
results.extend(_CheckForAnonymousVariables(input_api, output_api))
+ results.extend(_CheckUniquePtr(input_api, output_api))
results.extend(_CheckUserActionUpdate(input_api, output_api))
results.extend(_CheckNoDeprecatedCss(input_api, output_api))
results.extend(_CheckNoDeprecatedJs(input_api, output_api))
diff --git a/cc/PRESUBMIT.py b/cc/PRESUBMIT.py
index d160eac..26909c0b 100644
--- a/cc/PRESUBMIT.py
+++ b/cc/PRESUBMIT.py
@@ -144,34 +144,6 @@
return [output_api.PresubmitError('Use >> instead of > >:', items=errors)]
return []
-def CheckUniquePtr(input_api, output_api,
- white_list=CC_SOURCE_FILES, black_list=None):
- black_list = tuple(black_list or input_api.DEFAULT_BLACK_LIST)
- source_file_filter = lambda x: input_api.FilterSourceFile(x,
- white_list,
- black_list)
- errors = []
- for f in input_api.AffectedSourceFiles(source_file_filter):
- for line_number, line in f.ChangedContents():
- # Disallow:
- # return std::unique_ptr<T>(foo);
- # bar = std::unique_ptr<T>(foo);
- # But allow:
- # return std::unique_ptr<T[]>(foo);
- # bar = std::unique_ptr<T[]>(foo);
- if re.search(r'(=|\breturn)\s*std::unique_ptr<.*?(?<!])>\([^)]+\)', line):
- errors.append(output_api.PresubmitError(
- ('%s:%d uses explicit std::unique_ptr constructor. ' +
- 'Use std::make_unique<T>() instead.') %
- (f.LocalPath(), line_number)))
- # Disallow:
- # std::unique_ptr<T>()
- if re.search(r'\bstd::unique_ptr<.*?>\(\)', line):
- errors.append(output_api.PresubmitError(
- '%s:%d uses std::unique_ptr<T>(). Use nullptr instead.' %
- (f.LocalPath(), line_number)))
- return errors
-
def FindUnquotedQuote(contents, pos):
match = re.search(r"(?<!\\)(?P<quote>\")", contents[pos:])
return -1 if not match else match.start("quote") + pos
@@ -330,7 +302,6 @@
results += CheckChangeLintsClean(input_api, output_api)
results += CheckTodos(input_api, output_api)
results += CheckDoubleAngles(input_api, output_api)
- results += CheckUniquePtr(input_api, output_api)
results += CheckNamespace(input_api, output_api)
results += CheckForUseOfWrongClock(input_api, output_api)
results += FindUselessIfdefs(input_api, output_api)
diff --git a/components/viz/presubmit_checks.py b/components/viz/presubmit_checks.py
index ec43826f..e73bdfc 100644
--- a/components/viz/presubmit_checks.py
+++ b/components/viz/presubmit_checks.py
@@ -138,34 +138,6 @@
return [output_api.PresubmitError('Use >> instead of > >:', items=errors)]
return []
-def CheckUniquePtr(input_api, output_api,
- white_list, black_list=None):
- black_list = tuple(black_list or input_api.DEFAULT_BLACK_LIST)
- source_file_filter = lambda x: input_api.FilterSourceFile(x,
- white_list,
- black_list)
- errors = []
- for f in input_api.AffectedSourceFiles(source_file_filter):
- for line_number, line in f.ChangedContents():
- # Disallow:
- # return std::unique_ptr<T>(foo);
- # bar = std::unique_ptr<T>(foo);
- # But allow:
- # return std::unique_ptr<T[]>(foo);
- # bar = std::unique_ptr<T[]>(foo);
- if re.search(r'(=|\breturn)\s*std::unique_ptr<.*?(?<!])>\([^)]+\)', line):
- errors.append(output_api.PresubmitError(
- ('%s:%d uses explicit std::unique_ptr constructor. ' +
- 'Use std::make_unique<T>() instead.') %
- (f.LocalPath(), line_number)))
- # Disallow:
- # std::unique_ptr<T>()
- if re.search(r'\bstd::unique_ptr<.*?>\(\)', line):
- errors.append(output_api.PresubmitError(
- '%s:%d uses std::unique_ptr<T>(). Use nullptr instead.' %
- (f.LocalPath(), line_number)))
- return errors
-
def FindUnquotedQuote(contents, pos):
match = re.search(r"(?<!\\)(?P<quote>\")", contents[pos:])
return -1 if not match else match.start("quote") + pos
@@ -331,7 +303,6 @@
results += CheckChangeLintsClean(input_api, output_api, white_list)
results += CheckTodos(input_api, output_api)
results += CheckDoubleAngles(input_api, output_api, white_list)
- results += CheckUniquePtr(input_api, output_api, white_list)
results += CheckNamespace(input_api, output_api)
results += CheckMojoms(input_api, output_api)
results += CheckForUseOfWrongClock(input_api, output_api, white_list)
diff --git a/ui/PRESUBMIT.py b/ui/PRESUBMIT.py
index 7c0c787..810c0919 100644
--- a/ui/PRESUBMIT.py
+++ b/ui/PRESUBMIT.py
@@ -8,39 +8,6 @@
for more details about the presubmit API built into depot_tools.
"""
-INCLUDE_CPP_FILES_ONLY = (
- r'.*\.(cc|h|mm)$',
-)
-
-def CheckUniquePtr(input_api, output_api,
- white_list=INCLUDE_CPP_FILES_ONLY, black_list=None):
- black_list = tuple(black_list or input_api.DEFAULT_BLACK_LIST)
- source_file_filter = lambda x: input_api.FilterSourceFile(x,
- white_list,
- black_list)
- errors = []
- for f in input_api.AffectedSourceFiles(source_file_filter):
- for line_number, line in f.ChangedContents():
- # Disallow:
- # return std::unique_ptr<T>(foo);
- # bar = std::unique_ptr<T>(foo);
- # But allow:
- # return std::unique_ptr<T[]>(foo);
- # bar = std::unique_ptr<T[]>(foo);
- if input_api.re.search(
- r'(=|\breturn)\s*std::unique_ptr<[^\[\]>]+>\([^)]+\)', line):
- errors.append(output_api.PresubmitError(
- ('%s:%d uses explicit std::unique_ptr constructor. ' +
- 'Use std::make_unique<T>() or base::WrapUnique() instead.') %
- (f.LocalPath(), line_number)))
- # Disallow:
- # std::unique_ptr<T>()
- if input_api.re.search(r'\bstd::unique_ptr<[^<>]+>\(\)', line):
- errors.append(output_api.PresubmitError(
- '%s:%d uses std::unique_ptr<T>(). Use nullptr instead.' %
- (f.LocalPath(), line_number)))
- return errors
-
def CheckX11HeaderUsage(input_api, output_api):
"""X11 headers pollute the global namespace with macros for common
names so instead code should include "ui/gfx/x/x11.h" which hide the
@@ -68,7 +35,6 @@
def CheckChange(input_api, output_api):
results = []
- results += CheckUniquePtr(input_api, output_api)
results += CheckX11HeaderUsage(input_api, output_api)
return results
