PRESUBMIT: Enforce tracker prefix for all BUG entries

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 74955769cf8f7ab71d46f80d883c1165f7f49143..1666c6b905ccc7203b0cf3cdb0f9fd3ca069a07a 100755
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -106,7 +106,7 @@ def _RunCommand(command, cwd):
   return p.returncode, stdout, stderr
 
 
-def _VerifyNativeApiHeadersListIsValid(input_api, output_api):
+def VerifyNativeApiHeadersListIsValid(input_api, output_api):
   """Ensures the list of native API header directories is up to date."""
   non_existing_paths = []
   native_api_full_paths = [
@@ -142,7 +142,7 @@ You seem to be changing native API header files. Please make sure that you:
 Related files:
 """
 
-def _CheckNativeApiHeaderChanges(input_api, output_api):
+def CheckNativeApiHeaderChanges(input_api, output_api):
   """Checks to remind proper changing of native APIs."""
   files = []
   for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
@@ -156,7 +156,7 @@ def _CheckNativeApiHeaderChanges(input_api, output_api):
   return []
 
 
-def _CheckNoIOStreamInHeaders(input_api, output_api):
+def CheckNoIOStreamInHeaders(input_api, output_api):
   """Checks to make sure no .h files include <iostream>."""
   files = []
   pattern = input_api.re.compile(r'^#include\s*<iostream>',
@@ -177,7 +177,7 @@ def _CheckNoIOStreamInHeaders(input_api, output_api):
   return []
 
 
-def _CheckNoPragmaOnce(input_api, output_api):
+def CheckNoPragmaOnce(input_api, output_api):
   """Make sure that banned functions are not used."""
   files = []
   pattern = input_api.re.compile(r'^#pragma\s+once',
@@ -197,7 +197,7 @@ def _CheckNoPragmaOnce(input_api, output_api):
   return []
 
 
-def _CheckNoFRIEND_TEST(input_api, output_api):  # pylint: disable=invalid-name
+def CheckNoFRIEND_TEST(input_api, output_api):  # pylint: disable=invalid-name
   """Make sure that gtest's FRIEND_TEST() macro is not used, the
   FRIEND_TEST_ALL_PREFIXES() macro from testsupport/gtest_prod_util.h should be
   used instead since that allows for FLAKY_, FAILS_ and DISABLED_ prefixes."""
@@ -216,7 +216,7 @@ def _CheckNoFRIEND_TEST(input_api, output_api):  # pylint: disable=invalid-name
       'use FRIEND_TEST_ALL_PREFIXES() instead.\n' + '\n'.join(problems))]
 
 
-def _IsLintBlacklisted(blacklist_paths, file_path):
+def IsLintBlacklisted(blacklist_paths, file_path):
   """ Checks if a file is blacklisted for lint check."""
   for path in blacklist_paths:
     if file_path == path or os.path.dirname(file_path).startswith(path):
@@ -224,10 +224,10 @@ def _IsLintBlacklisted(blacklist_paths, file_path):
   return False
 
 
-def _CheckApprovedFilesLintClean(input_api, output_api,
+def CheckApprovedFilesLintClean(input_api, output_api,
                                  source_file_filter=None):
   """Checks that all new or non-blacklisted .cc and .h files pass cpplint.py.
-  This check is based on _CheckChangeLintsClean in
+  This check is based on CheckChangeLintsClean in
   depot_tools/presubmit_canned_checks.py but has less filters and only checks
   added files."""
   result = []
@@ -254,7 +254,7 @@ def _CheckApprovedFilesLintClean(input_api, output_api,
   files = []
   for f in input_api.AffectedSourceFiles(source_file_filter):
     # Note that moved/renamed files also count as added.
-    if f.Action() == 'A' or not _IsLintBlacklisted(blacklist_paths,
+    if f.Action() == 'A' or not IsLintBlacklisted(blacklist_paths,
                                                    f.LocalPath()):
       files.append(f.AbsoluteLocalPath())
 
@@ -270,7 +270,7 @@ def _CheckApprovedFilesLintClean(input_api, output_api,
 
   return result
 
-def _CheckNoSourcesAbove(input_api, gn_files, output_api):
+def CheckNoSourcesAbove(input_api, gn_files, output_api):
   # Disallow referencing source files with paths above the GN file location.
   source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
                                         re.MULTILINE | re.DOTALL)
@@ -298,7 +298,7 @@ def _CheckNoSourcesAbove(input_api, gn_files, output_api):
         items=violating_gn_files)]
   return []
 
-def _CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
+def CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
   # Disallow mixing .c and .cc source files in the same target.
   source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
                                         re.MULTILINE | re.DOTALL)
@@ -327,7 +327,7 @@ def _CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
         items=violating_gn_files.keys())]
   return []
 
-def _CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
+def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
   cwd = input_api.PresubmitLocalPath()
   script_path = os.path.join('tools_webrtc', 'presubmit_checks_lib',
                              'check_package_boundaries.py')
@@ -341,7 +341,7 @@ def _CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
         '%s' % stderr)]
   return []
 
-def _CheckGnChanges(input_api, output_api):
+def CheckGnChanges(input_api, output_api):
   source_file_filter = lambda x: input_api.FilterSourceFile(
       x, white_list=(r'.+\.(gn|gni)$',))
 
@@ -352,13 +352,13 @@ def _CheckGnChanges(input_api, output_api):
 
   result = []
   if gn_files:
-    result.extend(_CheckNoSourcesAbove(input_api, gn_files, output_api))
-    result.extend(_CheckNoMixingCAndCCSources(input_api, gn_files, output_api))
-    result.extend(_CheckNoPackageBoundaryViolations(
+    result.extend(CheckNoSourcesAbove(input_api, gn_files, output_api))
+    result.extend(CheckNoMixingCAndCCSources(input_api, gn_files, output_api))
+    result.extend(CheckNoPackageBoundaryViolations(
         input_api, gn_files, output_api))
   return result
 
-def _CheckUnwantedDependencies(input_api, output_api):
+def CheckUnwantedDependencies(input_api, output_api):
   """Runs checkdeps on #include statements added in this
   change. Breaking - rules is an error, breaking ! rules is a
   warning.
@@ -422,7 +422,32 @@ def _CheckUnwantedDependencies(input_api, output_api):
         warning_descriptions))
   return results
 
-def _CheckChangeHasBugField(input_api, output_api):
+def CheckCommitMessageBugEntry(input_api, output_api):
+  """Check that bug entries are well-formed in commit message."""
+  bogus_bug_msg = (
+      'Bogus BUG entry: %s. Please specify the issue tracker prefix and the '
+      'issue number, separated by a colon, e.g. webrtc:123 or chromium:12345.')
+  results = []
+  for bug in (input_api.change.BUG or '').split(','):
+    bug = bug.strip()
+    if bug.lower() == 'none':
+      continue
+    if ':' not in bug:
+      try:
+        if int(bug) > 100000:
+          # Rough indicator for current chromium bugs.
+          prefix_guess = 'chromium'
+        else:
+          prefix_guess = 'webrtc'
+        results.append('BUG entry requires issue tracker prefix, e.g. %s:%s' %
+                       (prefix_guess, bug))
+      except ValueError:
+        results.append(bogus_bug_msg % bug)
+    elif not re.match(r'\w+:\d+', bug):
+      results.append(bogus_bug_msg % bug)
+  return [output_api.PresubmitError(r) for r in results]
+
+def CheckChangeHasBugField(input_api, output_api):
   """Requires that the changelist have a BUG= field.
 
   This check is stricter than the one in depot_tools/presubmit_canned_checks.py
@@ -438,7 +463,7 @@ def _CheckChangeHasBugField(input_api, output_api):
         ' * https://bugs.webrtc.org - reference it using BUG=webrtc:XXXX\n'
         ' * https://crbug.com - reference it using BUG=chromium:XXXXXX')]
 
-def _CheckJSONParseErrors(input_api, output_api):
+def CheckJSONParseErrors(input_api, output_api):
   """Check that JSON files do not contain syntax errors."""
 
   def FilterFile(affected_file):
@@ -463,11 +488,12 @@ def _CheckJSONParseErrors(input_api, output_api):
   return results
 
 
-def _RunPythonTests(input_api, output_api):
+def RunPythonTests(input_api, output_api):
   def Join(*args):
     return input_api.os_path.join(input_api.PresubmitLocalPath(), *args)
 
   test_directories = [
+      '/',
       Join('webrtc', 'rtc_tools', 'py_event_log_analyzer'),
       Join('webrtc', 'rtc_tools'),
       Join('webrtc', 'audio', 'test', 'unittests'),
@@ -487,7 +513,7 @@ def _RunPythonTests(input_api, output_api):
   return input_api.RunTests(tests, parallel=True)
 
 
-def _CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
+def CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
   """Checks that the namespace google::protobuf has not been used."""
   files = []
   pattern = input_api.re.compile(r'google::protobuf')
@@ -507,7 +533,7 @@ def _CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
   return []
 
 
-def _CommonChecks(input_api, output_api):
+def CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
   # Filter out files that are in objc or ios dirs from being cpplint-ed since
@@ -518,7 +544,7 @@ def _CommonChecks(input_api, output_api):
     r"webrtc\/build\/ios\/SDK\/.*",
   )
   source_file_filter = lambda x: input_api.FilterSourceFile(x, None, black_list)
-  results.extend(_CheckApprovedFilesLintClean(
+  results.extend(CheckApprovedFilesLintClean(
       input_api, output_api, source_file_filter))
   results.extend(input_api.canned_checks.RunPylint(input_api, output_api,
       black_list=(r'^base[\\\/].*\.py$',
@@ -564,23 +590,23 @@ def _CommonChecks(input_api, output_api):
       input_api, output_api))
   results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
       input_api, output_api))
-  results.extend(_CheckNativeApiHeaderChanges(input_api, output_api))
-  results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
-  results.extend(_CheckNoPragmaOnce(input_api, output_api))
-  results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
-  results.extend(_CheckGnChanges(input_api, output_api))
-  results.extend(_CheckUnwantedDependencies(input_api, output_api))
-  results.extend(_CheckJSONParseErrors(input_api, output_api))
-  results.extend(_RunPythonTests(input_api, output_api))
-  results.extend(_CheckUsageOfGoogleProtobufNamespace(input_api, output_api))
-  results.extend(_CheckOrphanHeaders(input_api, output_api))
-  results.extend(_CheckNewLineAtTheEndOfProtoFiles(input_api, output_api))
+  results.extend(CheckNativeApiHeaderChanges(input_api, output_api))
+  results.extend(CheckNoIOStreamInHeaders(input_api, output_api))
+  results.extend(CheckNoPragmaOnce(input_api, output_api))
+  results.extend(CheckNoFRIEND_TEST(input_api, output_api))
+  results.extend(CheckGnChanges(input_api, output_api))
+  results.extend(CheckUnwantedDependencies(input_api, output_api))
+  results.extend(CheckJSONParseErrors(input_api, output_api))
+  results.extend(RunPythonTests(input_api, output_api))
+  results.extend(CheckUsageOfGoogleProtobufNamespace(input_api, output_api))
+  results.extend(CheckOrphanHeaders(input_api, output_api))
+  results.extend(CheckNewLineAtTheEndOfProtoFiles(input_api, output_api))
   return results
 
 
 def CheckChangeOnUpload(input_api, output_api):
   results = []
-  results.extend(_CommonChecks(input_api, output_api))
+  results.extend(CommonChecks(input_api, output_api))
   results.extend(
       input_api.canned_checks.CheckGNFormatted(input_api, output_api))
   return results
@@ -588,21 +614,22 @@ def CheckChangeOnUpload(input_api, output_api):
 
 def CheckChangeOnCommit(input_api, output_api):
   results = []
-  results.extend(_CommonChecks(input_api, output_api))
-  results.extend(_VerifyNativeApiHeadersListIsValid(input_api, output_api))
+  results.extend(CommonChecks(input_api, output_api))
+  results.extend(VerifyNativeApiHeadersListIsValid(input_api, output_api))
   results.extend(input_api.canned_checks.CheckOwners(input_api, output_api))
   results.extend(input_api.canned_checks.CheckChangeWasUploaded(
       input_api, output_api))
   results.extend(input_api.canned_checks.CheckChangeHasDescription(
       input_api, output_api))
-  results.extend(_CheckChangeHasBugField(input_api, output_api))
+  results.extend(CheckChangeHasBugField(input_api, output_api))
+  results.extend(CheckCommitMessageBugEntry(input_api, output_api))
   results.extend(input_api.canned_checks.CheckTreeIsOpen(
       input_api, output_api,
       json_url='http://webrtc-status.appspot.com/current?format=json'))
   return results
 
 
-def _CheckOrphanHeaders(input_api, output_api):
+def CheckOrphanHeaders(input_api, output_api):
   # We need to wait until we have an input_api object and use this
   # roundabout construct to import prebubmit_checks_lib because this file is
   # eval-ed and thus doesn't have __file__.
@@ -632,7 +659,7 @@ def _CheckOrphanHeaders(input_api, output_api):
   return results
 
 
-def _CheckNewLineAtTheEndOfProtoFiles(input_api, output_api):
+def CheckNewLineAtTheEndOfProtoFiles(input_api, output_api):
   """Checks that all .proto files are terminated with a newline."""
   error_msg = 'File {} must end with exactly one newline.'
   results = []