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 <pkasting@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538723}

PRESUBMIT.py

@@ -1400,6 +1400,40 @@ def _CheckForAnonymousVariables(input_api, output_api):
   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 @@ def _CommonChecks(input_api, output_api):
           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))

cc/PRESUBMIT.py

@@ -144,34 +144,6 @@ def CheckDoubleAngles(input_api, output_api, white_list=CC_SOURCE_FILES,
     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 @@ def CheckChangeOnUpload(input_api, output_api):
   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)

components/viz/presubmit_checks.py

@@ -138,34 +138,6 @@ def CheckDoubleAngles(input_api, output_api, white_list,
     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 @@ def RunAllChecks(input_api, output_api, white_list):
   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)

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 CheckX11HeaderUsage(input_api, output_api):
 
 def CheckChange(input_api, output_api):
   results = []
-  results += CheckUniquePtr(input_api, output_api)
   results += CheckX11HeaderUsage(input_api, output_api)
   return results