[mojo] Add presubmit check to prevent introducing old Mojo types

This change adds a presubmit check preventing new usages of the
following old Mojo types, which are now considered deprecated in
favor of the ones outlined in crbug.com/955171:

  - mojo::AssociatedBinding<>
  - mojo::AssociatedBindingSet<>
  - mojo::AssociatedInterfacePtr<>
  - mojo::AssociatedInterfacePtrInfo<>
  - mojo::AssociatedInterfaceRequest<>
  - mojo::Binding<>
  - mojo::BindingSet<>
  - mojo::InterfacePtr<>
  - mojo::InterfacePtrInfo<>
  - mojo::InterfaceRequest<>
  - mojo::MakeStrongAssociatedBinding()
  - mojo::MakeStrongBinding()
  - mojo::MakeRequest()
  - mojo::MakeRequestAssociatedWithDedicatedPipe()
  - mojo::StrongAssociatedBindingSet<>
  - mojo::StrongBindingSet<>

For now, we will only raise warnings during the presubmit stage
whenever any of these types get introduced in //third_party/blink,
but the plan is to progressively cover more directories and convert
such warnings into errors as soon as more areas get fully migrated
to the new types.

See the "Mojo Bindings Conversion CheatSheet" for more information:
https://docs.google.com/document/d/1Jwfbzbe8ozaoilhqj5mAPYbYGpgZCen_XAAAdwmyP1E

Bug: 955171
Change-Id: I0d40b1507d85cbc11397f9c2fb02fb96a7490911
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796371
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697555}

PRESUBMIT.py

@@ -1130,6 +1130,126 @@ _BANNED_CPP_FUNCTIONS = (
     ),
 )
 
+# Format: Sequence of tuples containing:
+# * String pattern or, if starting with a slash, a regular expression.
+# * Sequence of strings to show when the pattern matches.
+_DEPRECATED_MOJO_TYPES = (
+    (
+      r'/\bmojo::AssociatedBinding\b',
+      (
+        'mojo::AssociatedBinding<Interface> is deprecated.',
+        'Use mojo::AssociatedReceiver<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::AssociatedBindingSet\b',
+      (
+        'mojo::AssociatedBindingSet<Interface> is deprecated.',
+        'Use mojo::AssociatedReceiverSet<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::AssociatedInterfacePtr\b',
+      (
+        'mojo::AssociatedInterfacePtr<Interface> is deprecated.',
+        'Use mojo::AssociatedRemote<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::AssociatedInterfacePtrInfo\b',
+      (
+        'mojo::AssociatedInterfacePtrInfo<Interface> is deprecated.',
+        'Use mojo::PendingAssociatedRemote<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::AssociatedInterfaceRequest\b',
+      (
+        'mojo::AssociatedInterfaceRequest<Interface> is deprecated.',
+        'Use mojo::PendingAssociatedReceiver<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::Binding\b',
+      (
+        'mojo::Binding<Interface> is deprecated.',
+        'Use mojo::Receiver<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::BindingSet\b',
+      (
+        'mojo::BindingSet<Interface> is deprecated.',
+        'Use mojo::ReceiverSet<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::InterfacePtr\b',
+      (
+        'mojo::InterfacePtr<Interface> is deprecated.',
+        'Use mojo::Remote<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::InterfacePtrInfo\b',
+      (
+        'mojo::InterfacePtrInfo<Interface> is deprecated.',
+        'Use mojo::PendingRemote<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::InterfaceRequest\b',
+      (
+        'mojo::InterfaceRequest<Interface> is deprecated.',
+        'Use mojo::PendingReceiver<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::MakeRequest\b',
+      (
+        'mojo::MakeRequest is deprecated.',
+        'Use mojo::Remote::BindNewPipeAndPassReceiver() instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::MakeRequestAssociatedWithDedicatedPipe\b',
+      (
+        'mojo::MakeRequest is deprecated.',
+        'Use mojo::AssociatedRemote::'
+        'BindNewEndpointAndPassDedicatedReceiverForTesting() instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::MakeStrongBinding\b',
+      (
+        'mojo::MakeStrongBinding is deprecated.',
+        'Either migrate to mojo::UniqueReceiverSet, if possible, or use',
+        'mojo::MakeSelfOwnedReceiver() instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::MakeStrongAssociatedBinding\b',
+      (
+        'mojo::MakeStrongAssociatedBinding is deprecated.',
+        'Either migrate to mojo::UniqueAssociatedReceiverSet, if possible, or',
+        'use mojo::MakeSelfOwnedAssociatedReceiver() instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::StrongAssociatedBindingSet\b',
+      (
+        'mojo::StrongAssociatedBindingSet<Interface> is deprecated.',
+        'Use mojo::UniqueAssociatedReceiverSet<Interface> instead.',
+      ),
+    ),
+    (
+      r'/\bmojo::StrongBindingSet\b',
+      (
+        'mojo::StrongBindingSet<Interface> is deprecated.',
+        'Use mojo::UniqueReceiverSet<Interface> instead.',
+      ),
+    ),
+)
 
 _IPC_ENUM_TRAITS_DEPRECATED = (
     'You are using IPC_ENUM_TRAITS() in your code. It has been deprecated.\n'
@@ -1698,6 +1818,31 @@ def _CheckValidHostsInDEPS(input_api, output_api):
         long_text=error.output)]
 
 
+def _GetMessageForMatchingType(input_api, affected_file, line_number, line,
+                               type_name, message):
+  """Helper method for _CheckNoBannedFunctions and _CheckNoDeprecatedMojoTypes.
+
+  Returns an string composed of the name of the file, the line number where the
+  match has been found and the additional text passed as |message| in case the
+  target type name matches the text inside the line passed as parameter.
+  """
+  matched = False
+  if type_name[0:1] == '/':
+    regex = type_name[1:]
+    if input_api.re.search(regex, line):
+      matched = True
+  elif type_name in line:
+    matched = True
+
+  result = []
+  if matched:
+    result.append('    %s:%d:' % (affected_file.LocalPath(), line_number))
+    for message_line in message:
+      result.append('      %s' % message_line)
+
+  return result
+
+
 def _CheckNoBannedFunctions(input_api, output_api):
   """Make sure that banned functions are not used."""
   warnings = []
@@ -1723,20 +1868,13 @@ def _CheckNoBannedFunctions(input_api, output_api):
     return False
 
   def CheckForMatch(affected_file, line_num, line, func_name, message, error):
-    matched = False
-    if func_name[0:1] == '/':
-      regex = func_name[1:]
-      if input_api.re.search(regex, line):
-        matched = True
-    elif func_name in line:
-      matched = True
-    if matched:
-      problems = warnings
+    problems = _GetMessageForMatchingType(input_api, f, line_num, line,
+                                          func_name, message)
+    if problems:
       if error:
-        problems = errors
-      problems.append('    %s:%d:' % (affected_file.LocalPath(), line_num))
-      for message_line in message:
-        problems.append('      %s' % message_line)
+        errors.extend(problems)
+      else:
+        warnings.extend(problems)
 
   file_filter = lambda f: f.LocalPath().endswith(('.java'))
   for f in input_api.AffectedFiles(file_filter=file_filter):
@@ -1779,6 +1917,30 @@ def _CheckNoBannedFunctions(input_api, output_api):
   return result
 
 
+def _CheckNoDeprecatedMojoTypes(input_api, output_api):
+  """Make sure that old Mojo types are not used."""
+  warnings = []
+
+  file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm', '.h'))
+  for f in input_api.AffectedFiles(file_filter=file_filter):
+    # Only need to check Blink for warnings for now.
+    if not f.LocalPath().startswith('third_party/blink'):
+      continue
+
+    for line_num, line in f.ChangedContents():
+      for func_name, message in _DEPRECATED_MOJO_TYPES:
+        problems = _GetMessageForMatchingType(input_api, f, line_num, line,
+                                              func_name, message)
+        if problems:
+            warnings.extend(problems)
+
+  result = []
+  if (warnings):
+    result.append(output_api.PresubmitPromptWarning(
+        'Banned Mojo types were used.\n' + '\n'.join(warnings)))
+  return result
+
+
 def _CheckNoPragmaOnce(input_api, output_api):
   """Make sure that banned functions are not used."""
   files = []
@@ -3868,6 +4030,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckNoNewWStrings(input_api, output_api))
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoBannedFunctions(input_api, output_api))
+  results.extend(_CheckNoDeprecatedMojoTypes(input_api, output_api))
   results.extend(_CheckNoPragmaOnce(input_api, output_api))
   results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
   results.extend(_CheckUnwantedDependencies(input_api, output_api))

PRESUBMIT_test.py

@@ -1909,7 +1909,7 @@ class ServiceManifestOwnerTest(unittest.TestCase):
     self.assertEqual([], errors)
 
 
-class BannedFunctionCheckTest(unittest.TestCase):
+class BannedTypeCheckTest(unittest.TestCase):
 
   def testBannedIosObjcFunctions(self):
     input_api = MockInputApi()
@@ -1964,6 +1964,102 @@ class BannedFunctionCheckTest(unittest.TestCase):
     self.assertTrue('third_party/blink/ok/file3.cc' not in results[0].message)
     self.assertTrue('content/renderer/ok/file3.cc' not in results[0].message)
 
+  def testDeprecatedMojoTypes(self):
+    ok_paths = ['some/cpp']
+    warning_paths = ['third_party/blink']
+    test_cases = [
+      {
+        'type': 'mojo::AssociatedBinding<>;',
+        'file': 'file1.c'
+      },
+      {
+        'type': 'mojo::AssociatedBindingSet<>;',
+        'file': 'file2.c'
+      },
+      {
+        'type': 'mojo::AssociatedInterfacePtr<>',
+        'file': 'file3.cc'
+      },
+      {
+        'type': 'mojo::AssociatedInterfacePtrInfo<>',
+        'file': 'file4.cc'
+      },
+      {
+        'type': 'mojo::AssociatedInterfaceRequest<>',
+        'file': 'file5.cc'
+      },
+      {
+        'type': 'mojo::Binding<>',
+        'file': 'file6.cc'
+      },
+      {
+        'type': 'mojo::BindingSet<>',
+        'file': 'file7.cc'
+      },
+      {
+        'type': 'mojo::InterfacePtr<>',
+        'file': 'file8.cc'
+      },
+      {
+        'type': 'mojo::InterfacePtrInfo<>',
+        'file': 'file9.cc'
+      },
+      {
+        'type': 'mojo::InterfaceRequest<>',
+        'file': 'file10.cc'
+      },
+      {
+        'type': 'mojo::MakeRequest()',
+        'file': 'file11.cc'
+      },
+      {
+        'type': 'mojo::MakeRequestAssociatedWithDedicatedPipe()',
+        'file': 'file12.cc'
+      },
+      {
+        'type': 'mojo::MakeStrongBinding()<>',
+        'file': 'file13.cc'
+      },
+      {
+        'type': 'mojo::MakeStrongAssociatedBinding()<>',
+        'file': 'file14.cc'
+      },
+      {
+        'type': 'mojo::StrongAssociatedBindingSet<>',
+        'file': 'file15.cc'
+      },
+      {
+        'type': 'mojo::StrongBindingSet<>',
+        'file': 'file16.cc'
+      },
+    ]
+
+    # Build the list of MockFiles considering paths that should trigger warnings
+    # as well as paths that should not trigger anything.
+    input_api = MockInputApi()
+    input_api.files = []
+    for test_case in test_cases:
+      for path in ok_paths:
+        input_api.files.append(MockFile(os.path.join(path, test_case['file']),
+                                        [test_case['type']]))
+      for path in warning_paths:
+        input_api.files.append(MockFile(os.path.join(path, test_case['file']),
+                                        [test_case['type']]))
+
+    results = PRESUBMIT._CheckNoDeprecatedMojoTypes(input_api, MockOutputApi())
+
+    # Only warnings for now for all deprecated Mojo types.
+    self.assertEqual(1, len(results))
+
+    for test_case in test_cases:
+      # Check that no warnings or errors have been triggered for these paths.
+      for path in ok_paths:
+        self.assertFalse(path in results[0].message)
+
+      # Check warnings have been triggered for these paths.
+      for path in warning_paths:
+        self.assertTrue(path in results[0].message)
+
 
 class NoProductionCodeUsingTestOnlyFunctionsTest(unittest.TestCase):
   def testTruePositives(self):