Add lint checking for Objective-C++ sources (#1048)

* lint.sh now lints Objective-C++ too

* cpplint checks system-style includes that should be user-style

This prevents it from recognizing project sources as if they were C
system headers and then complaining that they're in the wrong place.

* cpplint checks #imports and #includes

* cpplint checks that C++ system headers aren't #imported

* cpplint checks for C system headers that could be C++ system headers

* cpplint checks that Objective-C sources include their headers

2 files changed, 94 insertions, 7 deletions

diff --git a/scripts/cpplint.py b/scripts/cpplint.py
index d1c80b3..f09695c 100644
--- a/scripts/cpplint.py
+++ b/scripts/cpplint.py
@@ -428,6 +428,11 @@ _CPP_HEADERS = frozenset([
     'cwctype',
     ])
 
+_C_SYSTEM_DIRECTORIES = frozenset([
+    'libkern',
+    'sys',
+])
+
 # Type names
 _TYPES = re.compile(
     r'^(?:'
@@ -685,7 +690,7 @@ def Search(pattern, s):
 
 def _IsSourceExtension(s):
   """File extension (excluding dot) matches a source file extension."""
-  return s in ('c', 'cc', 'cpp', 'cxx')
+  return s in ('c', 'cc', 'cpp', 'cxx', 'm', 'mm')
 
 
 class _IncludeState(object):
@@ -4411,7 +4416,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
     CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error)
 
 
-_RE_PATTERN_INCLUDE = re.compile(r'^\s*#\s*include\s*([<"])([^>"]*)[>"].*$')
+_RE_PATTERN_INCLUDE = re.compile(r'^\s*#\s*(?:include|import)\s*([<"])([^>"]*)[>"].*$')
 # Matches the first component of a filename delimited by -s and _s. That is:
 #  _RE_FIRST_COMPONENT.match('foo').group(0) == 'foo'
 #  _RE_FIRST_COMPONENT.match('foo.cc').group(0) == 'foo'
@@ -4444,6 +4449,11 @@ def _DropCommonSuffixes(filename):
     if (filename.endswith(suffix) and len(filename) > len(suffix) and
         filename[-len(suffix) - 1] in ('-', '_')):
       return filename[:-len(suffix) - 1]
+
+  for suffix in ['Tests.h', 'Test.m', 'Test.mm', 'Tests.m', 'Tests.mm']:
+    if (filename.endswith(suffix) and len(filename) > len(suffix)):
+      return filename[:-len(suffix)]
+
   return os.path.splitext(filename)[0]
 
 
@@ -4524,6 +4534,30 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error):
   fileinfo = FileInfo(filename)
   line = clean_lines.lines[linenum]
 
+  # system-style includes should not be used for project includes
+  match = Match(r'#include\s*<(([^/>]+)/[^>]+)', line)
+  if match:
+    if match.group(2) not in _C_SYSTEM_DIRECTORIES:
+      error(filename, linenum, 'build/include', 4,
+            '<%s> should be #include "%s" or #import <%s>' %
+            (match.group(1), match.group(1), match.group(1)))
+
+  # C++ system files should not be #imported
+  match = Match(r'#import\s*<([^/>.]+)>', line)
+  if match:
+    error(filename, linenum, 'build/include', 4,
+          'C++ header <%s> was #imported. Should be #include <%s>' %
+          (match.group(1), match.group(1)))
+
+  # Prefer C++ wrappers for C headers
+  match = Match(r'#include\s*<(([^>]+).h)>', line)
+  if match:
+    wrapper = 'c' + match.group(2)
+    if wrapper in _CPP_HEADERS:
+        error(filename, linenum, 'build/include', 4,
+              'Prefer C++ header <%s> for C system header %s' %
+              (wrapper, match.group(1)))
+
   # "include" should use the new style "foo/bar.h" instead of just "bar.h"
   # Only do this check if the included header follows google naming
   # conventions.  If not, assume that it's a 3rd party API that
diff --git a/scripts/lint.sh b/scripts/lint.sh
index d0f82b1..9e33c87 100755
--- a/scripts/lint.sh
+++ b/scripts/lint.sh
@@ -14,21 +14,74 @@
 
 # Lints C++ files for conformance with the Google C++ style guide
 
-options=(
+# Joins the given arguments with the separator given as the first argument.
+function join() {
+  local IFS="$1"
+  shift
+  echo "$*"
+}
+
+git_options=(
     -z    # \0 terminate output
 )
 
+objc_lint_filters=(
+  # Objective-C uses #import and does not use header guards
+  -build/header_guard
+
+  # Inline definitions of Objective-C blocks confuse
+  -readability/braces
+
+  # C-style casts are acceptable in Objective-C++
+  -readability/casting
+
+  # Objective-C needs use type 'long' for interop between types like NSInteger
+  # and printf-style functions.
+  -runtime/int
+
+  # cpplint is generally confused by Objective-C mixing with C++.
+  #   * Objective-C method invocations in a for loop make it think its a
+  #     range-for
+  #   * Objective-C dictionary literals confuse brace spacing
+  #   * Empty category declarations ("@interface Foo ()") look like function
+  #     invocations
+  -whitespace
+)
+
+objc_lint_options=(
+  # cpplint normally excludes Objective-C++
+  --extensions=h,m,mm
+
+  # Objective-C style allows longer lines
+  --linelength=100
+
+  --filter=$(join , "${objc_lint_filters[@]}")
+)
+
 if [[ $# -gt 0 ]]; then
   # Interpret any command-line argument as a revision range
   command=(git diff --name-only)
-  options+=("$@")
+  git_options+=("$@")
 
 else
   # Default to operating on all files that match the pattern
   command=(git ls-files)
 fi
 
-
-"${command[@]}" "${options[@]}" \
+# Straight C++ files get regular cpplint
+"${command[@]}" "${git_options[@]}" \
     -- 'Firestore/core/**/*.'{h,cc} \
-  | xargs -0 python scripts/cpplint.py --quiet
+  | xargs -0 python scripts/cpplint.py --quiet 2>&1
+CPP_STATUS=$?
+
+# Objective-C++ files get a looser cpplint
+"${command[@]}" "${git_options[@]}" \
+    -- 'Firestore/Source/**/*.'{h,m,mm} \
+      'Firestore/Example/Tests/**/*.'{h,m,mm} \
+      'Firestore/core/**/*.mm' \
+  | xargs -0 python scripts/cpplint.py "${objc_lint_options[@]}" --quiet 2>&1
+OBJC_STATUS=$?
+
+if [[ $CPP_STATUS != 0 || $OBJC_STATUS != 0 ]]; then
+  exit 1
+fi