Adding PRESUBMIT check to avoid mixing C, C++ and Objc-C/Obj-C++.

The error message will be something like:

GN targets cannot mix .c (or .cc) and .m (or .mm) source files.
Please create a separate target for each collection of sources.
Mixed sources:
{
  BUILD_GN_PATH: [
    [
      TARGET_NAME,
      [
        SOURCES
      ]
    ],
    ...
  ],
  ...
}

Bug: webrtc:7743
Change-Id: I45dd2c621b830e5aeb081fa8d17c9497a49c2554
Reviewed-on: https://webrtc-review.googlesource.com/1980
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Henrik Kjellander <kjellander@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19897}

PRESUBMIT.py

@@ -11,6 +11,7 @@ import os
 import re
 import subprocess
 import sys
+from collections import defaultdict
 
 
 # Files and directories that are *skipped* by cpplint in the presubmit script.
@@ -67,6 +68,7 @@ NATIVE_API_DIRS = (
   'modules/audio_device/include',
   'pc',
 )
+
 # These directories should not be used but are maintained only to avoid breaking
 # some legacy downstream code.
 LEGACY_API_DIRS = (
@@ -90,8 +92,22 @@ LEGACY_API_DIRS = (
   'system_wrappers/include',
   'voice_engine/include',
 )
+
 API_DIRS = NATIVE_API_DIRS[:] + LEGACY_API_DIRS[:]
 
+# TARGET_RE matches a GN target, and extracts the target name and the contents.
+TARGET_RE = re.compile(r'(?P<indent>\s*)\w+\("(?P<target_name>\w+)"\) {'
+                       r'(?P<target_contents>.*?)'
+                       r'(?P=indent)}',
+                       re.MULTILINE | re.DOTALL)
+
+# SOURCES_RE matches a block of sources inside a GN target.
+SOURCES_RE = re.compile(r'sources \+?= \[(?P<sources>.*?)\]',
+                        re.MULTILINE | re.DOTALL)
+
+# FILE_PATH_RE matchies a file path.
+FILE_PATH_RE = re.compile(r'"(?P<file_path>(\w|\/)+)(?P<extension>\.\w+)"')
+
 
 def _RunCommand(command, cwd):
   """Runs a command and returns the output from that command."""
@@ -297,33 +313,48 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api):
         items=violating_gn_files)]
   return []
 
-def CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
+def CheckNoMixingSources(input_api, gn_files, output_api):
-  # Disallow mixing .c and .cc source files in the same target.
+  """Disallow mixing C, C++ and Obj-C/Obj-C++ in the same target.
-  source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
+
-                                        re.MULTILINE | re.DOTALL)
+  See bugs.webrtc.org/7743 for more context.
-  file_pattern = input_api.re.compile(r'"(.*)"')
+  """
-  violating_gn_files = dict()
+  def _MoreThanOneSourceUsed(*sources_lists):
+    sources_used = 0
+    for source_list in sources_lists:
+      if len(source_list):
+        sources_used += 1
+    return sources_used > 1
+
+  errors = defaultdict(lambda: [])
   for gn_file in gn_files:
-    contents = input_api.ReadFile(gn_file)
+    gn_file_content = input_api.ReadFile(gn_file)
-    for source_block_match in source_pattern.finditer(contents):
+    for target_match in TARGET_RE.finditer(gn_file_content):
       c_files = []
       cc_files = []
-      for file_list_match in file_pattern.finditer(source_block_match.group(1)):
+      objc_files = []
-        source_file = file_list_match.group(1)
+      target_name = target_match.group('target_name')
-        if source_file.endswith('.c'):
+      target_contents = target_match.group('target_contents')
-          c_files.append(source_file)
+      for sources_match in SOURCES_RE.finditer(target_contents):
-        if source_file.endswith('.cc'):
+        for file_match in FILE_PATH_RE.finditer(sources_match.group(1)):
-          cc_files.append(source_file)
+          file_path = file_match.group('file_path')
-      if c_files and cc_files:
+          extension = file_match.group('extension')
-        violating_gn_files[gn_file.LocalPath()] = sorted(c_files + cc_files)
+          if extension == '.c':
-  if violating_gn_files:
+            c_files.append(file_path + extension)
+          if extension == '.cc':
+            cc_files.append(file_path + extension)
+          if extension in ['.m', '.mm']:
+            objc_files.append(file_path + extension)
+      if _MoreThanOneSourceUsed(c_files, cc_files, objc_files):
+        all_sources = sorted(c_files + cc_files + objc_files)
+        errors[gn_file.LocalPath()].append((target_name, all_sources))
+  if errors:
     return [output_api.PresubmitError(
-        'GN targets cannot mix .cc and .c source files. Please create a '
+        'GN targets cannot mix .c, .cc and .m (or .mm) source files.\n'
-        'separate target for each collection of sources.\n'
+        'Please create a separate target for each collection of sources.\n'
         'Mixed sources: \n'
         '%s\n'
-        'Violating GN files:' % json.dumps(violating_gn_files, indent=2),
+        'Violating GN files:\n%s\n' % (json.dumps(errors, indent=2),
-        items=violating_gn_files.keys())]
+                                       '\n'.join(errors.keys())))]
   return []
 
 def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
@@ -350,9 +381,9 @@ 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(CheckNoMixingSources(input_api, gn_files, output_api))
-    result.extend(CheckNoPackageBoundaryViolations(
+    result.extend(CheckNoPackageBoundaryViolations(input_api, gn_files,
-        input_api, gn_files, output_api))
+                                                   output_api))
   return result
 
 def CheckUnwantedDependencies(input_api, output_api):

presubmit_test.py

@@ -60,7 +60,7 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
     shutil.rmtree(self.tmp_dir, ignore_errors=True)
 
   def testErrorIfProtoFileDoesNotEndWithNewline(self):
-    self.__GenerateProtoWithoutNewlineAtTheEnd()
+    self._GenerateProtoWithoutNewlineAtTheEnd()
     self.input_api.files = [MockFile(self.proto_file_path)]
     errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
                                                         self.output_api)
@@ -70,13 +70,13 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
         str(errors[0]))
 
   def testNoErrorIfProtoFileEndsWithNewline(self):
-    self.__GenerateProtoWithNewlineAtTheEnd()
+    self._GenerateProtoWithNewlineAtTheEnd()
     self.input_api.files = [MockFile(self.proto_file_path)]
     errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
                                                         self.output_api)
     self.assertEqual(0, len(errors))
 
-  def __GenerateProtoWithNewlineAtTheEnd(self):
+  def _GenerateProtoWithNewlineAtTheEnd(self):
     with open(self.proto_file_path, 'w') as f:
       f.write(textwrap.dedent("""
         syntax = "proto2";
@@ -84,7 +84,7 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
         package webrtc.audioproc;
       """))
 
-  def __GenerateProtoWithoutNewlineAtTheEnd(self):
+  def _GenerateProtoWithoutNewlineAtTheEnd(self):
     with open(self.proto_file_path, 'w') as f:
       f.write(textwrap.dedent("""
         syntax = "proto2";
@@ -92,5 +92,72 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
         package webrtc.audioproc;"""))
 
 
+class CheckNoMixingSourcesTest(unittest.TestCase):
+
+  def setUp(self):
+    self.tmp_dir = tempfile.mkdtemp()
+    self.file_path = os.path.join(self.tmp_dir, 'BUILD.gn')
+    self.input_api = MockInputApi()
+    self.output_api = MockOutputApi()
+
+  def tearDown(self):
+    shutil.rmtree(self.tmp_dir, ignore_errors=True)
+
+  def testErrorIfCAndCppAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.cc', 'bar.h'])
+
+  def testErrorIfCAndObjCAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.m', 'bar.h'])
+
+  def testErrorIfCAndObjCppAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.mm', 'bar.h'])
+
+  def testErrorIfCppAndObjCAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(1, ['foo.cc', 'bar.m', 'bar.h'])
+
+  def testErrorIfCppAndObjCppAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(1, ['foo.cc', 'bar.mm', 'bar.h'])
+
+  def testNoErrorIfOnlyC(self):
+    self._AssertNumberOfErrorsWithSources(0, ['foo.c', 'bar.c', 'bar.h'])
+
+  def testNoErrorIfOnlyCpp(self):
+    self._AssertNumberOfErrorsWithSources(0, ['foo.cc', 'bar.cc', 'bar.h'])
+
+  def testNoErrorIfOnlyObjC(self):
+    self._AssertNumberOfErrorsWithSources(0, ['foo.m', 'bar.m', 'bar.h'])
+
+  def testNoErrorIfOnlyObjCpp(self):
+    self._AssertNumberOfErrorsWithSources(0, ['foo.mm', 'bar.mm', 'bar.h'])
+
+  def testNoErrorIfObjCAndObjCppAreMixed(self):
+    self._AssertNumberOfErrorsWithSources(0, ['foo.m', 'bar.mm', 'bar.h'])
+
+  def _AssertNumberOfErrorsWithSources(self, number_of_errors, sources):
+    assert 3 == len(sources), 'This function accepts a list of 3 source files'
+    self._GenerateBuildFile(textwrap.dedent("""
+      rtc_source_set("foo_bar") {
+        sources = [
+          "%s",
+          "%s",
+          "%s",
+        ],
+      }
+    """ % tuple(sources)))
+    self.input_api.files = [MockFile(self.file_path)]
+    errors = PRESUBMIT.CheckNoMixingSources(self.input_api,
+                                            [MockFile(self.file_path)],
+                                            self.output_api)
+    self.assertEqual(number_of_errors, len(errors))
+    if number_of_errors == 1:
+      for source in sources:
+        if not source.endswith('.h'):
+          self.assertTrue(source in str(errors[0]))
+
+  def _GenerateBuildFile(self, content):
+    with open(self.file_path, 'w') as f:
+      f.write(content)
+
+
 if __name__ == '__main__':
   unittest.main()

presubmit_test_mocks.py

@@ -25,6 +25,15 @@ class MockInputApi(object):
     # pylint: disable=unused-argument
     return self.files
 
+  def ReadFile(self, affected_file, mode='rU'):
+    filename = affected_file.AbsoluteLocalPath()
+    for f in self.files:
+      if f.LocalPath() == filename:
+        with open(filename, mode) as f:
+          return f.read()
+    # Otherwise, file is not in our mock API.
+    raise IOError, "No such file or directory: '%s'" % filename
+
 
 class MockOutputApi(object):
   """Mock class for the OutputApi class.
@@ -71,3 +80,6 @@ class MockFile(object):
 
   def LocalPath(self):
     return self._local_path
+
+  def AbsoluteLocalPath(self):
+    return self._local_path