From 0a6618705389543191a8c44278d4c6791c84602a Mon Sep 17 00:00:00 2001 From: Gil Date: Mon, 9 Apr 2018 11:54:39 -0700 Subject: 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 --- scripts/cpplint.py | 38 ++++++++++++++++++++++++++++++-- scripts/lint.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 94 insertions(+), 7 deletions(-) (limited to 'scripts') 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 -- cgit v1.2.3